Commit d4f98dbf authored by Dan Carpenter's avatar Dan Carpenter Committed by Jiri Kosina

HID: roccat: add bounds checking in kone_sysfs_write_settings()

This code doesn't check if "settings->startup_profile" is within bounds
and that could result in an out of bounds array access.  What the code
does do is it checks if the settings can be written to the firmware, so
it's possible that the firmware has a bounds check?  It's safer and
easier to verify when the bounds checking is done in the kernel.

Fixes: 14bf62cd ("HID: add driver for Roccat Kone gaming mouse")
Signed-off-by: default avatarDan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: default avatarJiri Kosina <jkosina@suse.cz>
parent 35556bed
...@@ -294,31 +294,40 @@ static ssize_t kone_sysfs_write_settings(struct file *fp, struct kobject *kobj, ...@@ -294,31 +294,40 @@ static ssize_t kone_sysfs_write_settings(struct file *fp, struct kobject *kobj,
struct kone_device *kone = hid_get_drvdata(dev_get_drvdata(dev)); struct kone_device *kone = hid_get_drvdata(dev_get_drvdata(dev));
struct usb_device *usb_dev = interface_to_usbdev(to_usb_interface(dev)); struct usb_device *usb_dev = interface_to_usbdev(to_usb_interface(dev));
int retval = 0, difference, old_profile; int retval = 0, difference, old_profile;
struct kone_settings *settings = (struct kone_settings *)buf;
/* I need to get my data in one piece */ /* I need to get my data in one piece */
if (off != 0 || count != sizeof(struct kone_settings)) if (off != 0 || count != sizeof(struct kone_settings))
return -EINVAL; return -EINVAL;
mutex_lock(&kone->kone_lock); mutex_lock(&kone->kone_lock);
difference = memcmp(buf, &kone->settings, sizeof(struct kone_settings)); difference = memcmp(settings, &kone->settings,
sizeof(struct kone_settings));
if (difference) { if (difference) {
retval = kone_set_settings(usb_dev, if (settings->startup_profile < 1 ||
(struct kone_settings const *)buf); settings->startup_profile > 5) {
if (retval) { retval = -EINVAL;
mutex_unlock(&kone->kone_lock); goto unlock;
return retval;
} }
retval = kone_set_settings(usb_dev, settings);
if (retval)
goto unlock;
old_profile = kone->settings.startup_profile; old_profile = kone->settings.startup_profile;
memcpy(&kone->settings, buf, sizeof(struct kone_settings)); memcpy(&kone->settings, settings, sizeof(struct kone_settings));
kone_profile_activated(kone, kone->settings.startup_profile); kone_profile_activated(kone, kone->settings.startup_profile);
if (kone->settings.startup_profile != old_profile) if (kone->settings.startup_profile != old_profile)
kone_profile_report(kone, kone->settings.startup_profile); kone_profile_report(kone, kone->settings.startup_profile);
} }
unlock:
mutex_unlock(&kone->kone_lock); mutex_unlock(&kone->kone_lock);
if (retval)
return retval;
return sizeof(struct kone_settings); return sizeof(struct kone_settings);
} }
static BIN_ATTR(settings, 0660, kone_sysfs_read_settings, static BIN_ATTR(settings, 0660, kone_sysfs_read_settings,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment