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  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]
Date:	Mon, 17 Mar 2014 22:27:18 +0100
From:	Bjørn Mork <>
To:	Ben Chan <>
Cc:,,, Oliver Neukum <>,
	Greg Kroah-Hartman <>,
	Greg Suarez <>
Subject: Re: [PATCH 2/2] net: cdc_ncm: respect operator preferred MTU reported by MBIM

Ben Chan <> writes:

> It's a bit messy how MTU is currently handled in MBIM. While wMTU may
> seem optional and redundant, it addresses some issues with
> wMaxSegmentSize and MBIM_CID_IP_CONFIGURATION, and hence why I suggest
> using wMTU when available:
> (1) wMaxSegmentSize
> The MBIM 1.0 errata-1 spec does suggest that wMaxSegmentSize must be
> at least 2048 and should not be used for determining IP MTU. However,
> some MBIM devices follow Microsoft's guideline, which suggests using
> wMaxSegmentSize to determine link MTU and its value should be between
> 1280 and 1500. The guideline may have been made before MBIM 1.0, but
> it clearly contradicts and violates the current spec. Unfortunately,
> it's followed by device vendors in practice. We could modify cdc_ncm
> not to have the lower bound (i.e. CDC_MBIM_MIN_DATAGRAM_SIZE = 2048)
> that it currently enforces. I don't feel like we should violate the
> spec in the driver if there are alternative solutions.
> MBIM_CID_IP_CONFIGURATION doesn't necessarily contain MTU information
> according to the spec. Bit 3 of IPv4ConfigurationAvailable /
> IPv6ConfigurationAvailable of MBIM_IP_CONFIGURATION_INFO indicates
> whether MTU information is available. As the Microsoft guideline also
> suggests that MBIM_CID_IP_CONFIGURATION wouldn't be used for MTU
> purpose, I wouldn't be too surprised that devices just don't bother to
> (3) wMTU
> The MBIM extended functional descriptor is optional, but device
> vendors do use it to indicate the MTU (mostly due to aforementioned
> confusion around wMaxSegmentSize). Using the wMTU field wouldn't add
> too much code or runtime overhead in kernel, so why don't we use it to
> set up the initial MTU when available? We could handle it in
> userspace, but I see the cdc_ncm driver is a better fit as it (like
> other net drivers) already sets up mtu and leaving the wMTU case would
> seem incomplete to me.
> While (1) and (2) are fixable, it's hard to convince device vendors to
> update their firmware just for that as carrier certifications impose a
> heavy cost of firmware changes.

This sounds all reasonable to me. Thanks for taking the time to explain
it in such detail. I did know that some vendors set wMaxSegmentSize too
low, but had no idea that vendors were using the extended descriptor

If so, then yes, it does make sense for the driver to base the default
MTU on this descriptor.

>> wMTU access needs le16_to_cpu.
> Good catch. I will fix it in patch v2.

Tip: I found this because my test script/makefile includes

I find that very useful when dealing with USB on a little endian system,
like most of us have.  It's all too easy to miss a conversion otherwise.

>> Could we move this final MTU correction from cdc_ncm_setup to
>> cdc_ncm_bind_common to avoid bloating the device struct with another
>> descriptor pointer we donæt really need to keep around?
>> I know we look into descriptors in cdc_ncm_setup, because we have to,
>> but ideally I would have loved to see cdc_ncm_setup dealing with *just*
>> the NCM/MBIM specific control setup messages and cdc_ncm_bind_common
>> dealing with all the functional descriptors.  That seems most logic to
>> me (but is of course only my personal opinion and nothing else - I do
>> not know what the original cdc_ncm author intended)
> I understand the argument against the extra descriptor pointer. But I
> think it's better to keep the mtu related code together so that one
> can easily see how MTU is determined when trying to change or refactor
> the code. I haven't looked into what cdc_ncm_setup was originally
> intended for. If we'd like to avoid adding an extra pointer in
> cdc_ncm_ctx, we could have cdc_ncm_bind passing a locally scoped
> context to cdc_ncm_setup.

No, the extra pointer doesn't matter much. Just keep it as it is.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

Powered by blists - more mailing lists