Re: [PATCH 1/3] net: wwan: mhi_wwan_mbim: guard against a cyclic NDP chain

[Loic Poulain]

On Thu, Sep 10, 2026 at 5:34 AM Guanglei Zhu <zhugl3@xxxxxxxxxxxx> wrote:
>
> The NDP traversal in mhi_mbim_rx() only stops when wNextNdpIndex is
> zero.  Nothing requires the offsets to advance, so a modem that
> points an NDP at itself, or at an earlier NDP, keeps the loop
> spinning forever on one CPU.
>
> Break out when the next NDP offset is not larger than the current
> one.
>
> Fixes: aa730a9905b7 ("net: wwan: Add MHI MBIM network driver")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Guanglei Zhu <zhugl3@xxxxxxxxxxxx>
> ---
>
> Verified in a QEMU guest with a fault injector feeding the driver's
> receive callback an NTB whose single NDP points at itself: the
> unpatched driver spins in mhi_mbim_rx() with one CPU pinned at 100%
> and the thread never returns.  With this check the loop terminates
> within one iteration.
>  drivers/net/wwan/mhi_wwan_mbim.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/net/wwan/mhi_wwan_mbim.c b/drivers/net/wwan/mhi_wwan_mbim.c
> index a94998712..ef158edeb 100644
> --- a/drivers/net/wwan/mhi_wwan_mbim.c
> +++ b/drivers/net/wwan/mhi_wwan_mbim.c
> @@ -254,6 +254,7 @@ static int mbim_rx_verify_ndp16(struct sk_buff *skb, struct usb_cdc_ncm_ndp16 *n
>  static void mhi_mbim_rx(struct mhi_mbim_context *mbim, struct sk_buff *skb)
>  {
>         int ndpoffset;
> +       int last_ndpoffset = 0;
>
>         /* Check NTB header and retrieve first NDP offset */
>         ndpoffset = mbim_rx_verify_nth16(mbim, skb);
> @@ -265,6 +266,13 @@ static void mhi_mbim_rx(struct mhi_mbim_context *mbim, struct sk_buff *skb)
>         /* Process each NDP */
>         while (1) {
>                 struct usb_cdc_ncm_ndp16 ndp16;
> +
> +               if (ndpoffset <= last_ndpoffset) {
> +                       net_err_ratelimited("mbim: non-increasing NDP offset (%u)\n",
> +                                           ndpoffset);
> +                       break;
> +               }
> +               last_ndpoffset = ndpoffset;

It would be better to address this under the next_ndp retrieval, something like:
```
n = (int)le16_to_cpu(ndp16.wNextNdpIndex);
if (n > ndpoffset)
   ndpoffset = n;
else
  break;
```

>                 struct usb_cdc_ncm_dpe16 dpe16;
>                 struct mhi_mbim_link *link;
>                 int nframes, n, dpeoffset;
> --
> 2.43.0
>