Subject: Re: KMSAN: uninit-value in cdc_ncm_set_dgram_size

From: Oliver Neukum <[email-masked]>
Date: Tue, 05 Nov 2019 12:15:16 +0100

Am Montag, den 04.11.2019, 22:22 +0100 schrieb Bjørn Mork:
> This looks like a false positive to me. max_datagram_size is two bytes
> declared as
> 
>         __le16 max_datagram_size;
> 
> and the code leading up to the access on drivers/net/usb/cdc_ncm.c:587
> is:
> 
>         /* read current mtu value from device */
>         err = usbnet_read_cmd(dev, USB_CDC_GET_MAX_DATAGRAM_SIZE,
>                               USB_TYPE_CLASS | USB_DIR_IN | USB_RECIP_INTERFACE,
>                               0, iface_no, &max_datagram_size, 2);

At this point err can be 1.

>         if (err < 0) {
>                 dev_dbg(&dev->intf->dev, "GET_MAX_DATAGRAM_SIZE failed\n");
>                 goto out;
>         }
> 
>         if (le16_to_cpu(max_datagram_size) == ctx->max_datagram_size)
> 
> 
> 
> AFAICS, there is no way max_datagram_size can be uninitialized here.
> usbnet_read_cmd() either read 2 bytes into it or returned an error,

No. usbnet_read_cmd() will return the number of bytes transfered up
to the number requested or an error.

> causing the access to be skipped.  Or am I missing something?

Yes. You can get half the MTU. We have a similar class of bugs
with MAC addresses.

	Regards
		Oliver