Skip to content

Commit 75d5546

Browse files
Benjamin TissoiresJiri Kosina
Benjamin Tissoires
authored and
Jiri Kosina
committed
HID: hidraw: tighten ioctl command parsing
The handling for variable-length ioctl commands in hidraw_ioctl() is rather complex and the check for the data direction is incomplete. Simplify this code by factoring out the various ioctls grouped by dir and size, and using a switch() statement with the size masked out, to ensure the rest of the command is correctly matched. Fixes: 9188e79 ("HID: add phys and name ioctls to hidraw") Reported-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org> Signed-off-by: Jiri Kosina <jkosina@suse.com>
1 parent 8c62074 commit 75d5546

2 files changed

Lines changed: 124 additions & 102 deletions

File tree

‎drivers/hid/hidraw.c‎

Lines changed: 122 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -394,141 +394,161 @@ static int hidraw_revoke(struct hidraw_list *list)
394394
return 0;
395395
}
396396

397-
static long hidraw_ioctl(struct file *file, unsigned int cmd,
398-
unsigned long arg)
397+
static long hidraw_fixed_size_ioctl(struct file *file, struct hidraw *dev, unsigned int cmd,
398+
void __user *arg)
399399
{
400-
struct inode *inode = file_inode(file);
401-
unsigned int minor = iminor(inode);
402-
long ret = 0;
403-
struct hidraw *dev;
404-
struct hidraw_list *list = file->private_data;
405-
void __user *user_arg = (void __user*) arg;
406-
407-
down_read(&minors_rwsem);
408-
dev = hidraw_table[minor];
409-
if (!dev || !dev->exist || hidraw_is_revoked(list)) {
410-
ret = -ENODEV;
411-
goto out;
412-
}
400+
struct hid_device *hid = dev->hid;
413401

414402
switch (cmd) {
415403
case HIDIOCGRDESCSIZE:
416-
if (put_user(dev->hid->rsize, (int __user *)arg))
417-
ret = -EFAULT;
404+
if (put_user(hid->rsize, (int __user *)arg))
405+
return -EFAULT;
418406
break;
419407

420408
case HIDIOCGRDESC:
421409
{
422410
__u32 len;
423411

424412
if (get_user(len, (int __user *)arg))
425-
ret = -EFAULT;
426-
else if (len > HID_MAX_DESCRIPTOR_SIZE - 1)
427-
ret = -EINVAL;
428-
else if (copy_to_user(user_arg + offsetof(
429-
struct hidraw_report_descriptor,
430-
value[0]),
431-
dev->hid->rdesc,
432-
min(dev->hid->rsize, len)))
433-
ret = -EFAULT;
413+
return -EFAULT;
414+
415+
if (len > HID_MAX_DESCRIPTOR_SIZE - 1)
416+
return -EINVAL;
417+
418+
if (copy_to_user(arg + offsetof(
419+
struct hidraw_report_descriptor,
420+
value[0]),
421+
hid->rdesc,
422+
min(hid->rsize, len)))
423+
return -EFAULT;
424+
434425
break;
435426
}
436427
case HIDIOCGRAWINFO:
437428
{
438429
struct hidraw_devinfo dinfo;
439430

440-
dinfo.bustype = dev->hid->bus;
441-
dinfo.vendor = dev->hid->vendor;
442-
dinfo.product = dev->hid->product;
443-
if (copy_to_user(user_arg, &dinfo, sizeof(dinfo)))
444-
ret = -EFAULT;
431+
dinfo.bustype = hid->bus;
432+
dinfo.vendor = hid->vendor;
433+
dinfo.product = hid->product;
434+
if (copy_to_user(arg, &dinfo, sizeof(dinfo)))
435+
return -EFAULT;
445436
break;
446437
}
447438
case HIDIOCREVOKE:
448439
{
449-
if (user_arg)
450-
ret = -EINVAL;
451-
else
452-
ret = hidraw_revoke(list);
453-
break;
440+
struct hidraw_list *list = file->private_data;
441+
442+
if (arg)
443+
return -EINVAL;
444+
445+
return hidraw_revoke(list);
454446
}
455447
default:
456-
{
457-
struct hid_device *hid = dev->hid;
458-
if (_IOC_TYPE(cmd) != 'H') {
459-
ret = -EINVAL;
460-
break;
461-
}
448+
/*
449+
* None of the above ioctls can return -EAGAIN, so
450+
* use it as a marker that we need to check variable
451+
* length ioctls.
452+
*/
453+
return -EAGAIN;
454+
}
462455

463-
if (_IOC_NR(cmd) == _IOC_NR(HIDIOCSFEATURE(0))) {
464-
int len = _IOC_SIZE(cmd);
465-
ret = hidraw_send_report(file, user_arg, len, HID_FEATURE_REPORT);
466-
break;
467-
}
468-
if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGFEATURE(0))) {
469-
int len = _IOC_SIZE(cmd);
470-
ret = hidraw_get_report(file, user_arg, len, HID_FEATURE_REPORT);
471-
break;
472-
}
456+
return 0;
457+
}
473458

474-
if (_IOC_NR(cmd) == _IOC_NR(HIDIOCSINPUT(0))) {
475-
int len = _IOC_SIZE(cmd);
476-
ret = hidraw_send_report(file, user_arg, len, HID_INPUT_REPORT);
477-
break;
478-
}
479-
if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGINPUT(0))) {
480-
int len = _IOC_SIZE(cmd);
481-
ret = hidraw_get_report(file, user_arg, len, HID_INPUT_REPORT);
482-
break;
483-
}
459+
static long hidraw_rw_variable_size_ioctl(struct file *file, struct hidraw *dev, unsigned int cmd,
460+
void __user *user_arg)
461+
{
462+
int len = _IOC_SIZE(cmd);
463+
464+
switch (cmd & ~IOCSIZE_MASK) {
465+
case HIDIOCSFEATURE(0):
466+
return hidraw_send_report(file, user_arg, len, HID_FEATURE_REPORT);
467+
case HIDIOCGFEATURE(0):
468+
return hidraw_get_report(file, user_arg, len, HID_FEATURE_REPORT);
469+
case HIDIOCSINPUT(0):
470+
return hidraw_send_report(file, user_arg, len, HID_INPUT_REPORT);
471+
case HIDIOCGINPUT(0):
472+
return hidraw_get_report(file, user_arg, len, HID_INPUT_REPORT);
473+
case HIDIOCSOUTPUT(0):
474+
return hidraw_send_report(file, user_arg, len, HID_OUTPUT_REPORT);
475+
case HIDIOCGOUTPUT(0):
476+
return hidraw_get_report(file, user_arg, len, HID_OUTPUT_REPORT);
477+
}
484478

485-
if (_IOC_NR(cmd) == _IOC_NR(HIDIOCSOUTPUT(0))) {
486-
int len = _IOC_SIZE(cmd);
487-
ret = hidraw_send_report(file, user_arg, len, HID_OUTPUT_REPORT);
488-
break;
489-
}
490-
if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGOUTPUT(0))) {
491-
int len = _IOC_SIZE(cmd);
492-
ret = hidraw_get_report(file, user_arg, len, HID_OUTPUT_REPORT);
493-
break;
494-
}
479+
return -EINVAL;
480+
}
495481

