Hi Dan,
On 1/24/23 10:23 AM, Dan Carpenter wrote:
Hello Udipto Goswami,
The patch 6a19da111057: "usb: gadget: f_fs: Prevent race during
ffs_ep0_queue_wait" from Dec 15, 2022, leads to the following Smatch
static checker warning:
drivers/usb/gadget/function/f_fs.c:313 __ffs_ep0_queue_wait()
warn: inconsistent returns '&ffs->ev.waitq.lock'.
drivers/usb/gadget/function/f_fs.c
276 static int __ffs_ep0_queue_wait(struct ffs_data *ffs, char *data, size_t len)
277 __releases(&ffs->ev.waitq.lock)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
278 {
279 struct usb_request *req = ffs->ep0req;
280 int ret;
281
282 if (!req)
283 return -EINVAL;
^^^^^^^^^^^^^^^
Drop the lock before returning?
Thanks for pointing this out.
I assumed the caller was handling this however its not the case. will
fix it like this :
@@ -279,8 +279,10 @@ static int __ffs_ep0_queue_wait(struct ffs_data
*ffs, char *data, size_t len)
struct usb_request *req = ffs->ep0req;
int ret;
- if (!req)
+ if (!req) {
+ spin_unlock_irq(&ffs->ev.waitq.lock);
return -EINVAL;
+ }
req->zero = len < le16_to_cpu(ffs->ev.setup.wLength);
284
285 req->zero = len < le16_to_cpu(ffs->ev.setup.wLength);
286
287 spin_unlock_irq(&ffs->ev.waitq.lock);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
288
289 req->buf = data;
290 req->length = len;
291
292 /*
293 * UDC layer requires to provide a buffer even for ZLP, but should
294 * not use it at all. Let's provide some poisoned pointer to catch
295 * possible bug in the driver.
296 */
297 if (req->buf == NULL)
298 req->buf = (void *)0xDEADBABE;
299
300 reinit_completion(&ffs->ep0req_completion);
301
302 ret = usb_ep_queue(ffs->gadget->ep0, req, GFP_ATOMIC);
303 if (ret < 0)
304 return ret;
305
306 ret = wait_for_completion_interruptible(&ffs->ep0req_completion);
307 if (ret) {
308 usb_ep_dequeue(ffs->gadget->ep0, req);
309 return -EINTR;
310 }
311
312 ffs->setup_state = FFS_NO_SETUP;
--> 313 return req->status ? req->status : req->actual;
314 }
regards,
dan carpenter
Thanks,
-Udipto
