[PATCH v3] usb: rtl8150: Initialize buffers to fix KMSAN uninit-value in rtl8150_open

[PATCH v3] usb: rtl8150: Initialize buffers to fix KMSAN uninit-value in rtl8150_open

[Dharanitharan R]

KMSAN reported an uninitialized value use in rtl8150_open().
Initialize rx_skb->data and intr_buff before submitting URBs to
ensure memory is in a defined state.

Changes in v3:
 - Fixed whitespace and indentation (checkpatch clean)
 - Corrected syzbot tag
 - Corrected syzbot hash and tag

Reported-by: syzbot+b4d5d8faea6996fd55e3@syzkaller.appspotmail.com
Signed-off-by: Dharanitharan R <dharanitharan725@gmail.com>
---
 drivers/net/usb/rtl8150.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index f1a868f0032e..a7116d03c3d3 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -735,33 +735,30 @@ static int rtl8150_open(struct net_device *netdev)
 	rtl8150_t *dev = netdev_priv(netdev);
 	int res;
 
-	if (dev->rx_skb == NULL)
-		dev->rx_skb = pull_skb(dev);
-	if (!dev->rx_skb)
-		return -ENOMEM;
-
 	set_registers(dev, IDR, 6, netdev->dev_addr);
 
 	/* Fix: initialize memory before using it (KMSAN uninit-value) */
 	memset(dev->rx_skb->data, 0, RTL8150_MTU);
 	memset(dev->intr_buff, 0, INTBUFSIZE);
 
-	usb_fill_bulk_urb(dev->rx_urb, dev->udev, usb_rcvbulkpipe(dev->udev, 1),
-		      dev->rx_skb->data, RTL8150_MTU, read_bulk_callback, dev);
-	if ((res = usb_submit_urb(dev->rx_urb, GFP_KERNEL))) {
-		if (res == -ENODEV)
-			netif_device_detach(dev->netdev);
+	usb_fill_bulk_urb(dev->rx_urb, dev->udev,
+			  usb_rcvbulkpipe(dev->udev, 1),
+			  dev->rx_skb->data, RTL8150_MTU,
+			  read_bulk_callback, dev);
+
+	res = usb_submit_urb(dev->rx_urb, GFP_KERNEL);
+	if (res) {
 		dev_warn(&netdev->dev, "rx_urb submit failed: %d\n", res);
 		return res;
 	}
 
-	usb_fill_int_urb(dev->intr_urb, dev->udev, usb_rcvintpipe(dev->udev, 3),
-		     dev->intr_buff, INTBUFSIZE, intr_callback,
-		     dev, dev->intr_interval);
-	if ((res = usb_submit_urb(dev->intr_urb, GFP_KERNEL))) {
-		if (res == -ENODEV)
-			netif_device_detach(dev->netdev);
-		dev_warn(&netdev->dev, "intr_urb submit failed: %d\n", res);
+	usb_fill_int_urb(dev->intr_urb, dev->udev,
+			 usb_rcvintpipe(dev->udev, 3),
+			 dev->intr_buff, INTBUFSIZE,
+			 intr_callback, dev, dev->intr_interval);
+
+	res = usb_submit_urb(dev->intr_urb, GFP_KERNEL);
+	if (res) {
 		usb_kill_urb(dev->rx_urb);
 		return res;
 	}
@@ -769,8 +766,7 @@ static int rtl8150_open(struct net_device *netdev)
 	enable_net_traffic(dev);
 	set_carrier(netdev);
 	netif_start_queue(netdev);
-
-	return res;
+	return 0;
 }
 
 static int rtl8150_close(struct net_device *netdev)
-- 
2.43.0

Re: [PATCH v3] usb: rtl8150: Initialize buffers to fix KMSAN uninit-value in rtl8150_open

[Andrew Lunn]

On Wed, Nov 05, 2025 at 07:56:26PM +0000, Dharanitharan R wrote:
> KMSAN reported an uninitialized value use in rtl8150_open().
> Initialize rx_skb->data and intr_buff before submitting URBs to
> ensure memory is in a defined state.

> @@ -769,8 +766,7 @@ static int rtl8150_open(struct net_device *netdev)
>  	enable_net_traffic(dev);
>  	set_carrier(netdev);
>  	netif_start_queue(netdev);
> -
> -	return res;
> +	return 0;
>  }

