netdev - Re: [PATCH 1/1] net: cdc_ncm: Allow for dwNtbOutMaxSize to be unset or zero

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87wnklivun.fsf@miraculix.mork.no>
Date:   Fri, 03 Dec 2021 15:52:48 +0100
From:   Bjørn Mork <bjorn@...k.no>
To:     Lee Jones <lee.jones@...aro.org>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Oliver Neukum <oliver@...kum.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, linux-usb@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH 1/1] net: cdc_ncm: Allow for dwNtbOutMaxSize to be unset
 or zero

Lee Jones <lee.jones@...aro.org> writes:

> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 24753a4da7e60..e303b522efb50 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -181,6 +181,8 @@ static u32 cdc_ncm_check_tx_max(struct usbnet *dev, u32 new_tx)
>  		min = ctx->max_datagram_size + ctx->max_ndp_size + sizeof(struct usb_cdc_ncm_nth32);
>  
>  	max = min_t(u32, CDC_NCM_NTB_MAX_SIZE_TX, le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize));
> +	if (max == 0)
> +		max = CDC_NCM_NTB_MAX_SIZE_TX; /* dwNtbOutMaxSize not set */
>  
>  	/* some devices set dwNtbOutMaxSize too low for the above default */
>  	min = min(min, max);

I believe this is the best possible fix, considering the regressions
anything stricter might cause.

We know of at least one MBIM device where dwNtbOutMaxSize is as low as
2048.

According to the MBIM spec, the minimum and default value for
wMaxSegmentSize is also 2048.  This implies that the calculated "min"
value is at least 2076, which is why we need that odd looking

  min = min(min, max);

So let's just fix this specific zero case without breaking the
non-conforming devices.


Reviewed-by: Bjørn Mork <bjorn@...k.no>