| 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
| ||
|
Message-ID: <CAHjaAcSsQtXJyRFFAyPggKTdsNEejReEQ3qFgFv4dQej6Tdo5g@mail.gmail.com>
Date: Wed, 12 Oct 2022 13:28:29 +0900
From: Seunghun Han <kkamagui@...il.com>
To: linux-kernel@...r.kernel.org
Cc: davem@...emloft.net, linux-usb@...r.kernel.org, oliver@...kum.org,
kuba@...nel.org, edumazet@...gle.com, pabeni@...hat.com
Subject: Re: [PATCH] net: usb: cdc_mbim: adding Microsoft mobile broadband modem
Does anyone have comments on my patch? If there are some points to
improve, please let me know. I'm waiting for them.
On Wed, Aug 3, 2022 at 6:02 PM Seunghun Han <kkamagui@...il.com> wrote:
>
> Microsoft branded mobile broadband modems typically used in the Surface series
> don't work with the Linux kernel. When I query firmware information to the
> modem using the mbimcli tool, it always returns the Failure (0x02) value like
> below.
>
> === Start of the firmware id query ===
> $> mbimcli -d /dev/cdc-wdm4 --ms-query-firmware-id -v
> [26 Jul 2022, 05:24:07] [Debug] opening device...
> [26 Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Queried max control message size: 4096
> [26 Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Sent message...
> <<<<<< RAW:
> <<<<<< length = 16
> <<<<<< data = 01:00:00:00:10:00:00:00:01:00:00:00:00:10:00:00
>
> [26 Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Sent message (translated)...
> <<<<<< Header:
> <<<<<< length = 16
> <<<<<< type = open (0x00000001)
> <<<<<< transaction = 1
> <<<<<< Contents:
> <<<<<< max control transfer = 4096
>
> [26 Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Received message...
> >>>>>> RAW:
> >>>>>> length = 16
> >>>>>> data = 01:00:00:80:10:00:00:00:01:00:00:00:02:00:00:00
>
> [26 Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Received message...Message Type 80000001
> [26 Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 864
> [26 Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 897
> [26 Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 923
> [26 Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 930
> [26 Jul 2022, 05:24:07] [Debug] [/dev/cdc-wdm4] Processing process_message 935
> [26 Jul 2022, 05:24:07] [Debug] getting open done result failed: closed
> error: couldn't open the MbimDevice: Failure
> === End of the firmware id query ===
>
> After kernel debugging, I found that the modem reported that the dwNtbInMaxSize
> value of ncm_parm was 16384 during the initialization sequence.
> So the cdc_ncm_update_rxtx_max() in cdc_ncm_bind_common() didn't send
> USB_CDC_SET_NTB_INPUT_SIZE command because the default input size was the same,
> 16384.
>
> It's good and proper behavior. However, Microsoft branded MBMs (including the
> latest one in Surface Go 3) fail until the kernel explicitly sets the input
> size.
>
> This patch adds a new table and code changes that explicitly send
> the USB_CDC_SET_NTB_INPUT_SIZE command to support Microsoft branded MBMs.
>
> Signed-off-by: Seunghun Han <kkamagui@...il.com>
> ---
> drivers/net/usb/cdc_mbim.c | 24 ++++++++++++++++++++++++
> drivers/net/usb/cdc_ncm.c | 2 +-
> include/linux/usb/cdc_ncm.h | 1 +
> 3 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/usb/cdc_mbim.c b/drivers/net/usb/cdc_mbim.c
> index c89639381eca..c0c23cfc02a7 100644
> --- a/drivers/net/usb/cdc_mbim.c
> +++ b/drivers/net/usb/cdc_mbim.c
> @@ -618,6 +618,22 @@ static const struct driver_info cdc_mbim_info_avoid_altsetting_toggle = {
> .data = CDC_MBIM_FLAG_AVOID_ALTSETTING_TOGGLE,
> };
>
> +/* Microsoft branded modems do not work properly without setting the input size
> + * explicitly in cdc_ncm_bind_common.
> + * CDC_MBIM_FLAG_SET_INPUT_SIZE_EXPLICITLY flag is used to set the input size
> + * during initialization.
> + */
> +static const struct driver_info cdc_mbim_info_set_input_size_explicitly = {
> + .description = "CDC MBIM",
> + .flags = FLAG_NO_SETINT | FLAG_MULTI_PACKET | FLAG_WWAN | FLAG_SEND_ZLP,
> + .bind = cdc_mbim_bind,
> + .unbind = cdc_mbim_unbind,
> + .manage_power = cdc_mbim_manage_power,
> + .rx_fixup = cdc_mbim_rx_fixup,
> + .tx_fixup = cdc_mbim_tx_fixup,
> + .data = CDC_MBIM_FLAG_SET_INPUT_SIZE_EXPLICITLY,
> +};
> +
> static const struct usb_device_id mbim_devs[] = {
> /* This duplicate NCM entry is intentional. MBIM devices can
> * be disguised as NCM by default, and this is necessary to
> @@ -665,6 +681,14 @@ static const struct usb_device_id mbim_devs[] = {
> .driver_info = (unsigned long)&cdc_mbim_info_avoid_altsetting_toggle,
> },
>
> + /* Some Microsoft branded mobile broadband modems used in the Surface
> + * seires are known to fail unless the input size is set explicitly.
> + * Applying it to all Microsoft branded MBMs.
> + */
> + { USB_VENDOR_AND_INTERFACE_INFO(0x045e, USB_CLASS_COMM, USB_CDC_SUBCLASS_MBIM, USB_CDC_PROTO_NONE),
> + .driver_info = (unsigned long)&cdc_mbim_info_set_input_size_explicitly,
> + },
> +
> /* default entry */
> { USB_INTERFACE_INFO(USB_CLASS_COMM, USB_CDC_SUBCLASS_MBIM, USB_CDC_PROTO_NONE),
> .driver_info = (unsigned long)&cdc_mbim_info_zlp,
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 8d5cbda33f66..915e29c987cb 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -407,7 +407,7 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx)
> val = cdc_ncm_check_rx_max(dev, new_rx);
>
> /* inform device about NTB input size changes */
> - if (val != ctx->rx_max) {
> + if (val != ctx->rx_max || ctx->drvflags & CDC_MBIM_FLAG_SET_INPUT_SIZE_EXPLICITLY) {
> __le32 dwNtbInMaxSize = cpu_to_le32(val);
>
> dev_info(&dev->intf->dev, "setting rx_max = %u\n", val);
> diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
> index 2d207cb4837d..a24f84b31a54 100644
> --- a/include/linux/usb/cdc_ncm.h
> +++ b/include/linux/usb/cdc_ncm.h
> @@ -88,6 +88,7 @@
> #define CDC_NCM_FLAG_NDP_TO_END 0x02 /* NDP is placed at end of frame */
> #define CDC_MBIM_FLAG_AVOID_ALTSETTING_TOGGLE 0x04 /* Avoid altsetting toggle during init */
> #define CDC_NCM_FLAG_PREFER_NTB32 0x08 /* prefer NDP32 over NDP16 */
> +#define CDC_MBIM_FLAG_SET_INPUT_SIZE_EXPLICITLY 0x10 /* Set input size explicitly during init */
>
> #define cdc_ncm_comm_intf_is_mbim(x) ((x)->desc.bInterfaceSubClass == USB_CDC_SUBCLASS_MBIM && \
> (x)->desc.bInterfaceProtocol == USB_CDC_PROTO_NONE)
> --
> 2.30.2
>
Powered by blists - more mailing lists