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

[Mark Peloquin]

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