[v1] ALSA: usb-audio: us144mkii: Fix null-deref in tascam_midi_in_urb_complete()

[PATCH] ALSA: usb-audio: us144mkii: Fix null-deref in tascam_midi_in_urb_complete()

Posted by Šerif Rami 5 months, 3 weeks ago

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@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/r/202508192109.lcMrINK1-lkp@intel.com/
Signed-off-by: Šerif Rami <ramiserifpersia@gmail.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);
+		goto out;
 	}
+
 out:
 	usb_put_urb(urb);
 }
 
+
 /**
  * tascam_midi_in_open() - Opens the MIDI input substream.
  * @substream: The ALSA rawmidi substream to open.
-- 
2.50.1

Re: [PATCH] ALSA: usb-audio: us144mkii: Fix null-deref in tascam_midi_in_urb_complete()

Posted by Dan Carpenter 5 months, 3 weeks ago

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@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/r/202508192109.lcMrINK1-lkp@intel.com/
> Signed-off-by: Šerif Rami <ramiserifpersia@gmail.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

Re: [PATCH] ALSA: usb-audio: usx2y: Fix null-deref in tascam_midi_in_urb_complete()

Posted by Šerif Rami 5 months, 3 weeks ago

Hi Dan,

Thanks for the review and for spotting these issues. You are correct.

I'll send a v2 shortly that addresses these points.

Thanks,
Šerif