496-
/* Begin Read-only ioctls. */
497-
if (_IOC_DIR(cmd) != _IOC_READ) {
498-
ret = -EINVAL;
499-
break;
500-
}
482+
static long hidraw_ro_variable_size_ioctl(struct file *file, struct hidraw *dev, unsigned int cmd,
483+
void __user *user_arg)
484+
{
485+
struct hid_device *hid = dev->hid;
486+
int len = _IOC_SIZE(cmd);
487+
int field_len;
488+
489+
switch (cmd & ~IOCSIZE_MASK) {
490+
case HIDIOCGRAWNAME(0):
491+
field_len = strlen(hid->name) + 1;
492+
if (len > field_len)
493+
len = field_len;
494+
return copy_to_user(user_arg, hid->name, len) ? -EFAULT : len;
495+
case HIDIOCGRAWPHYS(0):
496+
field_len = strlen(hid->phys) + 1;
497+
if (len > field_len)
498+
len = field_len;
499+
return copy_to_user(user_arg, hid->phys, len) ? -EFAULT : len;
500+
case HIDIOCGRAWUNIQ(0):
501+
field_len = strlen(hid->uniq) + 1;
502+
if (len > field_len)
503+
len = field_len;
504+
return copy_to_user(user_arg, hid->uniq, len) ? -EFAULT : len;
505+
}
501506

502-
if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGRAWNAME(0))) {
503-
int len = strlen(hid->name) + 1;
504-
if (len > _IOC_SIZE(cmd))
505-
len = _IOC_SIZE(cmd);
506-
ret = copy_to_user(user_arg, hid->name, len) ?
507-
-EFAULT : len;
508-
break;
509-
}
507+
return -EINVAL;
508+
}
510509

511-
if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGRAWPHYS(0))) {
512-
int len = strlen(hid->phys) + 1;
513-
if (len > _IOC_SIZE(cmd))
514-
len = _IOC_SIZE(cmd);
515-
ret = copy_to_user(user_arg, hid->phys, len) ?
516-
-EFAULT : len;
517-
break;
518-
}
510+
static long hidraw_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
511+
{
512+
struct inode *inode = file_inode(file);
513+
unsigned int minor = iminor(inode);
514+
struct hidraw *dev;
515+
struct hidraw_list *list = file->private_data;
516+
void __user *user_arg = (void __user *)arg;
517+
int ret;
519518

520-
if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGRAWUNIQ(0))) {
521-
int len = strlen(hid->uniq) + 1;
522-
if (len > _IOC_SIZE(cmd))
523-
len = _IOC_SIZE(cmd);
524-
ret = copy_to_user(user_arg, hid->uniq, len) ?
525-
-EFAULT : len;
526-
break;
527-
}
528-
}
519+
down_read(&minors_rwsem);
520+
dev = hidraw_table[minor];
521+
if (!dev || !dev->exist || hidraw_is_revoked(list)) {
522+
ret = -ENODEV;
523+
goto out;
524+
}
525+
526+
if (_IOC_TYPE(cmd) != 'H') {
527+
ret = -EINVAL;
528+
goto out;
529+
}
529530

531+
if (_IOC_NR(cmd) > HIDIOCTL_LAST || _IOC_NR(cmd) == 0) {
530532
ret = -ENOTTY;
533+
goto out;
531534
}
535+
536+
ret = hidraw_fixed_size_ioctl(file, dev, cmd, user_arg);
537+
if (ret != -EAGAIN)
538+
goto out;
539+
540+
switch (_IOC_DIR(cmd)) {
541+
case (_IOC_READ | _IOC_WRITE):
542+
ret = hidraw_rw_variable_size_ioctl(file, dev, cmd, user_arg);
543+
break;
544+
case _IOC_READ:
545+
ret = hidraw_ro_variable_size_ioctl(file, dev, cmd, user_arg);
546+
break;
547+
default:
548+
/* Any other IOC_DIR is wrong */
549+
ret = -EINVAL;
550+
}
551+
532552
out:
533553
up_read(&minors_rwsem);
534554
return ret;

‎include/uapi/linux/hidraw.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ struct hidraw_devinfo {
4848
#define HIDIOCGOUTPUT(len) _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x0C, len)
4949
#define HIDIOCREVOKE _IOW('H', 0x0D, int) /* Revoke device access */
5050

51+
#define HIDIOCTL_LAST _IOC_NR(HIDIOCREVOKE)
52+
5153
#define HIDRAW_FIRST_MINOR 0
5254
#define HIDRAW_MAX_DEVICES 64
5355
/* number of reports to buffer */

0 commit comments

Comments
 (0)