diff mbox series

USB: usbtmc: prevent kernel-infoleak

Message ID tencent_088B2EF2AEE00C8AE7D706CCD2CBC6484906@qq.com
State New
Headers show
Series USB: usbtmc: prevent kernel-infoleak | expand

Commit Message

Edward Adam Davis Sept. 4, 2024, 1:55 p.m. UTC
The syzbot reported a kernel-usb-infoleak in usbtmc_write,
we need to clear the structure before filling fields.

Fixes: 4ddc645f40e9 ("usb: usbtmc: Add ioctl for vendor specific write")
Reported-and-tested-by: syzbot+9d34f80f841e948c3fdb@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9d34f80f841e948c3fdb
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 drivers/usb/class/usbtmc.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Greg Kroah-Hartman Sept. 4, 2024, 2:09 p.m. UTC | #1
On Wed, Sep 04, 2024 at 09:55:43PM +0800, Edward Adam Davis wrote:
> The syzbot reported a kernel-usb-infoleak in usbtmc_write,
> we need to clear the structure before filling fields.

Really?


> 
> Fixes: 4ddc645f40e9 ("usb: usbtmc: Add ioctl for vendor specific write")
> Reported-and-tested-by: syzbot+9d34f80f841e948c3fdb@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=9d34f80f841e948c3fdb
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
>  drivers/usb/class/usbtmc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
> index 6bd9fe565385..e9ddaa9b580d 100644
> --- a/drivers/usb/class/usbtmc.c
> +++ b/drivers/usb/class/usbtmc.c
> @@ -759,6 +759,7 @@ static struct urb *usbtmc_create_urb(void)
>  		usb_free_urb(urb);
>  		return NULL;
>  	}
> +	memset(dmabuf, 0, bufsize);

To do this simpler, kzmalloc() above this would be nice.

But, this feels odd, where is the data leaking from?  This is used for
both the read and write path, but where is the leak happening?  A short
read?  If so, we need to properly truncate the buffer being sent to
userspace and not send the unread data.  If a short write, that makes no
sense.

So this needs a bit more work, please find the real issue here and don't
paper over the problem with "set it all to 0" like this.

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 6bd9fe565385..e9ddaa9b580d 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -759,6 +759,7 @@  static struct urb *usbtmc_create_urb(void)
 		usb_free_urb(urb);
 		return NULL;
 	}
+	memset(dmabuf, 0, bufsize);
 
 	urb->transfer_buffer = dmabuf;
 	urb->transfer_buffer_length = bufsize;