Re: [PATCH net-next v1 6/6] net: kunit: test netmem, page_pool, and skb frag refcounting

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] test: uninitialized mac_header causes out-of-bounds memcpy in skb_segment
- [Low] test: unchecked skb_clone allocation failure causes NULL pointer dereference
--

commit ab0c57271e2a8950e7df2bcb1c3668cf67f6f94b
Author: Mina Almasry <almasrymina@google.com>

net: kunit: test netmem, page_pool, and skb frag refcounting

Verify that non-pp (page._refcount / binding->ref) and page_pool
(pp_ref_count) references stay balanced without cross-counter leaks or
underflows across regular pages, page_pool pages, non-pp net_iovs, and
page_pool net_iovs with both pp_recycle=0 and pp_recycle=1.

> diff --git a/net/core/net_test.c b/net/core/net_test.c
> index 9c3a590865d26..f9f50f9159902 100644
> --- a/net/core/net_test.c
> +++ b/net/core/net_test.c

[ ... ]

> @@ -370,10 +370,757 @@ static void ip_tunnel_flags_test_run(struct kunit *test)

[ ... ]

> +	/* skb_segment on pp_recycle=1 skb propagates pp_recycle to segments */
> +	skb->protocol = htons(ETH_P_IP);
> +	skb_reset_network_header(skb);
> +	skb_shinfo(skb)->gso_size = 16;
> +	split = skb_segment(skb, NETIF_F_SG | NETIF_F_HW_CSUM);
> +	KUNIT_ASSERT_FALSE(test, IS_ERR_OR_NULL(split));

[Severity: High]
Can this missing MAC header reset cause a massive out-of-bounds memcpy?

Without calling skb_reset_mac_header(), the mac_header defaults to an
uninitialized value of ~0U (65535). Inside skb_segment(), this creates a
massive negative offset when calculating doffset, leading to a 64KB
out-of-bounds read via skb_copy_from_linear_data_offset() that will likely
crash the test with a kernel panic.

[ ... ]

> +	page_pool_ref_netmem(netmem);
> +	clone1 = skb_clone(skb, GFP_KERNEL);
> +	clone2 = skb_clone(clone1, GFP_KERNEL);
> +	KUNIT_ASSERT_NOT_NULL(test, clone1);
> +	KUNIT_ASSERT_NOT_NULL(test, clone2);

[Severity: Low]
Does this code dereference a potential NULL pointer if memory allocation fails?

If the first skb_clone() returns NULL for clone1 under memory pressure, it
is immediately passed to the second skb_clone(clone1, GFP_KERNEL). Because
skb_clone() dereferences its first argument unconditionally, this causes a
kernel panic before the test has a chance to safely fail at the subsequent
KUNIT_ASSERT_NOT_NULL() check.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010083935.3274178-1-almasrymina@google.com?part=6