lkml.org 
[lkml]   [2020]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.4 042/112] HID: roccat: add bounds checking in kone_sysfs_write_settings()
    Date
    From: Dan Carpenter <dan.carpenter@oracle.com>

    [ Upstream commit d4f98dbfe717490e771b6e701904bfcf4b4557f0 ]

    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: 14bf62cde794 ("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>
    ---
    drivers/hid/hid-roccat-kone.c | 23 ++++++++++++++++-------
    1 file changed, 16 insertions(+), 7 deletions(-)

    diff --git a/drivers/hid/hid-roccat-kone.c b/drivers/hid/hid-roccat-kone.c
    index c29265055ac1a..6c2b821c8d8b5 100644
    --- a/drivers/hid/hid-roccat-kone.c
    +++ b/drivers/hid/hid-roccat-kone.c
    @@ -299,31 +299,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,
    --
    2.25.1


    \
     
     \ /
      Last update: 2020-10-27 19:27    [W:4.034 / U:0.076 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site