Re: [Evms-devel] Re: [PATCH] EVMS core 3/4: evms_ioctl.h

From: "Mark Peloquin" <peloquin@us.ibm.com>
To: Andi Kleen <ak@suse.de>
Cc: linux-kernel@vger.kernel.org, evms-devel@lists.sourceforge.ne
Subject: Re: [Evms-devel] Re: [PATCH] EVMS core 3/4: evms_ioctl.h
Date: Fri, 4 Oct 2002 09:44:16 -0500	[thread overview]
Message-ID: <OF3A568674.694920EA-ON85256C48.004FE459@pok.ibm.com> (raw)

On 10/03/2002 at 6:49 PM, Andi Kleen wrote:
> I think you have some security holes in there:

 > +parms.buffer_address  = (u8 *)uvirt_to_kernel(parms32.buffer_address);
 > [...]
 > +set_fs(KERNEL_DS);
 > +rc = sys_ioctl(fd, kcmd, (unsigned long)karg);
 > +set_fs(old_fs);

> parms32.buffer_address comes from user space. With the set_fs you turn
> off all access checking. Surely when whatever sits at the bottom of
> sys_ioctl accesses it it'll use copy_from/to_user and it will do an
> unchecked reference of a user supplied pointer, allowing it to read/write
> all memory.

> Same bug is present in more functions.

> The rule is: when you do set_fs(KERNEL_DS) you have to copy all user
supplied
> pointers before it.

Yes, we became aware of this while working on sparc64 and have
coded the appropriate copy *before* set_fs(KERNEL_DS).
Unfortunately, that code didn't make it into CVS yet.
This will be fixed ASAP.

Thanks for pointing it out.
Mark