Patch "HID: roccat: Fix use-after-free in roccat_read()" has been added to the 5.19-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    HID: roccat: Fix use-after-free in roccat_read()

to the 5.19-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     hid-roccat-fix-use-after-free-in-roccat_read.patch
and it can be found in the queue-5.19 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit 368b4e3f9f29dee8d6124e101975bd2265824c1f
Author: Hyunwoo Kim <imv4bel@xxxxxxxxx>
Date:   Sun Sep 4 12:31:15 2022 -0700

    HID: roccat: Fix use-after-free in roccat_read()
    
    [ 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@xxxxxxxxx>
    Signed-off-by: Jiri Kosina <jkosina@xxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
index 26373b82fe81..6da80e442fdd 100644
--- a/drivers/hid/hid-roccat.c
+++ b/drivers/hid/hid-roccat.c
@@ -257,6 +257,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 */
@@ -276,6 +278,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;
 }



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux