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

Andi Kleen
2002-10-03T23:44:03Z

On Thu, Oct 03, 2002 at 05:30:46PM -0500, Kevin Corry wrote:
> In general, we are aware of the issues with using 32-bit user-space on top of
> 64-bit kernel. If you take a look at evms.c you will find several functions
> that get registered at init-time with the 32-to-64-bit ioctl conversion code.
> These take care of translating pointers from user-space to kernel-space in
> this situation. EVMS has been tested on ppc64 with success, and we have
> someone currently running tests on sparc64 to make sure it works there as
> well.

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.

-Andi