Hi,
OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx> 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 <hirofumi@xxxxxxxxxxxxxxxxxx>
> ---
>
> 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
Attachment:
signature.asc
Description: PGP signature
