linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Michal Pecio <michal.pecio@gmail.com>
To: Mathias Nyman <mathias.nyman@intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 0/1]  usb: xhci: Fix NULL pointer dereference on certain command aborts
Date: Tue, 3 Dec 2024 20:51:23 +0100	[thread overview]
Message-ID: <20241203205123.05b32413@foxbook> (raw)

Hi,

This fixes an issue discovered by code review prompted by this crash:
https://bugzilla.kernel.org/show_bug.cgi?id=219532

It has not been confirmed by the reporter and doesn't agree with some
of his own analysis, but:

1. it is consistent with logs provided by the reporter
2. it looks like a real bug in its own right anyway
3. it is rare enough that it may not happen again soon


I tried to minimize this patch and only address the potential crash.
The code looks like it could use more cleanup, though. Any comments
regarding the following suggestions?

1. It looks like we don't really need to ring the doorbell if cmd_list
   and cur_cmd are empty. Even if commands were no-oped, they are still
   on the list. And if there is anything between enqueue and dequeue
   which is not a link TRB and not on the list, we will have no idea
   how to handle the resulting event. IOW, it's not supposed to happen
   and probably never happening, or we would hear about it.

2. The whole business about turning commands into no-ops doesn't seem
   to happen in mainline today. Only the pending command gets aborted
   and it is removed from the list before the no-op loop runs.

3. xhci->current_cmd seems redundant - is there ever a valid reason for
   it being anything other than xhci->cmd_list first element?

   I find it suspicious that handle_cmd_completion() complains if the
   completed command isn't the first on cmd_list, but not if it doesn't
   match current_cmd and that current_cmd is then left unchanged.

Regards,
Michal

             reply	other threads:[~2024-12-03 19:51 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-03 19:51 Michal Pecio [this message]
2024-12-03 19:52 ` [PATCH 1/1] usb: xhci: Fix NULL pointer dereference on certain command aborts Michal Pecio
2024-12-04 17:53   ` Michal Pecio
2024-12-19 20:55 ` [PATCH v2] " Michal Pecio
2024-12-20 12:47   ` Mathias Nyman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241203205123.05b32413@foxbook \
    --to=michal.pecio@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).