[PATCH 1/2] add helper for highmem checks

[PATCH 1/2] add helper for highmem checks

[Dave Hansen]

Boris, could you check that this series also fixes the /dev/mem
problem you were seeing?

--

We have a new debugging check on x86 that has caught a number
of long-standing bugs.  However, there is a _bit_ of collateral
damage with things that call __pa(high_memory).

We are now checking that any addresses passed to __pa() are
*valid* and can be dereferenced.

"high_memory", however, is not valid.  It marks the start of
highmem, and isn't itself a valid pointer.  But, those users
are really just asking "is this vaddr mapped"?  So, give them
a helper that does that, plus is also kind to our new
debugging check.


Signed-off-by: Dave Hansen <dave@linux.vnet.ibm.com>
---

 linux-2.6.git-dave/arch/x86/mm/pat.c     |   11 ++++++-----
 linux-2.6.git-dave/drivers/char/mem.c    |    4 ++--
 linux-2.6.git-dave/drivers/mtd/mtdchar.c |    2 +-
 linux-2.6.git-dave/include/linux/mm.h    |   13 +++++++++++++
 4 files changed, 22 insertions(+), 8 deletions(-)

diff -puN drivers/char/mem.c~clean-up-highmem-checks drivers/char/mem.c
--- linux-2.6.git/drivers/char/mem.c~clean-up-highmem-checks	2013-02-08 08:42:37.291222110 -0800
+++ linux-2.6.git-dave/drivers/char/mem.c	2013-02-08 12:27:27.837477867 -0800
@@ -51,7 +51,7 @@ static inline unsigned long size_inside_
 #ifndef ARCH_HAS_VALID_PHYS_ADDR_RANGE
 static inline int valid_phys_addr_range(phys_addr_t addr, size_t count)
 {
-	return addr + count <= __pa(high_memory);
+	return !phys_addr_is_highmem(addr + count);
 }
 
 static inline int valid_mmap_phys_addr_range(unsigned long pfn, size_t size)
@@ -250,7 +250,7 @@ static int uncached_access(struct file *
 	 */
 	if (file->f_flags & O_DSYNC)
 		return 1;
-	return addr >= __pa(high_memory);
+	return phys_addr_is_highmem(addr);
 #endif
 }
 #endif
diff -puN include/linux/mm.h~clean-up-highmem-checks include/linux/mm.h
--- linux-2.6.git/include/linux/mm.h~clean-up-highmem-checks	2013-02-08 08:42:37.295222148 -0800
+++ linux-2.6.git-dave/include/linux/mm.h	2013-02-08 09:01:49.758254468 -0800
@@ -1771,5 +1771,18 @@ static inline unsigned int debug_guardpa
 static inline bool page_is_guard(struct page *page) { return false; }
 #endif /* CONFIG_DEBUG_PAGEALLOC */
 
+static inline phys_addr_t last_lowmem_phys_addr(void)
+{
+	/*
+	 * 'high_memory' is not a pointer that can be dereferenced, so
+	 * avoid calling __pa() on it directly.
+	 */
+	return __pa(high_memory - 1);
+}
+static inline bool phys_addr_is_highmem(phys_addr_t addr)
+{
+	return addr > last_lowmem_paddr();
+}
+
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MM_H */
diff -puN arch/arm/mm/mmap.c~clean-up-highmem-checks arch/arm/mm/mmap.c
diff -puN arch/arm/mach-u300/core.c~clean-up-highmem-checks arch/arm/mach-u300/core.c
diff -puN arch/mips/loongson/common/mem.c~clean-up-highmem-checks arch/mips/loongson/common/mem.c
diff -puN arch/mips/mm/cache.c~clean-up-highmem-checks arch/mips/mm/cache.c
diff -puN arch/sh/mm/mmap.c~clean-up-highmem-checks arch/sh/mm/mmap.c
diff -puN arch/x86/mm/pat.c~clean-up-highmem-checks arch/x86/mm/pat.c
--- linux-2.6.git/arch/x86/mm/pat.c~clean-up-highmem-checks	2013-02-08 08:48:29.486594289 -0800
+++ linux-2.6.git-dave/arch/x86/mm/pat.c	2013-02-08 09:03:32.435231850 -0800
@@ -542,7 +542,7 @@ int phys_mem_access_prot_allowed(struct
 	      boot_cpu_has(X86_FEATURE_K6_MTRR) ||
 	      boot_cpu_has(X86_FEATURE_CYRIX_ARR) ||
 	      boot_cpu_has(X86_FEATURE_CENTAUR_MCR)) &&
