Re: [PATCH] usb: gadget: f_hid: do not copy_from_user() under a spinlock

>>>>> "Linkai" == Linkai Gong <[email protected]> writes:

 > f_hidg_get_report() already copied the report from userspace into a
 > new entry, then called copy_from_user() again under
 > get_report_spinlock. That can fault and sleep in atomic context.

 > Update the existing entry with memcpy() from the copy already taken.

 > Fixes: a139c98f760e ("USB: gadget: f_hid: Add GET_REPORT via userspace IOCTL")
 > Cc: [email protected]
 > Signed-off-by: Linkai Gong <[email protected]>

Acked-by: Peter Korsgaard <[email protected]>

> ---
 >  drivers/usb/gadget/function/f_hid.c | 11 +++--------
 >  1 file changed, 3 insertions(+), 8 deletions(-)

 > diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
 > index 3c6b43d06a6d..e4621e5a0b69 100644
 > --- a/drivers/usb/gadget/function/f_hid.c
 > +++ b/drivers/usb/gadget/function/f_hid.c
 > @@ -667,14 +667,9 @@ static int f_hidg_get_report(struct file *file, struct usb_hidg_report __user *b
 >  	ptr = f_hidg_search_for_report(hidg, report_id);
 
 >  	if (ptr) {
 > -		/* Report already exists in list - update it */
 > -		if (copy_from_user(&ptr->report_data, buffer,
 > -				sizeof(struct usb_hidg_report))) {
 > -			spin_unlock_irqrestore(&hidg->get_report_spinlock, flags);
 > -			ERROR(cdev, "copy_from_user error\n");
 > -			kfree(entry);
 > -			return -EINVAL;
 > -		}
 > +		/* Report already exists; data was copied before taking the lock. */
 > +		memcpy(&ptr->report_data, &entry->report_data,
 > +		       sizeof(struct usb_hidg_report));
 >  		kfree(entry);
 >  	} else {
 >  		/* Report does not exist in list - add it */
 > -- 

 > 2.25.1


-- 
Bye, Peter Korsgaard