| Message ID | 20231205085404.175-2-aladyshev22@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: gadget: f_hid: fix report descriptor allocation | expand |
On Tue, Dec 05, 2023 at 11:54:03AM +0300, Konstantin Aladyshev wrote: > The commit "usb: gadget: f_hid: fix f_hidg lifetime vs cdev" > (89ff3dfac604614287ad5aad9370c3f984ea3f4b) has introduced a bug > that leads to hid device corruption after the replug operation. Nit, this should be written as 89ff3dfac604 ("usb: gadget: f_hid: fix f_hidg lifetime vs cdev") right? > Reverse device managed memory allocation for the report descriptor > to fix the issue. > > Tested: > This change was tested on the AMD EthanolX CRB server with the BMC > based on the OpenBMC distribution. The BMC provides KVM functionality > via the USB gadget device: > - before: KVM page refresh results in a broken USB device, > - after: KVM page refresh works without any issues. > > Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com> We need a Fixes: tag here and also a cc: stable so that this gets properly backported. > --- > drivers/usb/gadget/function/f_hid.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c > index ea85e2c701a1..3c8a9dd585c0 100644 > --- a/drivers/usb/gadget/function/f_hid.c > +++ b/drivers/usb/gadget/function/f_hid.c > @@ -92,6 +92,7 @@ static void hidg_release(struct device *dev) > { > struct f_hidg *hidg = container_of(dev, struct f_hidg, dev); > > + kfree(hidg->report_desc); > kfree(hidg->set_report_buf); > kfree(hidg); > } > @@ -1287,9 +1288,9 @@ static struct usb_function *hidg_alloc(struct usb_function_instance *fi) > hidg->report_length = opts->report_length; > hidg->report_desc_length = opts->report_desc_length; > if (opts->report_desc) { > - hidg->report_desc = devm_kmemdup(&hidg->dev, opts->report_desc, > - opts->report_desc_length, > - GFP_KERNEL); > + hidg->report_desc = kmemdup(opts->report_desc, > + opts->report_desc_length, > + GFP_KERNEL); Yet one more reason why devm_*() functions are dangerous to use :( Nice fix. thanks, greg k-h
diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c index ea85e2c701a1..3c8a9dd585c0 100644 --- a/drivers/usb/gadget/function/f_hid.c +++ b/drivers/usb/gadget/function/f_hid.c @@ -92,6 +92,7 @@ static void hidg_release(struct device *dev) { struct f_hidg *hidg = container_of(dev, struct f_hidg, dev); + kfree(hidg->report_desc); kfree(hidg->set_report_buf); kfree(hidg); } @@ -1287,9 +1288,9 @@ static struct usb_function *hidg_alloc(struct usb_function_instance *fi) hidg->report_length = opts->report_length; hidg->report_desc_length = opts->report_desc_length; if (opts->report_desc) { - hidg->report_desc = devm_kmemdup(&hidg->dev, opts->report_desc, - opts->report_desc_length, - GFP_KERNEL); + hidg->report_desc = kmemdup(opts->report_desc, + opts->report_desc_length, + GFP_KERNEL); if (!hidg->report_desc) { ret = -ENOMEM; goto err_put_device;
The commit "usb: gadget: f_hid: fix f_hidg lifetime vs cdev" (89ff3dfac604614287ad5aad9370c3f984ea3f4b) has introduced a bug that leads to hid device corruption after the replug operation. Reverse device managed memory allocation for the report descriptor to fix the issue. Tested: This change was tested on the AMD EthanolX CRB server with the BMC based on the OpenBMC distribution. The BMC provides KVM functionality via the USB gadget device: - before: KVM page refresh results in a broken USB device, - after: KVM page refresh works without any issues. Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com> --- drivers/usb/gadget/function/f_hid.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)