Skip to content

Commit

Permalink
HID: roccat: add bounds checking in kone_sysfs_write_settings()
Browse files Browse the repository at this point in the history
[ Upstream commit d4f98db ]

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: 14bf62c ("HID: add driver for Roccat Kone gaming mouse")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
Dan Carpenter authored and gregkh committed Oct 29, 2020
1 parent 4d86178 commit 02bf8fb
Showing 1 changed file with 16 additions and 7 deletions.
23 changes: 16 additions & 7 deletions drivers/hid/hid-roccat-kone.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 usb_device *usb_dev = interface_to_usbdev(to_usb_interface(dev));
int retval = 0, difference, old_profile;
struct kone_settings *settings = (struct kone_settings *)buf;

/* I need to get my data in one piece */
if (off != 0 || count != sizeof(struct kone_settings))
return -EINVAL;

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) {
retval = kone_set_settings(usb_dev,
(struct kone_settings const *)buf);
if (retval) {
mutex_unlock(&kone->kone_lock);
return retval;
if (settings->startup_profile < 1 ||
settings->startup_profile > 5) {
retval = -EINVAL;
goto unlock;
}

retval = kone_set_settings(usb_dev, settings);
if (retval)
goto unlock;

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);

if (kone->settings.startup_profile != old_profile)
kone_profile_report(kone, kone->settings.startup_profile);
}
unlock:
mutex_unlock(&kone->kone_lock);

if (retval)
return retval;

return sizeof(struct kone_settings);
}
static BIN_ATTR(settings, 0660, kone_sysfs_read_settings,
Expand Down

0 comments on commit 02bf8fb

Please sign in to comment.