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

Petko Manolov
2026-04-05T08:52:23Z

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