Hi, OGAWA Hirofumi <hirof...@mail.parknet.co.jp> writes: > If race with timeout or stop event on empty cmd_list was happened, > handle_cmd_completion() deferences wrong pointer from xhci->cmd_list. > > So this makes sure we don't dereference wrong pointer from empty > xhci->cmd_list. > > Signed-off-by: OGAWA Hirofumi <hirof...@mail.parknet.co.jp> > --- > > drivers/usb/host/xhci-ring.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff -puN drivers/usb/host/xhci-ring.c~xhci-fix-empty-list > drivers/usb/host/xhci-ring.c > --- linux/drivers/usb/host/xhci-ring.c~xhci-fix-empty-list 2016-10-18 > 21:20:07.973055018 +0900 > +++ linux-hirofumi/drivers/usb/host/xhci-ring.c 2016-10-18 > 21:21:35.949060365 +0900 > @@ -1336,7 +1336,12 @@ static void handle_cmd_completion(struct > return; > } > > - cmd = list_entry(xhci->cmd_list.next, struct xhci_command, cmd_list);
> + if (list_empty(&xhci->cmd_list))
> + cmd = NULL;
> + else {
> + cmd = list_first_entry(&xhci->cmd_list, struct xhci_command,
> + cmd_list);
> + }
These few lines could become a nice helper. Something like:
static inline struct xhci_command *next_command(struct xhci_hcd *xhci)
{
if (list_empty(&xhci->cmd_list))
return NULL;
return list_first_entry(&xhci->cmd_list, struct xhci_command, cmd_list);
}
> @@ -1350,9 +1355,10 @@ static void handle_cmd_completion(struct
Before the lines below, there's a call to del_timer() and a
tracepoint. I have a feeling we don't want those to happen if the list
is corrupt. Perhaps you should skip del_timer() and adding a log to
trace buffer, no?
> return;
> }
>
> - if (cmd->command_trb != xhci->cmd_ring->dequeue) {
> + if (cmd == NULL || cmd->command_trb != xhci->cmd_ring->dequeue) {
> xhci_err(xhci,
> - "Command completion event does not match command\n");
> + "Command completion event %s\n",
> + cmd ? "does not match command" : "on empty list");
> return;
> }
--
balbi
signature.asc
Description: PGP signature