-	    (pfn << PAGE_SHIFT) >= __pa(high_memory)) {
+	    phys_addr_is_highmem(pfn << PAGE_SHIFT)) {
 		flags = _PAGE_CACHE_UC;
 	}
 #endif
@@ -560,12 +560,13 @@ int kernel_map_sync_memtype(u64 base, un
 {
 	unsigned long id_sz;
 
-	if (base > __pa(high_memory-1))
+	if (phys_addr_is_highmem(base))
 		return 0;
 
-	id_sz = (__pa(high_memory-1) <= base + size) ?
-				__pa(high_memory) - base :
-				size;
+	if (phys_addr_is_highmem(base + size - 1))
+		id_sz = last_lowmem_phys_addr() - base + 1;
+	else
+		id_sz = size;
 
 	if (ioremap_change_attr((unsigned long)__va(base), id_sz, flags) < 0) {
 		printk(KERN_INFO "%s:%d ioremap_change_attr failed %s "
diff -puN drivers/mtd/mtdchar.c~clean-up-highmem-checks drivers/mtd/mtdchar.c
--- linux-2.6.git/drivers/mtd/mtdchar.c~clean-up-highmem-checks	2013-02-08 08:59:26.632884014 -0800
+++ linux-2.6.git-dave/drivers/mtd/mtdchar.c	2013-02-08 09:50:56.410398581 -0800
@@ -1189,7 +1189,7 @@ static int mtdchar_mmap(struct file *fil
 		vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
 
 #ifdef pgprot_noncached
-		if (file->f_flags & O_DSYNC || off >= __pa(high_memory))
+		if (file->f_flags & O_DSYNC || phys_addr_is_highmem(off))
 			vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
 #endif
 		if (io_remap_pfn_range(vma, vma->vm_start, off >> PAGE_SHIFT,
diff -puN arch/um/kernel/physmem.c~clean-up-highmem-checks arch/um/kernel/physmem.c
_

[PATCH 2/2] make /dev/kmem return error for highmem

[Dave Hansen]

I was auding the /dev/mem code for more questionable uses of
__pa(), and ran across this.

My assumption is that if you use /dev/kmem, you expect to be
able to read the kernel virtual mappings.  However, those
mappings _stop_ as soon as we hit high memory.  The
pfn_valid() check in here is good for memory holes, but since
highmem pages are still valid, it does no good for those.

Also, since we are now checking that __pa() is being done on
valid virtual addresses, this might have tripped the new
check.  Even with the new check, this code would have been
broken with the NUMA remapping code had we not ripped it
out:

	https://patchwork.kernel.org/patch/2075911/

Signed-off-by: Dave Hansen <dave@linux.vnet.ibm.com>
---

 linux-2.6.git-dave/drivers/char/mem.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff -puN drivers/char/mem.c~make-kmem-return-error-for-highmem drivers/char/mem.c
--- linux-2.6.git/drivers/char/mem.c~make-kmem-return-error-for-highmem	2013-02-08 12:27:57.033770045 -0800
+++ linux-2.6.git-dave/drivers/char/mem.c	2013-02-08 12:27:57.041770125 -0800
@@ -337,10 +337,19 @@ static int mmap_mem(struct file *file, s
 #ifdef CONFIG_DEVKMEM
 static int mmap_kmem(struct file *file, struct vm_area_struct *vma)
 {
+	unsigned long kernel_vaddr;
 	unsigned long pfn;
 
+	kernel_vaddr = (u64)vma->vm_pgoff << PAGE_SHIFT;
+	/*
+	 * pfn_valid() (below) does not trip for highmem addresses.  This
+	 * essentially means that we will be mapping gibberish in for them
+	 * instead of what the _kernel_ has mapped at the requested address.
+	 */
+	if (kernel_vaddr >= high_memory)
+		return -EIO;
 	/* Turn a kernel-virtual address into a physical page frame */
-	pfn = __pa((u64)vma->vm_pgoff << PAGE_SHIFT) >> PAGE_SHIFT;
+	pfn = __pa(kernel_vaddr) >> PAGE_SHIFT;
 
 	/*
 	 * RED-PEN: on some architectures there is more mapped memory than
diff -puN mm/nommu.c~make-kmem-return-error-for-highmem mm/nommu.c
_

Re: [PATCH 2/2] make /dev/kmem return error for highmem

[H. Peter Anvin]

On 02/08/2013 12:28 PM, Dave Hansen wrote:
> I was auding the /dev/mem code for more questionable uses of
> __pa(), and ran across this.
>
> My assumption is that if you use /dev/kmem, you expect to be
> able to read the kernel virtual mappings.  However, those
> mappings _stop_ as soon as we hit high memory.  The
> pfn_valid() check in here is good for memory holes, but since
> highmem pages are still valid, it does no good for those.
>
> Also, since we are now checking that __pa() is being done on
> valid virtual addresses, this might have tripped the new
> check.  Even with the new check, this code would have been
> broken with the NUMA remapping code had we not ripped it
> out:
>

It would be great if you could take a stab at fixing /dev/mem and 
/dev/kmem... there are a bunch of problems with both which seem to 
really translate to "HIGHMEM was never properly implemented"...

	-hpa


-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH 1/2] add helper for highmem checks

[H. Peter Anvin]

On 02/08/2013 12:28 PM, Dave Hansen wrote:
>   #endif
> diff -puN include/linux/mm.h~clean-up-highmem-checks include/linux/mm.h
> --- linux-2.6.git/include/linux/mm.h~clean-up-highmem-checks	2013-02-08 08:42:37.295222148 -0800
> +++ linux-2.6.git-dave/include/linux/mm.h	2013-02-08 09:01:49.758254468 -0800
> @@ -1771,5 +1771,18 @@ static inline unsigned int debug_guardpa
>   static inline bool page_is_guard(struct page *page) { return false; }
>   #endif /* CONFIG_DEBUG_PAGEALLOC */
>
> +static inline phys_addr_t last_lowmem_phys_addr(void)
> +{
> +	/*
> +	 * 'high_memory' is not a pointer that can be dereferenced, so
> +	 * avoid calling __pa() on it directly.
> +	 */
> +	return __pa(high_memory - 1);
> +}
> +static inline bool phys_addr_is_highmem(phys_addr_t addr)
> +{
> +	return addr > last_lowmem_paddr();
> +}
> +

Are we sure that high_memory - 1 is always a valid reference?  Consider 
especially the case where there is MMIO beyond end of memory on a system 
which has less RAM than the HIGHMEM boundary...

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH 1/2] add helper for highmem checks

[Dave Hansen]

On 02/08/2013 12:50 PM, H. Peter Anvin wrote:
> On 02/08/2013 12:28 PM, Dave Hansen wrote:
>> +static inline phys_addr_t last_lowmem_phys_addr(void)
>> +{
>> +    /*
>> +     * 'high_memory' is not a pointer that can be dereferenced, so
>> +     * avoid calling __pa() on it directly.
>> +     */
>> +    return __pa(high_memory - 1);
>> +}
>> +static inline bool phys_addr_is_highmem(phys_addr_t addr)
>> +{
>> +    return addr > last_lowmem_paddr();
>> +}
>> +
> 
> Are we sure that high_memory - 1 is always a valid reference?  Consider
> especially the case where there is MMIO beyond end of memory on a system
> which has less RAM than the HIGHMEM boundary...

Yeah, I think it is.  "high_memory" should point at either the end of
RAM, or the end of the linear map, whichever is lower.  See setup_arch():

        max_pfn = e820_end_of_ram_pfn();
	...
        high_memory = (void *)__va(max_pfn * PAGE_SIZE - 1) + 1;

or in the highmem init code:

        high_memory = (void *) __va(max_low_pfn * PAGE_SIZE - 1) + 1;

Re: [PATCH 1/2] add helper for highmem checks

[Borislav Petkov]

On Fri, Feb 08, 2013 at 12:28:13PM -0800, Dave Hansen wrote:
> 
> Boris, could you check that this series also fixes the /dev/mem
> problem you were seeing?
> 
> --
> 
> We have a new debugging check on x86 that has caught a number
> of long-standing bugs.  However, there is a _bit_ of collateral
> damage with things that call __pa(high_memory).
> 
> We are now checking that any addresses passed to __pa() are
> *valid* and can be dereferenced.
> 
> "high_memory", however, is not valid.  It marks the start of
> highmem, and isn't itself a valid pointer.  But, those users
> are really just asking "is this vaddr mapped"?  So, give them
> a helper that does that, plus is also kind to our new
> debugging check.
> 
> 
> Signed-off-by: Dave Hansen <dave@linux.vnet.ibm.com>
> ---
> 
>  linux-2.6.git-dave/arch/x86/mm/pat.c     |   11 ++++++-----
>  linux-2.6.git-dave/drivers/char/mem.c    |    4 ++--
>  linux-2.6.git-dave/drivers/mtd/mtdchar.c |    2 +-
>  linux-2.6.git-dave/include/linux/mm.h    |   13 +++++++++++++
>  4 files changed, 22 insertions(+), 8 deletions(-)
> 
> diff -puN drivers/char/mem.c~clean-up-highmem-checks drivers/char/mem.c
> --- linux-2.6.git/drivers/char/mem.c~clean-up-highmem-checks	2013-02-08 08:42:37.291222110 -0800
> +++ linux-2.6.git-dave/drivers/char/mem.c	2013-02-08 12:27:27.837477867 -0800
> @@ -51,7 +51,7 @@ static inline unsigned long size_inside_
>  #ifndef ARCH_HAS_VALID_PHYS_ADDR_RANGE
>  static inline int valid_phys_addr_range(phys_addr_t addr, size_t count)
>  {
> -	return addr + count <= __pa(high_memory);
> +	return !phys_addr_is_highmem(addr + count);
>  }
>  
>  static inline int valid_mmap_phys_addr_range(unsigned long pfn, size_t size)
> @@ -250,7 +250,7 @@ static int uncached_access(struct file *
>  	 */
>  	if (file->f_flags & O_DSYNC)
>  		return 1;
> -	return addr >= __pa(high_memory);
> +	return phys_addr_is_highmem(addr);
>  #endif
>  }
>  #endif
> diff -puN include/linux/mm.h~clean-up-highmem-checks include/linux/mm.h
> --- linux-2.6.git/include/linux/mm.h~clean-up-highmem-checks	2013-02-08 08:42:37.295222148 -0800
> +++ linux-2.6.git-dave/include/linux/mm.h	2013-02-08 09:01:49.758254468 -0800
> @@ -1771,5 +1771,18 @@ static inline unsigned int debug_guardpa
>  static inline bool page_is_guard(struct page *page) { return false; }
>  #endif /* CONFIG_DEBUG_PAGEALLOC */
>  
> +static inline phys_addr_t last_lowmem_phys_addr(void)
> +{
> +	/*
> +	 * 'high_memory' is not a pointer that can be dereferenced, so
> +	 * avoid calling __pa() on it directly.
> +	 */
> +	return __pa(high_memory - 1);
> +}
> +static inline bool phys_addr_is_highmem(phys_addr_t addr)
> +{
> +	return addr > last_lowmem_paddr();

I think you mean last_lowmem_phys_addr() here:

include/linux/mm.h: In function ‘phys_addr_is_highmem’:
include/linux/mm.h:1764:2: error: implicit declaration of function ‘last_lowmem_paddr’ [-Werror=implicit-function-declaration]
cc1: some warnings being treated as errors
make[1]: *** [arch/x86/kernel/asm-offsets.s] Error 1

Changed.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH 1/2] add helper for highmem checks

[Borislav Petkov]

On Sat, Feb 09, 2013 at 10:41:21AM +0100, Borislav Petkov wrote:
> > +static inline bool phys_addr_is_highmem(phys_addr_t addr)
> > +{
> > +	return addr > last_lowmem_paddr();
> 
> I think you mean last_lowmem_phys_addr() here:
> 
> include/linux/mm.h: In function ‘phys_addr_is_highmem’:
> include/linux/mm.h:1764:2: error: implicit declaration of function ‘last_lowmem_paddr’ [-Werror=implicit-function-declaration]
> cc1: some warnings being treated as errors
> make[1]: *** [arch/x86/kernel/asm-offsets.s] Error 1
> 
> Changed.

With this change, they definitely fix something because I even get X on
the box started. Previously, it would spit out the warning and wouldn't
start X with the login window. And my suspicion is that wdm (WINGs
display manager) I'm using, does /dev/mem accesses when it starts and it
obviously failed. Now not so much :-)

Tested-by: Borislav Petkov <bp@suse.de>

Thanks.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH 1/2] add helper for highmem checks

[Dave Hansen]

On 02/09/2013 02:47 AM, Borislav Petkov wrote:
> On Sat, Feb 09, 2013 at 10:41:21AM +0100, Borislav Petkov wrote:
> With this change, they definitely fix something because I even get X on
> the box started. Previously, it would spit out the warning and wouldn't
> start X with the login window. And my suspicion is that wdm (WINGs
> display manager) I'm using, does /dev/mem accesses when it starts and it
> obviously failed. Now not so much :-)

That's crazy.  Didn't expect that at all.

I guess X is happier getting an error than getting random pages back.
I'm working on a set of patches now that should get it _working_ instead
of just returning an error.

Re: [PATCH 1/2] add helper for highmem checks

[H. Peter Anvin]

On 02/11/2013 09:32 AM, Dave Hansen wrote:
> On 02/09/2013 02:47 AM, Borislav Petkov wrote:
>> On Sat, Feb 09, 2013 at 10:41:21AM +0100, Borislav Petkov wrote:
>> With this change, they definitely fix something because I even get X on
>> the box started. Previously, it would spit out the warning and wouldn't
>> start X with the login window. And my suspicion is that wdm (WINGs
>> display manager) I'm using, does /dev/mem accesses when it starts and it
>> obviously failed. Now not so much :-)
>
> That's crazy.  Didn't expect that at all.
>
> I guess X is happier getting an error than getting random pages back.
> I'm working on a set of patches now that should get it _working_ instead
> of just returning an error.
>

Awesome :)

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH 1/2] add helper for highmem checks

[Borislav Petkov]

On Mon, Feb 11, 2013 at 09:32:41AM -0800, Dave Hansen wrote:
> That's crazy. Didn't expect that at all.
>
> I guess X is happier getting an error than getting random pages back.

Yeah, I think this is something special only this window manager wdm
does. The line below has appeared repeatedly in the logs earlier:

Feb  5 23:02:02 a1 wdm: Cannot read randomFile "/dev/mem", errno = 14

This happens when wdm starts so I'm going to guess it uses it for
something funny, "randomFile" it calls it??

With the WARN_ON check added and booting 3.8-rc6, it would choke wdm
somehow and it wouldn't start properly so that even the error out above
doesn't happen. Oh well ...

> I'm working on a set of patches now that should get it _working_
> instead of just returning an error.

Yeah, send them on and I'll run them.

Thanks.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH 1/2] add helper for highmem checks

[H. Peter Anvin]

Oh, craptastic.  X used to hash /dev/mem to get a random seed.  It should have stopped that long ago, and used /dev/[u]random.

Borislav Petkov <bp@alien8.de> wrote:

>On Mon, Feb 11, 2013 at 09:32:41AM -0800, Dave Hansen wrote:
>> That's crazy. Didn't expect that at all.
>>
>> I guess X is happier getting an error than getting random pages back.
>
>Yeah, I think this is something special only this window manager wdm
>does. The line below has appeared repeatedly in the logs earlier:
>
>Feb  5 23:02:02 a1 wdm: Cannot read randomFile "/dev/mem", errno = 14
>
>This happens when wdm starts so I'm going to guess it uses it for
>something funny, "randomFile" it calls it??
>
>With the WARN_ON check added and booting 3.8-rc6, it would choke wdm
>somehow and it wouldn't start properly so that even the error out above
>doesn't happen. Oh well ...
>
>> I'm working on a set of patches now that should get it _working_
>> instead of just returning an error.
>
>Yeah, send them on and I'll run them.
>
>Thanks.

-- 
Sent from my mobile phone. Please excuse brevity and lack of formatting.

Re: [PATCH 1/2] add helper for highmem checks

[Borislav Petkov]

On Mon, Feb 11, 2013 at 11:44:12AM -0800, H. Peter Anvin wrote:
> Oh, craptastic. X used to hash /dev/mem to get a random seed. It
> should have stopped that long ago, and used /dev/[u]random.

That's because debian still has this WINGs window manager which hasn't
seen any new releases since 2005: http://voins.program.ru/wdm/ and I'm
using it because I don't want the pompous crap of the other display
managers.

But this one uses /dev/mem as a randomFile only by default - there's a
configuration variable DisplayManager.randomFile which can be pointed
away from /dev/mem so that's easily fixable.

Mind you, I wouldnt've caught the issue if I wasn't using this ancient
thing in its default settings :o).

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH 1/2] add helper for highmem checks

