Re: [PATCH net-next v7 10/10] r8152: enable UPS for RTL8157 and RTL8159 QFN68

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] r8152: uninitialized stack memory usage on ups timeout
--

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

r8152: enable UPS for RTL8157 and RTL8159 QFN68

This commit enables UPS for RTL8157 and RTL8159 QFN68 devices by adding
r8157_ups_en() and modifying UPS flags encoding.

> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 95f510da376de..7a1b34db1d88b 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
[ ... ]
> @@ -4484,6 +4522,49 @@ static void r8156_ups_en(struct r8152 *tp, bool enable)
>  	}
>  }
>  
> +static void r8157_ups_en(struct r8152 *tp, bool enable)
> +{
> +	if (enable) {
[ ... ]
> +	} else {
> +		ocp_byte_clr_bits(tp, MCU_TYPE_USB, USB_POWER_CUT,
> +				  UPS_EN | USP_PREWAKE);
> +
> +		ocp_byte_clr_bits(tp, MCU_TYPE_USB, USB_MISC_2,
> +				  UPS_FORCE_PWR_DOWN);
> +
> +		if (ocp_read_word(tp, MCU_TYPE_USB, USB_MISC_0) & PCUT_STATUS) {
> +			u32 ocp_data;
> +			int ret;
> +
> +			ret = read_poll_timeout(ocp_read_word, ocp_data,
> +						ocp_data & BACKUP_RESTRORE, 200,
> +						20000, false, tp, MCU_TYPE_USB,
> +						USB_GPHY_CTRL);
> +			if (ret) {
> +				rtl_set_inaccessible(tp);
> +				dev_err(&tp->intf->dev,
> +					"backup-restore timed out\n");
> +				return;

[Severity: High]
If the backup-restore wait in r8157_ups_en() times out, it flags the device
inaccessible but returns void. This hides the failure from callers like
rtl8157_runtime_enable(), which will then proceed with initialization.

Because the device is inaccessible, generic_ocp_read() will abort early
without writing to the provided buffer. Accessors like ocp_read_word()
do not check this return value and process uninitialized stack memory:

static u16 ocp_read_word(struct r8152 *tp, u16 type, u16 index)
{
	u32 data;
	__le32 tmp;
...
	generic_ocp_read(tp, index, sizeof(tmp), &tmp, type | byen);

	data = __le32_to_cpu(tmp);
...

This causes the driver to branch randomly based on garbage stack values.
For example, this is called shortly after resume:

static inline u16 rtl8152_get_speed(struct r8152 *tp)
{
	return ocp_read_word(tp, MCU_TYPE_PLA, PLA_PHYSTATUS);
}

Could r8157_ups_en() be updated to return an error code instead of void
so the caller can properly handle the timeout?

> +			}
> +
> +			/* clear USB fw_ver_reg */
> +			ocp_write_byte(tp, MCU_TYPE_USB, USB_FW_USB_VER, 0);
> +
> +			tp->rtl_ops.hw_phy_cfg(tp);
> +
> +			rtl8152_set_speed(tp, tp->autoneg, tp->speed,
> +					  tp->duplex, tp->advertising);
> +		}
> +	}
> +}

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