Re: [PATCH] usb: gadget: f_hid: fixed NULL pointer dereference
From: Maxim Devaev <hidden>
Date: 2021-07-23 15:22:15
Subsystem:
the rest, usb subsystem · Maintainers:
Linus Torvalds, Greg Kroah-Hartman
Greg KH [off-list ref] writes: Who is the original author here? Please put their name as the "From:" line in the changelog so we can give proper credit if it was not you.
Phil Elwell. Fixed.
Test for !req and then abort, and then continue on. No need for moving the copy_from_user line in, right? Should make the change smaller overall.
Fixed.
Blank line after this please.
Added.
Are the braces still needed here?
No, you're right. Thanks for the review! Here the new version of the patch: From 72a504af8e3df3d7a44e9954b8cf03268795c07f Mon Sep 17 00:00:00 2001 From: Phil Elwell <redacted> Date: Fri, 23 Jul 2021 11:59:49 +0300 Subject: [PATCH] usb: gadget: f_hid: fixed NULL pointer dereference Disconnecting and reconnecting the USB cable can lead to crashes and a variety of kernel log spam. The problem was found and reproduced on the Raspberry Pi [1] and the original fix was created in Raspberry's own fork [2]. Signed-off-by: Maxim Devaev <redacted> Signed-off-by: Phil Elwell <redacted> Link: https://github.com/raspberrypi/linux/issues/3870 [1] Link: https://github.com/raspberrypi/linux/commit/a6e47d5f4efbd2ea6a0b6565cd2f9b7bb217ded5 [2] --- drivers/usb/gadget/function/f_hid.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
index 02683ac07..08e73e812 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c@@ -338,6 +338,11 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer, spin_lock_irqsave(&hidg->write_spinlock, flags); + if (!hidg->req) { + spin_unlock_irqrestore(&hidg->write_spinlock, flags); + return -ESHUTDOWN; + } + #define WRITE_COND (!hidg->write_pending) try_again: /* write queue */
@@ -358,8 +363,14 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer, count = min_t(unsigned, count, hidg->report_length); spin_unlock_irqrestore(&hidg->write_spinlock, flags); - status = copy_from_user(req->buf, buffer, count); + if (!req) { + ERROR(hidg->func.config->cdev, "hidg->req is NULL\n"); + status = -ESHUTDOWN; + goto release_write_pending; + } + + status = copy_from_user(req->buf, buffer, count); if (status != 0) { ERROR(hidg->func.config->cdev, "copy_from_user error\n");
@@ -387,14 +398,17 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer, spin_unlock_irqrestore(&hidg->write_spinlock, flags); + if (!hidg->in_ep->enabled) { + ERROR(hidg->func.config->cdev, "in_ep is disabled\n"); + status = -ESHUTDOWN; + goto release_write_pending; + } + status = usb_ep_queue(hidg->in_ep, req, GFP_ATOMIC); - if (status < 0) { - ERROR(hidg->func.config->cdev, - "usb_ep_queue error on int endpoint %zd\n", status); + if (status < 0) goto release_write_pending; - } else { + else status = count; - } return status; release_write_pending:
--
2.32.0