[H. Peter Anvin]

On 02/11/2013 02:34 PM, Borislav Petkov wrote:
> On Mon, Feb 11, 2013 at 11:44:12AM -0800, H. Peter Anvin wrote:
>> Oh, craptastic. X used to hash /dev/mem to get a random seed. It
>> should have stopped that long ago, and used /dev/[u]random.
> 
> That's because debian still has this WINGs window manager which hasn't
> seen any new releases since 2005: http://voins.program.ru/wdm/ and I'm
> using it because I don't want the pompous crap of the other display
> managers.
> 
> But this one uses /dev/mem as a randomFile only by default - there's a
> configuration variable DisplayManager.randomFile which can be pointed
> away from /dev/mem so that's easily fixable.
> 
> Mind you, I wouldnt've caught the issue if I wasn't using this ancient
> thing in its default settings :o).
> 

The X server itself used to do that.  Are you saying that wdm is a
*privileged process*?

	-hpa

Re: [PATCH 1/2] add helper for highmem checks

[Borislav Petkov]

On Mon, Feb 11, 2013 at 02:46:43PM -0800, H. Peter Anvin wrote:
> The X server itself used to do that. Are you saying that wdm is a
> *privileged process*?

Nah, it is a simple display manager you start with /etc/init.d/wdm init
script. Like the other display managers gdm, kdm, etc.

