fix rawctl compat ioctls breakage on amd64 and itanic
Subsystems (2)
Al Viro
maintainerPATCH 1/1 fix rawctl compat ioctls breakage on amd64 and itanic
RAW_SETBIND and RAW_GETBIND 32bit versions are fscked in interesting
ways.
1) fs/compat_ioctl.c has COMPATIBLE_IOCTL(RAW_SETBIND) followed by HANDLE_IOCTL(RAW_SETBIND, raw_ioctl). The latter is ignored.
2) on amd64 (and itanic) the damn thing is broken - we have int + u64 + u64 and layouts on i386 and amd64 are _not_ the same. raw_ioctl() would work there, but it's never called due to (1). As it is, i386 /sbin/raw definitely doesn't work on amd64 boxen.
3) switching to raw_ioctl() as is would *not* work on e.g. sparc64 and ppc64, which would be rather sad, seeing that normal userland there is 32bit. The thing is, slapping __packed on the struct in question does not DTRT - it eliminates *all* padding. The real solution is to use compat_u64.
4) of course, all that stuff has no business being outside of raw.c in the first place - there should be ->compat_ioctl() for /dev/rawctl instead of messing with compat_ioctl.c.
Patch below deals with that crap. Builds, tested, seems to be working fine... I don't know if there's any point in merging that before .28-final - it does fix a real bug, all right, but
a) bug affects very few binaries - it's triggered by i386 binary that
does /dev/rawctl ioctls and happens to be run on amd64.
b) the breakage had been there for about 5 years, so it's not
exactly a recent regression.
Up to you...
---
drivers/char/raw.c +142 -92
@@ -20,6 +20,7 @@#include <linux/device.h> #include <linux/mutex.h> #include <linux/smp_lock.h> +#include <linux/compat.h> #include <asm/uaccess.h>
@@ -128,11 +129,81 @@ raw_ioctl(struct inode *inode, struct file *filp,return blkdev_ioctl (bdev,0 , command, arg); } -static void bind_device (struct raw_config_request *rq) +static int bind_set (int number,u64 major,u64 minor) { -device_destroy (raw_class,MKDEV (RAW_MAJOR , rq->raw_minor)); -device_create (raw_class,NULL ,MKDEV (RAW_MAJOR , rq->raw_minor),NULL , -"raw%d" , rq->raw_minor); +dev_t dev =MKDEV (major, minor); +struct raw_device_data *rawdev; +int err =0 ; + +if (number <=0 || number >=MAX_RAW_MINORS ) +return -EINVAL ; + +if (MAJOR (dev) != major ||MINOR (dev) != minor) +return -EINVAL ; + + rawdev = &raw_devices[number]; + +/* +* This is like making block devices, so demand the +* same capability +*/ +if (!capable (CAP_SYS_ADMIN )) +return -EPERM ; + +/* +* For now, we don't need to check that the underlying +* block device is present or not: we can do that when +* the raw device is opened. Just check that the +* major/minor numbers make sense. +*/ + +if (MAJOR (dev) ==0 && dev !=0 ) +return -EINVAL ; + +mutex_lock (&raw_mutex); +if (rawdev->inuse) { +mutex_unlock (&raw_mutex); +return -EBUSY ; + } +if (rawdev->binding) { +bdput (rawdev->binding); +module_put (THIS_MODULE ); + } +if (!dev) { +/* unbind */ + rawdev->binding =NULL ; +device_destroy (raw_class,MKDEV (RAW_MAJOR , number)); + }else { + rawdev->binding =bdget (dev); +if (rawdev->binding ==NULL ) { + err = -ENOMEM ; + }else { +dev_t raw =MKDEV (RAW_MAJOR , number); +__module_get (THIS_MODULE ); +device_destroy (raw_class, raw); +device_create (raw_class,NULL , raw,NULL , +"raw%d" , number); + } + } +mutex_unlock (&raw_mutex); +return err; +} + +static int bind_get (int number,dev_t *dev) +{ +struct raw_device_data *rawdev; +struct block_device *bdev; + +if (number <=0 || number >=MAX_RAW_MINORS ) +return -EINVAL ; + + rawdev = &raw_devices[number]; + +mutex_lock (&raw_mutex); + bdev = rawdev->binding; + *dev = bdev ? bdev->bd_dev :0 ; +mutex_unlock (&raw_mutex); +return 0 ; }/*
@@ -143,103 +214,79 @@ static int raw_ctl_ioctl(struct inode *inode, struct file *filp, unsigned int command, unsigned long arg) {struct raw_config_request rq; -struct raw_device_data *rawdev; -int err =0 ; +dev_t dev; +int err;switch (command) {case RAW_SETBIND : +if (copy_from_user (&rq, (void __user *) arg,sizeof (rq))) +return -EFAULT ; + +return bind_set (rq.raw_minor, rq.block_major, rq.block_minor); +case RAW_GETBIND : +if (copy_from_user (&rq, (void __user *) arg,sizeof (rq))) +return -EFAULT ; -/* First, find out which raw minor we want */ + err =bind_get (rq.raw_minor, &dev); +if (err) +return err; -if (copy_from_user (&rq, (void __user *) arg,sizeof (rq))) { - err = -EFAULT ; - goto out; - } + rq.block_major =MAJOR (dev); + rq.block_minor =MINOR (dev); -if (rq.raw_minor <=0 || rq.raw_minor >=MAX_RAW_MINORS ) { - err = -EINVAL ; - goto out; - } - rawdev = &raw_devices[rq.raw_minor]; - -if (command ==RAW_SETBIND ) { -dev_t dev; - -/* -* This is like making block devices, so demand the -* same capability -*/ -if (!capable (CAP_SYS_ADMIN )) { - err = -EPERM ; - goto out; - } - -/* -* For now, we don't need to check that the underlying -* block device is present or not: we can do that when -* the raw device is opened. Just check that the -* major/minor numbers make sense. -*/ - - dev =MKDEV (rq.block_major, rq.block_minor); -if ((rq.block_major ==0 && rq.block_minor !=0 ) || -MAJOR (dev) != rq.block_major || -MINOR (dev) != rq.block_minor) { - err = -EINVAL ; - goto out; - } - -mutex_lock (&raw_mutex); -if (rawdev->inuse) { -mutex_unlock (&raw_mutex); - err = -EBUSY ; - goto out; - } -if (rawdev->binding) { -bdput (rawdev->binding); -module_put (THIS_MODULE ); - } -if (rq.block_major ==0 && rq.block_minor ==0 ) { -/* unbind */ - rawdev->binding =NULL ; -device_destroy (raw_class, -MKDEV (RAW_MAJOR , rq.raw_minor)); - }else { - rawdev->binding =bdget (dev); -if (rawdev->binding ==NULL ) - err = -ENOMEM ; -else { -__module_get (THIS_MODULE ); -bind_device (&rq); - } - } -mutex_unlock (&raw_mutex); - }else { -struct block_device *bdev; - -mutex_lock (&raw_mutex); - bdev = rawdev->binding; -if (bdev) { - rq.block_major =MAJOR (bdev->bd_dev); - rq.block_minor =MINOR (bdev->bd_dev); - }else { - rq.block_major = rq.block_minor =0 ; - } -mutex_unlock (&raw_mutex); -if (copy_to_user ((void __user *)arg, &rq,sizeof (rq))) { - err = -EFAULT ; - goto out; - } - } -break ; -default : - err = -EINVAL ; -break ; +if (copy_to_user ((void __user *)arg, &rq,sizeof (rq))) +return -EFAULT ; + +return 0 ; } -out: -return err; + +return -EINVAL ; +} + +#ifdef CONFIG_COMPAT +struct raw32_config_request +{ +compat_int_t raw_minor; +compat_u64 block_major; +compat_u64 block_minor; +}; + +static long raw_ctl_compat_ioctl (struct file *file,unsigned int cmd, +unsigned long arg) +{ +struct raw32_config_request __user *user_req =compat_ptr (arg); +struct raw32_config_request rq; +dev_t dev; +int err =0 ; + +switch (cmd) { +case RAW_SETBIND : +if (copy_from_user (&rq, user_req,sizeof (rq))) +return -EFAULT ; + +return bind_set (rq.raw_minor, rq.block_major, rq.block_minor); + +case RAW_GETBIND : +if (copy_from_user (&rq, user_req,sizeof (rq))) +return -EFAULT ; + + err =bind_get (rq.raw_minor, &dev); +if (err) +return err; + + rq.block_major =MAJOR (dev); + rq.block_minor =MINOR (dev); + +if (copy_to_user (user_req, &rq,sizeof (rq))) +return -EFAULT ; + +return 0 ; + } + +return -EINVAL ; } +#endif static const struct file_operations raw_fops = { .read = do_sync_read,
@@ -254,6 +301,9 @@ static const struct file_operations raw_fops = {static const struct file_operations raw_ctl_fops = { .ioctl = raw_ctl_ioctl, +#ifdef CONFIG_COMPAT + .compat_ioctl = raw_ctl_compat_ioctl, +#endif .open = raw_open, .owner =THIS_MODULE , };
fs/compat_ioctl.c +0 -75
93 lines not on this page · raw message