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

Takashi Iwai
2024-07-24T07:21:31Z

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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=78eccfb8b3c9a85fc6c5
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> 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;