But it looks like wdm has copied stuff from xdm (from the README):

"Wdm is a modification of XFree86's xdm package for graphically handling
authentication and system login. Most of xdm has been preserved (XFree86
4.2.1.1) with the Login interface based on a WINGs implementation using
Tom Rothamel's "external greet" interface (see AUTHORS)."

And from looking at the part in the source which does the /dev/mem
accesses, it comes from XFree86's source apparently, this is at the
beginning of src/wdm/genauth.c:

/* $Xorg: genauth.c,v 1.5 2001/02/09 02:05:40 xorgcvs Exp $ */
/*

   Copyright 1988, 1998  The Open Group
...

so this explains why it behaves like the X server in that respect.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

Re: [PATCH 1/2] add helper for highmem checks

[H. Peter Anvin]

On 02/11/2013 03:00 PM, Borislav Petkov wrote:
> On Mon, Feb 11, 2013 at 02:46:43PM -0800, H. Peter Anvin wrote:
>> The X server itself used to do that. Are you saying that wdm is a
>> *privileged process*?
> 
> Nah, it is a simple display manager you start with /etc/init.d/wdm init
> script. Like the other display managers gdm, kdm, etc.
> 
> But it looks like wdm has copied stuff from xdm (from the README):
> 
> "Wdm is a modification of XFree86's xdm package for graphically handling
> authentication and system login. Most of xdm has been preserved (XFree86
> 4.2.1.1) with the Login interface based on a WINGs implementation using
> Tom Rothamel's "external greet" interface (see AUTHORS)."
> 
> And from looking at the part in the source which does the /dev/mem
> accesses, it comes from XFree86's source apparently, this is at the
> beginning of src/wdm/genauth.c:
> 

Oh, it's not a *window manager*, it is a *session manager* (display
manager), and so it runs as root by default.

Plug the damned hole, submit a bug report to Debian to change the
default, and let's be done with it.  That being said, it did flag a real
problem, but what it is doing is dangerous.

	-hpa