[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aKS-B6O8ZEAN_X7x@stanley.mountain>
Date: Tue, 19 Aug 2025 21:10:15 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Šerif Rami <ramiserifpersia@...il.com>
Cc: Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>,
Stephen Rothwell <sfr@...b.auug.org.au>,
"open list:SOUND" <linux-sound@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>,
kernel test robot <lkp@...el.com>
Subject: Re: [PATCH] ALSA: usb-audio: us144mkii: Fix null-deref in
tascam_midi_in_urb_complete()
On Tue, Aug 19, 2025 at 07:38:30PM +0200, Šerif Rami wrote:
> The smatch tool reported a potential null pointer dereference in
> tascam_midi_in_urb_complete(). The 'tascam' variable, derived from
> 'urb->context', was checked for nullity in one place, but dereferenced
> without a check in several other places.
>
> This patch fixes the issue by adding a null check at the beginning of
> the function. If 'tascam' is null, the function now safely exits.
> This prevents any potential crashes from null pointer dereferences.
>
> It also fixes a latent bug where 'usb_put_urb()' could
> be called twice for the same URB on submission failure, which would
> lead to a use-after-free error.
>
> Reported-by: kernel test robot <lkp@...el.com>
> Reported-by: Dan Carpenter <dan.carpenter@...aro.org>
> Closes: https://lore.kernel.org/r/202508192109.lcMrINK1-lkp@intel.com/
> Signed-off-by: Šerif Rami <ramiserifpersia@...il.com>
> ---
> sound/usb/usx2y/us144mkii_midi.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/sound/usb/usx2y/us144mkii_midi.c b/sound/usb/usx2y/us144mkii_midi.c
> index 5759f6010..1aca38f38 100644
> --- a/sound/usb/usx2y/us144mkii_midi.c
> +++ b/sound/usb/usx2y/us144mkii_midi.c
> @@ -41,6 +41,9 @@ void tascam_midi_in_urb_complete(struct urb *urb)
> struct tascam_card *tascam = urb->context;
> int ret;
>
> + if (!tascam)
> + goto out;
> +
> if (urb->status) {
> if (urb->status != -ENOENT && urb->status != -ECONNRESET &&
> urb->status != -ESHUTDOWN && urb->status != -EPROTO) {
> @@ -51,7 +54,7 @@ void tascam_midi_in_urb_complete(struct urb *urb)
> goto out;
> }
>
> - if (tascam && atomic_read(&tascam->midi_in_active) &&
> + if (atomic_read(&tascam->midi_in_active) &&
> urb->actual_length > 0) {
> kfifo_in_spinlocked(&tascam->midi_in_fifo, urb->transfer_buffer,
> urb->actual_length, &tascam->midi_in_lock);
> @@ -66,11 +69,14 @@ void tascam_midi_in_urb_complete(struct urb *urb)
> "Failed to resubmit MIDI IN URB: error %d\n", ret);
> usb_unanchor_urb(urb);
> usb_put_urb(urb);
usb_put_urb() is still called twice, though?
> + goto out;
> }
> +
> out:
> usb_put_urb(urb);
> }
>
> +
You accidentally added a double blank line here.
> /**
> * tascam_midi_in_open() - Opens the MIDI input substream.
> * @substream: The ALSA rawmidi substream to open.
regards,
dan carpenter
Powered by blists - more mailing lists