diff mbox series

AW: [PATCH v2] HID: roccat: add bounds checking in kone_sysfs_write_settings()

Message ID ab4625b2b2ea41dd83ff9e192a027f41@bfs.de
State New
Headers show
Series AW: [PATCH v2] HID: roccat: add bounds checking in kone_sysfs_write_settings() | expand

Commit Message

Walter Harms Aug. 24, 2020, 3:35 p.m. UTC 
hello Dan, 

i notice that you can shorten the line to:
(line above checks for count==sizeof(struct kone_settings))

difference = memcmp(settings, &kone->settings, count);

nothing special just to shorten the line and make use of count.

and just to save one indent level and because its  readabel nicely:
    if ( ! difference ) 
          goto unlock;

hope that helps

re,
 wh
diff mbox series

Patch

diff --git a/drivers/hid/hid-roccat-kone.c b/drivers/hid/hid-roccat-kone.c
index 2ff4c8e366ff..1ca64481145e 100644
--- a/drivers/hid/hid-roccat-kone.c
+++ b/drivers/hid/hid-roccat-kone.c
@@ -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,