Re: [PATCH net-next v7 01/10] r8152: split r8156_init by generation and factor out wait_autoload_done

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] net: r8152: uninitialized stack memory read via read_poll_timeout()
--

commit 7e3b002970c7595bd4777521d55501576d7f77e7
Author: Chih Kai Hsu <hsu.chih.kai@realtek.com>

r8152: split r8156_init by generation and factor out wait_autoload_done

This commit splits r8156_init into two generation-specific functions and
factors out the repeated AUTOLOAD_DONE polling loop into a new common
wait_autoload_done helper using read_poll_timeout.

> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index af3d7dcb2f141..518a2b42c2200 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c

[ ... ]

> @@ -4172,6 +4172,23 @@ static u16 r8153_phy_status(struct r8152 *tp, u16 desired)
>  	return data;
>  }
>  
> +static int wait_autoload_done(struct r8152 *tp)
> +{
> +	u16 ocp_data;
> +	int ret;
> +
> +	ret = read_poll_timeout(ocp_read_word, ocp_data,
> +				test_bit(RTL8152_INACCESSIBLE, &tp->flags) ||
> +				(ocp_data & AUTOLOAD_DONE),
> +				20000, 10 * USEC_PER_SEC, false, tp,
> +				MCU_TYPE_PLA, PLA_BOOT_CTRL);

[Severity: Medium]
When the device is inaccessible, could this read uninitialized stack memory?

The read_poll_timeout macro executes the read operation before evaluating
the condition. In this case, ocp_read_word is called before checking
test_bit(RTL8152_INACCESSIBLE, &tp->flags).

If the device is inaccessible, generic_ocp_read returns -ENODEV and leaves
the local variable tmp uninitialized. However, ocp_read_word ignores this
return code and proceeds to read the uninitialized memory:

drivers/net/usb/r8152.c:ocp_read_word() {
    __le32 tmp;
    ...
    generic_ocp_read(tp, index, sizeof(tmp), &tmp, type | byen);

    data = __le32_to_cpu(tmp);
    ...
}

The original open-coded loops explicitly checked the inaccessible flag before
calling ocp_read_word, which avoided this.

> +
> +	if (ret)
> +		dev_err(&tp->intf->dev, "autoload done timeout\n");
> +
> +	return test_bit(RTL8152_INACCESSIBLE, &tp->flags) ? -ENODEV : 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014081904.6491-1-nic_swsd@realtek.com?part=1