Re: [PATCH net] net: usb: cdc_ncm: don't underflow the NDP room check

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] net: usb: cdc_ncm: missing Fixes trailer for NDP room check
  underflow fix
--

commit af2c2a63b21a4cf89327e3f98f1ee6e70ffe8420
Author: xujiahao <xujiahao@cmss.chinamobile.com>

> net: usb: cdc_ncm: don't underflow the NDP room check
>
> cdc_ncm_ndp16() and cdc_ncm_ndp32() verify that the new NDP and the
> datagram fit into the NTB with an unsigned subtraction:
>
>         if ((ctx->tx_curr_size - skb->len - reserve) < ctx->max_ndp_size)
>                 return NULL;
>
> tx_max is only clamped to max_datagram_size + max_ndp_size + the NTH
> header size, so tx_curr_size is just big enough for one datagram and
> one NDP.  A second datagram in the same NTB is therefore exactly the
> case this check has to reject, and that is also the case where
> skb->len + reserve exceeds tx_curr_size: the subtraction wraps around to
> a huge value, the check passes, and the skb_put_zero() of max_ndp_size
> bytes that follows overruns the buffer:
>
>   skbuff: skb_over_panic: text:ffff80000db33558 len:428 put:172
>   head:ffff0000c2c66100 tail:0x1ac end:0x140
>   kernel BUG at net/core/skbuff.c:200
>   skb_put_zero include/linux/skbuff.h:2595 [inline]
>   cdc_ncm_ndp16 drivers/net/usb/cdc_ncm.c:1132 [inline]
>   cdc_ncm_fill_tx_frame+0xdf0/0x30e0 drivers/net/usb/cdc_ncm.c:1309
>
> Here end is 0x140 = 320 = 140 + 172 + 8, i.e. the one-datagram minimum.
>
> The room check in the aggregation loop cannot catch this, because it is
> done after the NDP has already been pushed.
>
> Rewrite the test as an addition, which cannot underflow and gives the
> same answer whenever the original subtraction did not wrap.
>
> Reported-by: syzbot+1994e2ecf323ed90f255@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=1994e2ecf323ed90f255
> Signed-off-by: xujiahao <xujiahao@cmss.chinamobile.com>

[Severity: Low]
Since this fixes the unsigned subtraction underflow in cdc_ncm_ndp16() and
cdc_ncm_ndp32(), should the commit message include these Fixes: tags?

  Fixes: c78b7c586657 ("net: cdc_ncm: refactoring for tx multiplexing")
  Fixes: 0fa81b304a79 ("cdc_ncm: Implement the 32-bit version of NCM Transfer Block")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109060913.526268-1-xujiahao@cmss.chinamobile.com?part=1