From: Thomas H. <th...@sh...> - 2009-08-19 07:33:09
|
Kristian Høgsberg wrote: >> I also think without looking to closely that the drm_read() function doesn't >> do return values properly (see ldd3 and man 2 read for return values for the >> various blocking modes). In particular, wait_event_interruptible() may >> return -ERESTARTSYS which should never be returned to user-space. >> > > Returning -ERESTARTSYS from read is the expected behaviour in case you > get a signal as far as I know and is handled by glibc. It's used all > over the kernel. > Ah , yes. I was confusing syscalls and ioctls. Sorry. > >> It also looks like the new ioctl argument lengths are not 64 bit aligned? >> > > Not sure what you mean... that the size of the ioctl struct isn't a > multiple of 64 bits? That's not a requirement. > You'll get into 64 / 32 bit incompatibilities. DRM used to check that the user-space size and the kernel-space size of core ioctl arguments were the same, and that they had the same read / write mode. This has been changed so that the kernel always uses the kernel space size when copying. (I'm not sure why, since it was actually a good way to catch errors and should ideally be implemented by drivers as well). Anyway, If your 32-bit argument is 12 bytes and the 64-bit kernel tries to copy 16, you might end up with an EFAULT in the ioctl. Of course one could change the drm implementation to always use the user-space size as it does for driver-specific ioctls. Better to use all structs padded to 64-bits. Particularly if there are arguments which have structs embedded within other structs. /Thomas > cheers, > Kristian > |