While i agree that res is guaranteed to by 0, this change has nothing
to do with fixing a KMSAN report. Please make it a separate patch in a
patch series. A patch should do one thing, and the commit message
should explain the "why?" of the change.

Please also take a read of:

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

The Subject line should indicate which tree this is for.

	Andrew

Re: [PATCH v3] usb: rtl8150: Initialize buffers to fix KMSAN uninit-value in rtl8150_open

[Andrew Lunn]

> -	usb_fill_bulk_urb(dev->rx_urb, dev->udev, usb_rcvbulkpipe(dev->udev, 1),
> -		      dev->rx_skb->data, RTL8150_MTU, read_bulk_callback, dev);
> -	if ((res = usb_submit_urb(dev->rx_urb, GFP_KERNEL))) {
> -		if (res == -ENODEV)
> -			netif_device_detach(dev->netdev);
> +	usb_fill_bulk_urb(dev->rx_urb, dev->udev,
> +			  usb_rcvbulkpipe(dev->udev, 1),
> +			  dev->rx_skb->data, RTL8150_MTU,
> +			  read_bulk_callback, dev);

If im reading this correctly, the usb_fill_bulk_urb() is identical,
you have just changed the wrapping. So this again has nothing to do
with the issue you are trying to fix. Changes like this make it harder
to see the real change which fixes the problem. And at the moment, i
don't see the actual fix.

If you want to make changes like this, please do so in a patch of its
own which only changes the wrapping, nothing else. That is then easy
to review.

Lots of small changes, which hopefully are obviously correct, with
good commit messages please.

    Andrew

---
pw-bot: cr

Re: [PATCH v3] usb: rtl8150: Initialize buffers to fix KMSAN uninit-value in rtl8150_open

[Greg KH]

On Wed, Nov 05, 2025 at 07:56:26PM +0000, Dharanitharan R wrote:
> KMSAN reported an uninitialized value use in rtl8150_open().
> Initialize rx_skb->data and intr_buff before submitting URBs to
> ensure memory is in a defined state.
> 
> Changes in v3:
>  - Fixed whitespace and indentation (checkpatch clean)
>  - Corrected syzbot tag
>  - Corrected syzbot hash and tag

Please follow the documentation for where to put this version
information.

> Reported-by: syzbot+b4d5d8faea6996fd55e3@syzkaller.appspotmail.com
> Signed-off-by: Dharanitharan R <dharanitharan725@gmail.com>
> ---
>  drivers/net/usb/rtl8150.c | 34 +++++++++++++++-------------------
>  1 file changed, 15 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> index f1a868f0032e..a7116d03c3d3 100644
> --- a/drivers/net/usb/rtl8150.c
> +++ b/drivers/net/usb/rtl8150.c
> @@ -735,33 +735,30 @@ static int rtl8150_open(struct net_device *netdev)
>  	rtl8150_t *dev = netdev_priv(netdev);
>  	int res;
>  
> -	if (dev->rx_skb == NULL)
> -		dev->rx_skb = pull_skb(dev);
> -	if (!dev->rx_skb)
> -		return -ENOMEM;

It looks like you are attempting to break existing situations?


> -
>  	set_registers(dev, IDR, 6, netdev->dev_addr);
>  
>  	/* Fix: initialize memory before using it (KMSAN uninit-value) */
>  	memset(dev->rx_skb->data, 0, RTL8150_MTU);
>  	memset(dev->intr_buff, 0, INTBUFSIZE);
>  
> -	usb_fill_bulk_urb(dev->rx_urb, dev->udev, usb_rcvbulkpipe(dev->udev, 1),
> -		      dev->rx_skb->data, RTL8150_MTU, read_bulk_callback, dev);
> -	if ((res = usb_submit_urb(dev->rx_urb, GFP_KERNEL))) {
> -		if (res == -ENODEV)
> -			netif_device_detach(dev->netdev);
> +	usb_fill_bulk_urb(dev->rx_urb, dev->udev,
> +			  usb_rcvbulkpipe(dev->udev, 1),
> +			  dev->rx_skb->data, RTL8150_MTU,
> +			  read_bulk_callback, dev);
> +
> +	res = usb_submit_urb(dev->rx_urb, GFP_KERNEL);
> +	if (res) {

Why did you break the -ENODEV situation?

How was this tested?

greg k-h