Re: [PATCH] ALSA: line6: init buf to zero

[Takashi Iwai]

On Wed, 24 Jul 2024 07:58:45 +0200,
Edward Adam Davis wrote:
> 
> Syzbot report KMSAN uninit-value warnings.
> When alloc buffer for midi_buffer->buf, init mem to 0.
> 
> Reported-and-tested-by: syzbot+78eccfb8b3c9a85fc6c5@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=78eccfb8b3c9a85fc6c5
> Signed-off-by: Edward Adam Davis <eadavis@xxxxxx>
> ---
>  sound/usb/line6/midibuf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/usb/line6/midibuf.c b/sound/usb/line6/midibuf.c
> index e7f830f7526c..1b699cb3b38d 100644
> --- a/sound/usb/line6/midibuf.c
> +++ b/sound/usb/line6/midibuf.c
> @@ -48,7 +48,7 @@ void line6_midibuf_reset(struct midi_buffer *this)
>  
>  int line6_midibuf_init(struct midi_buffer *this, int size, int split)
>  {
> -	this->buf = kmalloc(size, GFP_KERNEL);
> +	this->buf = kzalloc(size, GFP_KERNEL);

Thanks for the patch.  But this just hides the KMSAN warning, and it
doesn't really address the cause - why it was exposed at all; the
driver code had already a check and should have accessed only the
updated data, but by some reason it slipped.

Through a quick glance, I see a possible.  If that's the cause, the
patch like below might help.  I checked the reproducer locally but
couldn't  trigger the bug on my image, unfortunately, so it's just a
wild guess, and it might be shooting a wrong way.  Let's see.


thanks,

Takashi

-- 8< --
--- a/sound/usb/line6/driver.c
+++ b/sound/usb/line6/driver.c
@@ -286,12 +286,14 @@ static void line6_data_received(struct urb *urb)
 {
 	struct usb_line6 *line6 = (struct usb_line6 *)urb->context;
 	struct midi_buffer *mb = &line6->line6midi->midibuf_in;
+	unsigned long flags;
 	int done;
 
 	if (urb->status == -ESHUTDOWN)
 		return;
 
 	if (line6->properties->capabilities & LINE6_CAP_CONTROL_MIDI) {
+		spin_lock_irqsave(&line6->line6midi->lock, flags);
 		done =
 			line6_midibuf_write(mb, urb->transfer_buffer, urb->actual_length);
 
@@ -300,12 +302,15 @@ static void line6_data_received(struct urb *urb)
 			dev_dbg(line6->ifcdev, "%d %d buffer overflow - message skipped\n",
 				done, urb->actual_length);
 		}
+		spin_unlock_irqrestore(&line6->line6midi->lock, flags);
 
 		for (;;) {
+			spin_lock_irqsave(&line6->line6midi->lock, flags);
 			done =
 				line6_midibuf_read(mb, line6->buffer_message,
 						   LINE6_MIDI_MESSAGE_MAXLEN,
 						   LINE6_MIDIBUF_READ_RX);
+			spin_unlock_irqrestore(&line6->line6midi->lock, flags);
 
 			if (done <= 0)
 				break;