usb: gadget: xudc: fix retrieve request logic
Hans Yang [Sat, 22 Oct 2016 05:58:36 +0000 (13:58 +0800)]
This commit fixes a wrong memory access issue.
When the gadget function driver dequeues a request which
may has been completed by HW, then at handling its transfer
event, retrieving its corresponding request pointer could
be incorrect. SW should check if the request queue
is empty and the request is the correct one to avoid incorrect
memory access.

Below shows the call trace for an incorrect memory access case
Unable to handle kernel paging request at virtual
address ff0000c0be320e0a
PC is at ep_matches+0x38/0x2b4
LR is at usb_ep_autoconfig_ss+0x148/0x180
[<ffffffc00078b50c>] ep_matches+0x38/0x2b4
[<ffffffc00078b8d0>] usb_ep_autoconfig_ss+0x148/0x180
[<ffffffc00078b918>] usb_ep_autoconfig+0x10/0x18
[<ffffffc0007a9c84>] __ffs_func_bind_do_descs+0x100/0x170
[<ffffffc0007aa94c>] ffs_do_descs+0x4c/0x13c
[<ffffffc0007b29b8>] ffs_func_bind+0x224/0x338
[<ffffffc00078bdc4>] usb_add_function+0x98/0x180
[<ffffffc0007b3a68>] functionfs_bind_config+0xd0/0x100
[<ffffffc0007b3ab0>] ffs_function_bind_config+0x18/0x20
[<ffffffc0007a94e8>] android_bind_config+0x44/0x94
[<ffffffc00078c40c>] usb_add_config+0x7c/0x2ac
[<ffffffc0007b3cf4>] android_enable.isra.53+0x44/0x78
[<ffffffc0007ba154>] ffs_ep0_write+0x5c0/0x7bc
[<ffffffc0001a11ec>] vfs_write+0xc0/0x17c
[<ffffffc0001a1a28>] SyS_write+0x94/0x164

Bug 1824860

Change-Id: If742a8b4f8686cfa147c085913cf065b54dd022b
Signed-off-by: Hans Yang <hansy@nvidia.com>
Reviewed-on: http://git-master/r/1243044
Reviewed-by: Mark Kuo <mkuo@nvidia.com>
GVS: Gerrit_Virtual_Submit
Reviewed-by: Vinayak Pane <vpane@nvidia.com>

drivers/usb/gadget/nvxxx_udc.c

index 4189013..9c6dc23 100644 (file)
@@ -3084,13 +3084,29 @@ bool is_request_dequeued(struct nv_udc_s *nvudc, struct nv_udc_ep *udc_ep,
        return status;
 }
 
+/*
+ * Determine if the given TRB is in the range [first trb, last trb] for the
+ * given request.
+ */
+static bool trb_in_request(struct nv_udc_request *req,
+                       struct transfer_trb_s *trb)
+{
+       if (trb >= req->first_trb && (trb <= req->last_trb ||
+                                     req->last_trb < req->first_trb))
+               return true;
+       if (trb < req->first_trb && trb <= req->last_trb &&
+           req->last_trb < req->first_trb)
+               return true;
+       return false;
+}
+
 int nvudc_handle_exfer_event(struct nv_udc_s *nvudc, struct event_trb_s *event)
 {
        u8 ep_index = XHCI_GETF(EVE_TRB_ENDPOINT_ID, event->eve_trb_dword3);
        struct nv_udc_ep *udc_ep_ptr = &nvudc->udc_ep[ep_index];
        struct ep_cx_s *p_ep_cx = nvudc->p_epcx + ep_index;
        u16 comp_code;
-       struct nv_udc_request *udc_req_ptr;
+       struct nv_udc_request *udc_req_ptr = NULL;
        bool trbs_dequeued = false;
        struct transfer_trb_s *p_trb;
        u64 trb_pt;
@@ -3103,8 +3119,14 @@ int nvudc_handle_exfer_event(struct nv_udc_s *nvudc, struct event_trb_s *event)
        trb_pt = (u64)event->trb_pointer_lo +
                ((u64)(event->trb_pointer_hi) << 32);
        p_trb = tran_trb_dma_to_virt(udc_ep_ptr, trb_pt);
-       udc_req_ptr = list_entry(udc_ep_ptr->queue.next,
-                               struct nv_udc_request, queue);
+
+       if (p_trb) {
+               udc_req_ptr = list_first_entry_or_null(&(udc_ep_ptr->queue),
+                                               struct nv_udc_request, queue);
+               if (udc_req_ptr && !trb_in_request(udc_req_ptr, p_trb))
+                       udc_req_ptr = NULL;
+       }
+
        comp_code = XHCI_GETF(EVE_TRB_COMPL_CODE, event->eve_trb_dword2);
 
 #ifdef NV_DISABLE_RCV_DET
@@ -3166,6 +3188,13 @@ int nvudc_handle_exfer_event(struct nv_udc_s *nvudc, struct event_trb_s *event)
                        trb_transfer_length = XHCI_GETF(EVE_TRB_TRAN_LEN,
                                                event->eve_trb_dword2);
 
+                       /* For coverity check, it shouldn't enter here */
+                       if (unlikely(!udc_req_ptr)) {
+                               dev_err(nvudc->dev, "%s short pkt event without valid request\n",
+                                               __func__);
+                               break;
+                       }
+
                        udc_req_ptr->usb_req.actual =
                                udc_req_ptr->usb_req.length -
                                trb_transfer_length;