[PATCH] usb: rtl8150: avoid using uninitialized CSCR value

public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH] usb: rtl8150: avoid using uninitialized CSCR value
@ 2026-04-02  7:07 Morduan Zang
  2026-04-02 15:51 ` Petko Manolov
  2026-04-03 15:45 ` Simon Horman
  0 siblings, 2 replies; 5+ messages in thread
From: Morduan Zang @ 2026-04-02  7:07 UTC (permalink / raw)
  To: Petko Manolov
  Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, linux-usb, netdev, linux-kernel,
	syzbot+9db6c624635564ad813c, Morduan Zang

Check get_registers() when reading CSCR in set_carrier().
If the control transfer fails, don't use the stack value.

Reported-by: syzbot+9db6c624635564ad813c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=9db6c624635564ad813c
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Morduan Zang <zhangdandan@uniontech.com>
---
 drivers/net/usb/rtl8150.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 4cda0643afb6..7e32726d3e6f 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -722,7 +722,11 @@ static void set_carrier(struct net_device *netdev)
 	rtl8150_t *dev = netdev_priv(netdev);
 	short tmp;
 
-	get_registers(dev, CSCR, 2, &tmp);
+	if (get_registers(dev, CSCR, 2, &tmp) < 0) {
+		netif_carrier_off(netdev);
+		return;
+	}
+
 	if (tmp & CSCR_LINK_STATUS)
 		netif_carrier_on(netdev);
 	else
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] usb: rtl8150: avoid using uninitialized CSCR value
  2026-04-02  7:07 [PATCH] usb: rtl8150: avoid using uninitialized CSCR value Morduan Zang
@ 2026-04-02 15:51 ` Petko Manolov
  2026-04-03 15:45 ` Simon Horman
  1 sibling, 0 replies; 5+ messages in thread
From: Petko Manolov @ 2026-04-02 15:51 UTC (permalink / raw)
  To: Morduan Zang
  Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, linux-usb, netdev, linux-kernel,
	syzbot+9db6c624635564ad813c

On 26-04-02 15:07:43, Morduan Zang wrote:
> Check get_registers() when reading CSCR in set_carrier().
> If the control transfer fails, don't use the stack value.
> 
> Reported-by: syzbot+9db6c624635564ad813c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=9db6c624635564ad813c
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Morduan Zang <zhangdandan@uniontech.com>
> ---
>  drivers/net/usb/rtl8150.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> index 4cda0643afb6..7e32726d3e6f 100644
> --- a/drivers/net/usb/rtl8150.c
> +++ b/drivers/net/usb/rtl8150.c
> @@ -722,7 +722,11 @@ static void set_carrier(struct net_device *netdev)
>  	rtl8150_t *dev = netdev_priv(netdev);
>  	short tmp;
>  
> -	get_registers(dev, CSCR, 2, &tmp);
> +	if (get_registers(dev, CSCR, 2, &tmp) < 0) {
> +		netif_carrier_off(netdev);
> +		return;
> +	}
> +
>  	if (tmp & CSCR_LINK_STATUS)
>  		netif_carrier_on(netdev);
>  	else
> -- 

Nice catch.  You can add my Acked-by.


cheers,
Petko

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] usb: rtl8150: avoid using uninitialized CSCR value
  2026-04-02  7:07 [PATCH] usb: rtl8150: avoid using uninitialized CSCR value Morduan Zang
  2026-04-02 15:51 ` Petko Manolov
@ 2026-04-03 15:45 ` Simon Horman
  2026-04-05  8:52   ` Petko Manolov
  1 sibling, 1 reply; 5+ messages in thread
From: Simon Horman @ 2026-04-03 15:45 UTC (permalink / raw)
  To: Morduan Zang
  Cc: Petko Manolov, Andrew Lunn, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-usb, netdev, linux-kernel,
	syzbot+9db6c624635564ad813c

On Thu, Apr 02, 2026 at 03:07:43PM +0800, Morduan Zang wrote:
> Check get_registers() when reading CSCR in set_carrier().
> If the control transfer fails, don't use the stack value.
> 
> Reported-by: syzbot+9db6c624635564ad813c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=9db6c624635564ad813c
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Morduan Zang <zhangdandan@uniontech.com>
> ---
>  drivers/net/usb/rtl8150.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> index 4cda0643afb6..7e32726d3e6f 100644
> --- a/drivers/net/usb/rtl8150.c
> +++ b/drivers/net/usb/rtl8150.c
> @@ -722,7 +722,11 @@ static void set_carrier(struct net_device *netdev)
>  	rtl8150_t *dev = netdev_priv(netdev);
>  	short tmp;
>  
> -	get_registers(dev, CSCR, 2, &tmp);
> +	if (get_registers(dev, CSCR, 2, &tmp) < 0) {
> +		netif_carrier_off(netdev);
> +		return;
> +	}
> +
>  	if (tmp & CSCR_LINK_STATUS)
>  		netif_carrier_on(netdev);
>  	else

I wonder if we can handle this a bit more succinctly,
while still making it clear that the error is handled.
Something like this:

diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
index 4cda0643afb6..816759ced56c 100644
--- a/drivers/net/usb/rtl8150.c
+++ b/drivers/net/usb/rtl8150.c
@@ -722,7 +722,8 @@ static void set_carrier(struct net_device *netdev)
 	rtl8150_t *dev = netdev_priv(netdev);
 	short tmp;
 
-	get_registers(dev, CSCR, 2, &tmp);
+	if (get_registers(dev, CSCR, 2, &tmp) < 0)
+		tmp = 0;
 	if (tmp & CSCR_LINK_STATUS)
 		netif_carrier_on(netdev);
 	else

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] usb: rtl8150: avoid using uninitialized CSCR value
  2026-04-03 15:45 ` Simon Horman
@ 2026-04-05  8:52   ` Petko Manolov
  2026-04-05 23:38     ` Andrew Lunn
  0 siblings, 1 reply; 5+ messages in thread
From: Petko Manolov @ 2026-04-05  8:52 UTC (permalink / raw)
  To: Simon Horman
  Cc: Morduan Zang, Andrew Lunn, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, linux-usb, netdev, linux-kernel,
	syzbot+9db6c624635564ad813c

On 26-04-03 16:45:38, Simon Horman wrote:
> On Thu, Apr 02, 2026 at 03:07:43PM +0800, Morduan Zang wrote:
> > Check get_registers() when reading CSCR in set_carrier().
> > If the control transfer fails, don't use the stack value.
> > 
> > Reported-by: syzbot+9db6c624635564ad813c@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=9db6c624635564ad813c
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Signed-off-by: Morduan Zang <zhangdandan@uniontech.com>
> > ---
> >  drivers/net/usb/rtl8150.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> > index 4cda0643afb6..7e32726d3e6f 100644
> > --- a/drivers/net/usb/rtl8150.c
> > +++ b/drivers/net/usb/rtl8150.c
> > @@ -722,7 +722,11 @@ static void set_carrier(struct net_device *netdev)
> >  	rtl8150_t *dev = netdev_priv(netdev);
> >  	short tmp;
> >  
> > -	get_registers(dev, CSCR, 2, &tmp);
> > +	if (get_registers(dev, CSCR, 2, &tmp) < 0) {
> > +		netif_carrier_off(netdev);
> > +		return;
> > +	}
> > +
> >  	if (tmp & CSCR_LINK_STATUS)
> >  		netif_carrier_on(netdev);
> >  	else
> 
> I wonder if we can handle this a bit more succinctly,
> while still making it clear that the error is handled.
> Something like this:
> 
> diff --git a/drivers/net/usb/rtl8150.c b/drivers/net/usb/rtl8150.c
> index 4cda0643afb6..816759ced56c 100644
> --- a/drivers/net/usb/rtl8150.c
> +++ b/drivers/net/usb/rtl8150.c
> @@ -722,7 +722,8 @@ static void set_carrier(struct net_device *netdev)
>  	rtl8150_t *dev = netdev_priv(netdev);
>  	short tmp;
>  
> -	get_registers(dev, CSCR, 2, &tmp);
> +	if (get_registers(dev, CSCR, 2, &tmp) < 0)
> +		tmp = 0;
>  	if (tmp & CSCR_LINK_STATUS)
>  		netif_carrier_on(netdev);
>  	else

I was wondering if calling netif_carrier_off() is the right thing to do in case
get_registers() fail.

There are multiple get_registers() calls that don't check the error and if we do
this in set_carrier() maybe we should do the same thing across the whole driver?


cheers,
Petko

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] usb: rtl8150: avoid using uninitialized CSCR value
  2026-04-05  8:52   ` Petko Manolov
@ 2026-04-05 23:38     ` Andrew Lunn
  0 siblings, 0 replies; 5+ messages in thread
From: Andrew Lunn @ 2026-04-05 23:38 UTC (permalink / raw)
  To: Petko Manolov
  Cc: Simon Horman, Morduan Zang, Andrew Lunn, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, linux-usb, netdev,
	linux-kernel, syzbot+9db6c624635564ad813c

> > -	get_registers(dev, CSCR, 2, &tmp);
> > +	if (get_registers(dev, CSCR, 2, &tmp) < 0)
> > +		tmp = 0;
> >  	if (tmp & CSCR_LINK_STATUS)
> >  		netif_carrier_on(netdev);
> >  	else
> 
> I was wondering if calling netif_carrier_off() is the right thing to do in case
> get_registers() fail.
> 
> There are multiple get_registers() calls that don't check the error and if we do
> this in set_carrier() maybe we should do the same thing across the whole driver?

What does it actually mean if get_registers() fails? The device is
gone? Hot unplugged? If so, you are going to get a cascade of errors,
and then hopefully the USB core code removes the device?

Are there any legitimate reasons for get_registers() to fail if the
device is still plugged in?

It seems netif_carrier_off() is unnecessary?

   Andrew

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-04-05 23:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-02  7:07 [PATCH] usb: rtl8150: avoid using uninitialized CSCR value Morduan Zang
2026-04-02 15:51 ` Petko Manolov
2026-04-03 15:45 ` Simon Horman
2026-04-05  8:52   ` Petko Manolov
2026-04-05 23:38     ` Andrew Lunn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox