Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:
Critical: 0 · High: 0 · Medium: 1 · Low: 0
- [Medium] In mctp_usblib_test_rx_split_header() (drivers/net/mctp/mctp-
usblib-test.c:436) an skb dequeued from dev->rx_pkts is released with…
--
net: mctp: usblib: Complete rx tests
This adds two KUnit cases to the mctp-usblib test suite exercising the
non-spanning receive path: a packet shorter than the minimum header length,
and a packet whose declared payload length is not satisfied by a single
submission. Both new cases are registered in mctp_usblib_test_cases[].
> diff --git a/drivers/net/mctp/mctp-usblib-test.c
> b/drivers/net/mctp/mctp-usblib-test.c
> index b0266be79e94..22aaa4795cbd 100644
> --- a/drivers/net/mctp/mctp-usblib-test.c
> +++ b/drivers/net/mctp/mctp-usblib-test.c
[ ... ]
> @@ -533,6 +593,8 @@ static struct kunit_case mctp_usblib_test_cases[] = {
> KUNIT_CASE(mctp_usblib_test_rx_header_splits),
^^^^
[Severity: Medium]
While looking at the suite that these two new cases join, is the skb
release in mctp_usblib_test_rx_split_header() correct? That helper is not
touched by this patch (it comes from the earlier "Add test for splits
inside headers" change in the same series), but it is the only skb release
site in this file that does not use the skbuff API:
drivers/net/mctp/mctp-usblib-test.c:mctp_usblib_test_rx_split_header() {
...
skb = __skb_dequeue(&dev->rx_pkts);
KUNIT_EXPECT_NOT_NULL(test, skb);
if (skb) {
KUNIT_EXPECT_EQ(test, skb->len, len - HDR_LEN);
kfree(skb);
}
}
Does this leak skb->head? kfree() returns only the sk_buff head object to
its cache and never runs skb_release_all()/skb_release_data(), so the
separately allocated data buffer is never freed.
In span mode mctp_usblib_rx_prepare() sizes that buffer as
roundup(MCTP_USB_1_1_PKTLEN_MAX, ep_pktlen), i.e. 8192 bytes, and
mctp_usblib_test_rx_header_splits() calls the helper eight times, so a
single run of the suite would leak roughly 64 KB and likely trip kmemleak
in CI. The other release sites in the same file use kfree_skb(), and
mctp_usblib_test_dev_action() uses skb_queue_purge().
Would kfree_skb(skb) be the right call here?
--
Sashiko AI review ·
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260903-dev-mctp-usb-lib-test-v2-0-313cf15e0b56%40codeconstruct.com.au
