lkml.org 
[lkml]   [2022]   [Oct]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.19 213/229] HID: roccat: Fix use-after-free in roccat_read()
    Date
    From: Hyunwoo Kim <imv4bel@gmail.com>

    [ Upstream commit cacdb14b1c8d3804a3a7d31773bc7569837b71a4 ]

    roccat_report_event() is responsible for registering
    roccat-related reports in struct roccat_device.

    int roccat_report_event(int minor, u8 const *data)
    {
    struct roccat_device *device;
    struct roccat_reader *reader;
    struct roccat_report *report;
    uint8_t *new_value;

    device = devices[minor];

    new_value = kmemdup(data, device->report_size, GFP_ATOMIC);
    if (!new_value)
    return -ENOMEM;

    report = &device->cbuf[device->cbuf_end];

    /* passing NULL is safe */
    kfree(report->value);
    ...

    The registered report is stored in the struct roccat_device member
    "struct roccat_report cbuf[ROCCAT_CBUF_SIZE];".
    If more reports are received than the "ROCCAT_CBUF_SIZE" value,
    kfree() the saved report from cbuf[0] and allocates a new reprot.
    Since there is no lock when this kfree() is performed,
    kfree() can be performed even while reading the saved report.

    static ssize_t roccat_read(struct file *file, char __user *buffer,
    size_t count, loff_t *ppos)
    {
    struct roccat_reader *reader = file->private_data;
    struct roccat_device *device = reader->device;
    struct roccat_report *report;
    ssize_t retval = 0, len;
    DECLARE_WAITQUEUE(wait, current);

    mutex_lock(&device->cbuf_lock);

    ...

    report = &device->cbuf[reader->cbuf_start];
    /*
    * If report is larger than requested amount of data, rest of report
    * is lost!
    */
    len = device->report_size > count ? count : device->report_size;

    if (copy_to_user(buffer, report->value, len)) {
    retval = -EFAULT;
    goto exit_unlock;
    }
    ...

    The roccat_read() function receives the device->cbuf report and
    delivers it to the user through copy_to_user().
    If the N+ROCCAT_CBUF_SIZE th report is received while copying of
    the Nth report->value is in progress, the pointer that copy_to_user()
    is working on is kfree()ed and UAF read may occur. (race condition)

    Since the device node of this driver does not set separate permissions,
    this is not a security vulnerability, but because it is used for
    requesting screen display of profile or dpi settings,
    a user using the roccat device can apply udev to this device node or
    There is a possibility to use it by giving.

    Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
    Signed-off-by: Jiri Kosina <jkosina@suse.cz>
    Signed-off-by: Sasha Levin <sashal@kernel.org>
    ---
    drivers/hid/hid-roccat.c | 4 ++++
    1 file changed, 4 insertions(+)

    diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
    index 5be8de70c651..c9cec00b4e6e 100644
    --- a/drivers/hid/hid-roccat.c
    +++ b/drivers/hid/hid-roccat.c
    @@ -260,6 +260,8 @@ int roccat_report_event(int minor, u8 const *data)
    if (!new_value)
    return -ENOMEM;

    + mutex_lock(&device->cbuf_lock);
    +
    report = &device->cbuf[device->cbuf_end];

    /* passing NULL is safe */
    @@ -279,6 +281,8 @@ int roccat_report_event(int minor, u8 const *data)
    reader->cbuf_start = (reader->cbuf_start + 1) % ROCCAT_CBUF_SIZE;
    }

    + mutex_unlock(&device->cbuf_lock);
    +
    wake_up_interruptible(&device->wait);
    return 0;
    }
    --
    2.35.1


    \
     
     \ /
      Last update: 2022-10-24 14:40    [W:2.070 / U:0.052 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site