[PATCH]: Fix D-cache corruption in mremap() (was Re:

[PATCH]: Fix D-cache corruption in mremap() (was Re:

[David Miller]

Thanks to some excellent corrupted status and available files
Daniel sent to me, I was able to track down the bug.  This
bug has been around basically forever, just minor data layout
changes due to arbitrary unrelated kernel and userland library
modifications make the bug go away for some time and then
reappear.  It was always there.

Anyways, give this patch below a test, it should fix things.
It is against the current 2.6.17 tree.

I'll also post a little reproducer program that can be used to
accurately see if the patch really fixes the problem on your
machine or not, which should help with the variability of the
dpkg corruption case.

Please test this, I need to know if this works for everyone
or not.

Thanks.

diff-tree 0b0968a3e691771bf87e1ce747b2c7d23b5526c8 (from 951bc82c53f30ec6b4c2d04a051e74ea9a89b669)
Author: David S. Miller <davem@sunset.sfo1.dsl.speakeasy.net>
Date:   Thu Jun 1 17:47:25 2006 -0700

    [SPARC64]: Fix D-cache corruption in mremap
    
    If we move a mapping from one virtual address to another,
    and this changes the virtual color of the mapping to those
    pages, we can see corrupt data due to D-cache aliasing.
    
    Check for and deal with this by overriding the move_pte()
    macro.  Set things up so that other platforms can cleanly
    override the move_pte() macro too.
    
    Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/asm-generic/pgtable.h b/include/asm-generic/pgtable.h
index 358e4d3..c2059a3 100644
--- a/include/asm-generic/pgtable.h
+++ b/include/asm-generic/pgtable.h
@@ -159,17 +159,8 @@ static inline void ptep_set_wrprotect(st
 #define lazy_mmu_prot_update(pte)	do { } while (0)
 #endif
 
-#ifndef __HAVE_ARCH_MULTIPLE_ZERO_PAGE
+#ifndef __HAVE_ARCH_MOVE_PTE
 #define move_pte(pte, prot, old_addr, new_addr)	(pte)
-#else
-#define move_pte(pte, prot, old_addr, new_addr)				\
-({									\
- 	pte_t newpte = (pte);						\
-	if (pte_present(pte) && pfn_valid(pte_pfn(pte)) &&		\
-			pte_page(pte) = ZERO_PAGE(old_addr))		\
-		newpte = mk_pte(ZERO_PAGE(new_addr), (prot));		\
-	newpte;								\
-})
 #endif
 
 /*
diff --git a/include/asm-mips/pgtable.h b/include/asm-mips/pgtable.h
index 702a28f..69cebbd 100644
--- a/include/asm-mips/pgtable.h
+++ b/include/asm-mips/pgtable.h
@@ -70,7 +70,15 @@ extern unsigned long zero_page_mask;
 #define ZERO_PAGE(vaddr) \
 	(virt_to_page(empty_zero_page + (((unsigned long)(vaddr)) & zero_page_mask)))
 
-#define __HAVE_ARCH_MULTIPLE_ZERO_PAGE
+#define __HAVE_ARCH_MOVE_PTE
+#define move_pte(pte, prot, old_addr, new_addr)				\
+({									\
+ 	pte_t newpte = (pte);						\
+	if (pte_present(pte) && pfn_valid(pte_pfn(pte)) &&		\
+			pte_page(pte) = ZERO_PAGE(old_addr))		\
+		newpte = mk_pte(ZERO_PAGE(new_addr), (prot));		\
+	newpte;								\
+})
 
 extern void paging_init(void);
 
diff --git a/include/asm-sparc64/pgtable.h b/include/asm-sparc64/pgtable.h
index c44e746..cd464f4 100644
--- a/include/asm-sparc64/pgtable.h
+++ b/include/asm-sparc64/pgtable.h
@@ -689,6 +689,23 @@ static inline void set_pte_at(struct mm_
 #define pte_clear(mm,addr,ptep)		\
 	set_pte_at((mm), (addr), (ptep), __pte(0UL))
 
+#ifdef DCACHE_ALIASING_POSSIBLE
+#define __HAVE_ARCH_MOVE_PTE
+#define move_pte(pte, prot, old_addr, new_addr)				\
+({									\
+ 	pte_t newpte = (pte);						\
+	if (tlb_type != hypervisor && pte_present(pte)) {		\
+		unsigned long this_pfn = pte_pfn(pte);			\
+									\
+		if (pfn_valid(this_pfn) &&				\
+		    (((old_addr) ^ (new_addr)) & (1 << 13)))		\
+			flush_dcache_page_all(current->mm,		\
+					      pfn_to_page(this_pfn));	\
+	}								\
+	newpte;								\
+})
+#endif
+
 extern pgd_t swapper_pg_dir[2048];
 extern pmd_t swapper_low_pmd_dir[2048];
 

Re: [PATCH]: Fix D-cache corruption in mremap()

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Thu, 01 Jun 2006 17:52:58 -0700 (PDT)

> I'll also post a little reproducer program that can be used to
> accurately see if the patch really fixes the problem on your
> machine or not, which should help with the variability of the
> dpkg corruption case.

As promised:

/* mremap() stress tester for D-cache aliasing platforms.
 *
 * Copyright (C) 2006 David S. Miller (davem@davemloft.net)
 *
 * It tries to exercise the case where mremap() with MREMAP_MAYMOVE
 * will actually place the area somewhere else and not just extend
 * or shrink at the existing mapping location.
 *
 * This can cause problems on virtually indexed cache platforms
 * if they do not implement move_pte() with logic to handle a
 * change of virtual color.  If the cache virtual color changes
 * when mremap() moves the mapping around, we can end up accessing
 * stale aliases in the cache on subsequent cpu accesses to the
 * new virtual addresses.
 *
 * This bug was first discovered as file corruption occuring occaisionally
 * in 'dpkg'.  When 'dpkg' is building a 'status' or 'available' file it
 * uses an expanding allocator called 'varbuf' which uses realloc()
 * heavilly to expand it's internal buffer.  In glibc, for very large
 * malloc() buffer sizes, realloc() uses mremap() with MREMAP_MAYMOVE to
 * try and satisfy expansion requests.  Most of the time there is room
 * in the address space, but if we bump up against anoter mmap() region
 * it can move things around.  If this results in different D-cache coloring
 * for the region we can end up reading corrupt data from existing aliases
 * in the D-cache.
 *
 * It was very hard to reproduce, so this test case was written.
 */

#define _GNU_SOURCE
#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>

/* XXX There is no easy way to get at mremap()'s MREAMP_FIXED functionality
 * XXX with older glibc versions...
 */
extern void *mremap(void *old_address, size_t old_size, size_t new_size,
		    unsigned long flags, ...);
# define MREMAP_MAYMOVE	1
# define MREMAP_FIXED	2

extern void *mmap(void *start, size_t length, int prot, int flags, int fd,
		  off_t offset);

/* Same on all platforms... */
#define PROT_READ	0x1		/* Page can be read.  */
#define PROT_WRITE	0x2		/* Page can be written.  */

#if defined(__alpha__)
#define MAP_FIXED	0x100		/* Interpret addr exactly.  */
#else
#if defined(__parisc__)
#define MAP_FIXED	0x04		/* Interpret addr exactly.  */
#else
#define MAP_FIXED	0x10		/* Interpret addr exactly.  */
#endif
#endif

#if defined(__alpha__) || defined(__parisc__)
#define MAP_ANONYMOUS	0x10		/* Don't use a file.  */
#else
#if defined(__mips__) || defined(__xtensa__)
#define MAP_ANONYMOUS	0x800		/* Don't use a file.  */
#else
#define MAP_ANONYMOUS	0x20		/* Don't use a file.  */
#endif
#endif

#define MAP_PRIVATE	0x02		/* Changes are private.  */

#define MAP_FAILED	((void *) -1)

static int page_size;

static void *unmapped_region;

#define MAX_OFFSET	8

static int init_main_buf(void **main_buf_p, int *main_buf_size_p)
{
	page_size = getpagesize();
	*main_buf_size_p = page_size;

	unmapped_region = mmap(NULL, (MAX_OFFSET + 1) * page_size,
			       PROT_READ,
			       MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
	if (unmapped_region = (void *) MAP_FAILED) {
		perror("mmap() of unmapped_region");
		return 1;
	}
	fprintf(stdout, "unmapped region at %p\n", unmapped_region);
	fflush(stdout);

	*main_buf_p = mmap(unmapped_region, *main_buf_size_p,
			   PROT_READ | PROT_WRITE,
			   MAP_FIXED | MAP_PRIVATE | MAP_ANONYMOUS,
			   -1, 0);
	if (*main_buf_p = (void *) MAP_FAILED) {
		perror("Initial 1-page mmap() of main_buf");
		return 1;
	}

	fprintf(stdout, "Initial main_buf at %p\n", *main_buf_p);
	fflush(stdout);

	return 0;
}

static void destroy_main_buf(void *main_buf, int main_buf_size)
{
	munmap(main_buf, main_buf_size);
}

static unsigned int key = 0x00000001;

static void fill_page(void *start)
{
	unsigned int *p = start;
	unsigned int *end = start + page_size;

	while (p < end) {
		volatile unsigned int *xp = (volatile unsigned int *) p;

		(void) *xp;

		*p++ = (unsigned int) (p - (unsigned int *) start) + key;
	}
}

static int remap_page(void **main_buf_p, int *main_buf_size_p, int off)
{
	void *p;

	p = mremap(*main_buf_p,
		   *main_buf_size_p,
		   *main_buf_size_p,
		   MREMAP_FIXED | MREMAP_MAYMOVE,
		   unmapped_region + (off * page_size));
	if (p = (void *) MAP_FAILED) {
		perror("mremap() of main_buf");
		return 1;
	}
	*main_buf_p = p;

	return 0;
}

static void check_page(void *start)
{
	unsigned int *p = start;
	unsigned int *end = start + page_size;

	while (p < end) {
		unsigned int val = *p;
		unsigned int exp_val;

		exp_val = (unsigned int) (p - (unsigned int *) start) + key;

		if (val != exp_val) {
			fprintf(stderr, "Bogus value %08x should be %08x\n",
				val, exp_val);
			exit(99);
		}
		p++;
	}
}

static int do_test(void)
{
	void *main_buf;
	int main_buf_size;
	int err, i;

	err = init_main_buf(&main_buf, &main_buf_size);
	if (err)
		return 1;

	while (1) {
		for (i = 0; i < MAX_OFFSET; i++) {
			fill_page(main_buf);

			remap_page(&main_buf, &main_buf_size, i + 1);

			check_page(main_buf);

			key++;
		}
	}

	destroy_main_buf(main_buf, main_buf_size);

	return 0;
}

int main(int argc, char **argv, char **envp)
{
	page_size = getpagesize();

	return do_test();
}

Re: [PATCH]: Fix D-cache corruption in mremap()

[Christian Joensson]

On 6/2/06, David Miller <davem@davemloft.net> wrote:
> From: David Miller <davem@davemloft.net>
> Date: Thu, 01 Jun 2006 17:52:58 -0700 (PDT)
>
> > I'll also post a little reproducer program that can be used to
> > accurately see if the patch really fixes the problem on your
> > machine or not, which should help with the variability of the
> > dpkg corruption case.
>
> As promised:
>
> /* mremap() stress tester for D-cache aliasing platforms.
>  *
>  * Copyright (C) 2006 David S. Miller (davem@davemloft.net)
>  *

great, so if I compile with gcc and run it... I get this:

[chj@arnljot ~]$ gcc mremap-stress-tester.c
[chj@arnljot ~]$ ./a.out
unmapped region at 0xf7d72000
Initial main_buf at 0xf7d72000
Bogus value 00000001 should be 00000002
[chj@arnljot ~]$ uname -a
Linux arnljot 2.6.16-1.2128sp4 #1 Fri May 26 10:08:13 EDT 2006 sparc64
sparc64 sparc64 GNU/Linux
[chj@arnljot ~]$

which means?

-- 
Cheers,

/ChJ

Re: [PATCH]: Fix D-cache corruption in mremap()

[David Miller]

From: "Christian Joensson" <christian.joensson@gmail.com>
Date: Fri, 2 Jun 2006 10:04:29 +0200

> On 6/2/06, David Miller <davem@davemloft.net> wrote:
> > From: David Miller <davem@davemloft.net>
> > Date: Thu, 01 Jun 2006 17:52:58 -0700 (PDT)
> >
> > > I'll also post a little reproducer program that can be used to
> > > accurately see if the patch really fixes the problem on your
> > > machine or not, which should help with the variability of the
> > > dpkg corruption case.
> >
> > As promised:
> >
> > /* mremap() stress tester for D-cache aliasing platforms.
> >  *
> >  * Copyright (C) 2006 David S. Miller (davem@davemloft.net)
> >  *
> 
> great, so if I compile with gcc and run it... I get this:
> 
> [chj@arnljot ~]$ gcc mremap-stress-tester.c
> [chj@arnljot ~]$ ./a.out
> unmapped region at 0xf7d72000
> Initial main_buf at 0xf7d72000
> Bogus value 00000001 should be 00000002
> [chj@arnljot ~]$ uname -a
> Linux arnljot 2.6.16-1.2128sp4 #1 Fri May 26 10:08:13 EDT 2006 sparc64
> sparc64 sparc64 GNU/Linux
> [chj@arnljot ~]$
> 
> which means?

It means the kernel you are running has the bug, try to rerun
it with the kernel patch I posted, it should no longer produce
the:

Bogus value 00000001 should be 00000002

lines.

Re: [PATCH]: Fix D-cache corruption in mremap() (was Re: 2.6.17-rc3+git

[Meelis Roos]

> Anyways, give this patch below a test, it should fix things.
> It is against the current 2.6.17 tree.
>
> I'll also post a little reproducer program that can be used to
> accurately see if the patch really fixes the problem on your
> machine or not, which should help with the variability of the
> dpkg corruption case.
>
> Please test this, I need to know if this works for everyone
> or not.

Works on my Ulta 5 - without the patch I get the error at once and with 
the patch I get no error for half an hour.

-- 
Meelis Roos (mroos@linux.ee)

Re: [PATCH]: Fix D-cache corruption in mremap() (was Re: 2.6.17-rc3+git

[Gustavo Zacarias]

David Miller wrote:

> I'll also post a little reproducer program that can be used to
> accurately see if the patch really fixes the problem on your
> machine or not, which should help with the variability of the
> dpkg corruption case.
> 
> Please test this, I need to know if this works for everyone
> or not.

Works for me too on my u5.
On a side note it seems to have fixed the alsa oops i reported some time 
ago that's missing from the archives.
Basically listen to some ogg file with alsa output with ogg123 
(vorbis-tools), usually when it's finished or if you break it you'll get 
it nicely (at least with snd-sun-cs4231, i think ens1371 too but i have 
it out of my box atm).

-- 
Gustavo Zacarias
Gentoo/SPARC monkey

Re: [PATCH]: Fix D-cache corruption in mremap()

[Daniel Smolik]

David Miller napsal(a):
> From: "Christian Joensson" <christian.joensson@gmail.com>
> Date: Fri, 2 Jun 2006 10:04:29 +0200
> 
> 
>>On 6/2/06, David Miller <davem@davemloft.net> wrote:
>>
>>>From: David Miller <davem@davemloft.net>
>>>Date: Thu, 01 Jun 2006 17:52:58 -0700 (PDT)
>>>
>>>
>>>>I'll also post a little reproducer program that can be used to
>>>>accurately see if the patch really fixes the problem on your
>>>>machine or not, which should help with the variability of the
>>>>dpkg corruption case.

May be OK on my E250 ? mmremap stress program never returns is it ok ?
Problem with dselect is solved. In wich version of kernel will be 
patch included ?

			Many thanks
					Dan

Re: [PATCH]: Fix D-cache corruption in mremap()

[David Miller]

From: Daniel Smolik <marvin@mydatex.cz>
Date: Fri, 02 Jun 2006 22:18:29 +0200

> May be OK on my E250 ? mmremap stress program never returns is it ok ?
> Problem with dselect is solved. In wich version of kernel will be 
> patch included ?

The program just runs forever until it triggers the bug.
If it runs fine for more than a few seconds you are ok.

Thank you very much for testing.

I plan to put this into 2.6.17 and give it to the 2.6.16-stable folks
as well.  If Chris W. or Greg KH are still doing 2.6.15-stable
releases I'll ask them to toss it into there too.

I'll try to take care of that over the weekend.

Re: [PATCH]: Fix D-cache corruption in mremap()

[David Miller]

From: Meelis Roos <mroos@linux.ee>
Date: Fri, 2 Jun 2006 16:17:29 +0300 (EEST)

> Works on my Ulta 5 - without the patch I get the error at once and with 
> the patch I get no error for half an hour.

Great, please watch for any dpkg database file corruption.  That's the
reason all of this started :)

Re: [PATCH]: Fix D-cache corruption in mremap()

[David Miller]

From: Gustavo Zacarias <gustavoz@gentoo.org>
Date: Fri, 02 Jun 2006 12:31:37 -0300

> On a side note it seems to have fixed the alsa oops i reported some time 
> ago that's missing from the archives.
> Basically listen to some ogg file with alsa output with ogg123 
> (vorbis-tools), usually when it's finished or if you break it you'll get 
> it nicely (at least with snd-sun-cs4231, i think ens1371 too but i have 
> it out of my box atm).

Glad to hear it fixes other bugs :)

Thanks for testing.

Re: [PATCH]: Fix D-cache corruption in mremap()

[Jurij Smakov]

On Fri, 2 Jun 2006, David Miller wrote:

> Great, please watch for any dpkg database file corruption.  That's the
> reason all of this started :)

I've finally got to reinstalling my machine and re-run the same 
upgrade/install which triggered the bug last time. With patched up kernel 
everything worked fine. Thanks a lot for taking care of it so fast, Dave.

Best regards,

Jurij Smakov                                        jurij@wooyd.org
Key: http://www.wooyd.org/pgpkey/                   KeyID: C99E03CC

Re: [PATCH]: Fix D-cache corruption in mremap()

[David Miller]

From: Jurij Smakov <jurij@wooyd.org>
Date: Sat, 3 Jun 2006 12:01:24 -0700 (PDT)

> On Fri, 2 Jun 2006, David Miller wrote:
> 
> > Great, please watch for any dpkg database file corruption.  That's the
> > reason all of this started :)
> 
> I've finally got to reinstalling my machine and re-run the same 
> upgrade/install which triggered the bug last time. With patched up kernel 
> everything worked fine. Thanks a lot for taking care of it so fast, Dave.

Thanks for testing.

Re: [PATCH]: Fix D-cache corruption in mremap()

[Meelis Roos]

>> Works on my Ulta 5 - without the patch I get the error at once and with
>> the patch I get no error for half an hour.
>
> Great, please watch for any dpkg database file corruption.  That's the
> reason all of this started :)

Seems to have survived a large debian update since the patch was in 
place, so it seems to work. Thanks!

-- 
Meelis Roos (mroos@linux.ee)

Re: [PATCH]: Fix D-cache corruption in mremap()

[Marc Zyngier]

Just to add yet another testing point.

The latest git runs nicely on my oddest sparc64 system, a Netra AX1105
(a strange crossover between a Blade-100 and a Netra-X1).

Thanks a lot for fixing that, David.

	M.
-- 
And if you don't know where you're going, any road will take you there...

Re: [PATCH]: Fix D-cache corruption in mremap() (was Re: 2.6.17-rc3+git seems to corrupt dpkg databa

[Ludovic Courtès]

Hi,

7 days, 14 hours, 26 minutes, 10 seconds ago, 
David Miller wrote:
> Anyways, give this patch below a test, it should fix things.
> It is against the current 2.6.17 tree.

It seems to fix the bug (according to `mremap-stress' and with
2.6.17-rc6) on my U5 (I was hoping it would also somehow allow X.org 7
to run but unfortunately it doesn't ;-)).

Thanks!

Ludovic.