Re: [PATCH net v2] net: usb: cdc_ncm: reject short GET_NTB_PARAMETERS responses

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usbnet: cdc_ncm: strict 28-byte length check breaks pre-errata 26-byte hardware
--

commit 21684fc876b53044a42f2941b653d84cd5d8ed14
Author: Xinsheng Zhu <xinsheng.zhu.ny@gmail.com>
net: usb: cdc_ncm: reject short GET_NTB_PARAMETERS responses

This patch updates the initialization sequence to strictly validate the
length of the GET_NTB_PARAMETERS response. Instead of only checking for
negative errors, it requires the response to exactly match the 28-byte
size of the ncm_parm structure to avoid using zero-filled parameters.

Link: https://lore.kernel.org/netdev/20261005041525.49111-1-xinsheng.zhu.ny@gmail.com/

> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -524,9 +524,9 @@ static int cdc_ncm_init(struct usbnet *dev)
>  			      |USB_RECIP_INTERFACE,
>  			      0, iface_no, &ctx->ncm_parm,
>  			      sizeof(ctx->ncm_parm));
> -	if (err < 0) {
> +	if (err != sizeof(ctx->ncm_parm)) {

[Severity: High]
Could this exact length check unconditionally fail on pre-errata CDC NCM 1.0
devices?

According to the driver's fallback logic further down in cdc_ncm_init(),
"devices prior to NCM Errata shall set this field to zero" when referring to
wNtbOutMaxDatagrams. Pre-errata devices typically return a 26-byte structure
instead of the full 28-byte structure.

Previously, with the leniency of checking if err is negative, a 26-byte read
would succeed and the kzalloc-backed ctx would safely leave the trailing
missing fields as zero. By requiring exactly 28 bytes, does this patch
unintentionally reject valid 26-byte responses from legacy hardware with -EIO,
preventing probe entirely on those devices?

>  		dev_err(&dev->intf->dev, "failed GET_NTB_PARAMETERS\n");
> -		return err; /* GET_NTB_PARAMETERS is required */
> +		return err < 0 ? err : -EIO; /* GET_NTB_PARAMETERS is required */
>  	}
>  
>  	/* set CRC Mode */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011035150.53315-1-xinsheng.zhu.ny@gmail.com?part=1