Re: [PATCH net-next v8 05/11] 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 to consider:
- [Medium] r8152: uninitialized stack memory read via
  read_poll_timeout() in wait_autoload_done()
--

commit 0223d579ae41de9da02eeed7c11b0b17d1632cc5
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 r8156_init() and r8157_init() by chip
generation and factors the PLA_BOOT_CTRL polling loops into a shared
wait_autoload_done() helper using read_poll_timeout().

> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 6f55aed4709e1..364a0573f6cd1 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -4177,6 +4177,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);
> +
> +	if (ret)
> +		dev_err(&tp->intf->dev, "autoload done timeout\n");
> +
> +	return test_bit(RTL8152_INACCESSIBLE, &tp->flags) ? -ENODEV : 0;
> +}

[Severity: Medium]
As reported by Jakub Kicinski on v7 [1], does read_poll_timeout() cause
ocp_read_word() to read and return uninitialized stack memory when
RTL8152_INACCESSIBLE is set?

read_poll_timeout() runs ocp_read_word() before evaluating the stop
condition test_bit(RTL8152_INACCESSIBLE, &tp->flags).  When
RTL8152_INACCESSIBLE is set (for instance if a USB transfer fails during the
polling sleep, or in r8153b_ups_en() when the preceding ocp_read_word()
fails and returns 0xffff), generic_ocp_read() returns -ENODEV immediately
without initializing tmp, which ocp_read_word() then converts and returns by
value:

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

	data = __le32_to_cpu(tmp);
	data >>= (shift * 8);
	data &= 0xffff;

	return (u16)data;
}

Returning an uninitialized value triggers a KMSAN uninit-value report, and
if generic_ocp_read() instead fails with -ENOMEM in get_registers() without
setting RTL8152_INACCESSIBLE, wait_autoload_done() also branches on the
uninitialized ocp_data.

[1] https://lore.kernel.org/r/20261021005210.1254886-1-kuba@kernel.org

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