netdev - Re: [2/3] net: amd-xgbe: add ARP offload ethtool self-test

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20260203023713.896262-1-kuba@kernel.org>
Date: Mon,  2 Feb 2026 18:37:13 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Raju.Rangoju@....com
Cc: Jakub Kicinski <kuba@...nel.org>,
	netdev@...r.kernel.org
Subject: Re: [2/3] net: amd-xgbe: add ARP offload ethtool self-test

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: amd-xgbe: add ARP offload ethtool self-test

This patch adds an ethtool self-test to verify the hardware ARP offload
functionality. The test creates an ARP request, enables ARP offload, and
verifies the hardware responds with an ARP reply.

> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-selftest.c b/drivers/net/ethernet/amd/xgbe/xgbe-selftest.c
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-selftest.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-selftest.c

[ ... ]

> +static int xgbe_test_arpoffload(struct xgbe_prv_data *pdata)
> +{

[ ... ]

> +	tdata->pt.type = htons(ETH_P_ARP);
> +	tdata->pt.func = xgbe_test_arp_validate;
> +	tdata->pt.dev = pdata->netdev;
> +	tdata->pt.af_packet_priv = tdata;
> +	tdata->packet = &attr;
> +	dev_add_pack(&tdata->pt);

[ ... ]

> +	/* Create ARP request packet */
> +	skb = arp_create(ARPOP_REQUEST, ETH_P_ARP, htonl(dst_ip),
> +			 pdata->netdev,	htonl(src_ip), NULL, src, bcast);
> +	if (!skb) {
> +		ret = -ENOMEM;
> +		goto free;
> +	}

Can this cause a use-after-free? The packet handler is registered via
dev_add_pack() before arp_create() is called. When arp_create() fails,
'goto free' jumps directly to kfree(tdata) without calling
dev_remove_pack(&tdata->pt).

This leaves the packet handler registered with a dangling pointer
(tdata->pt.af_packet_priv = tdata). When any ARP packet subsequently
arrives, xgbe_test_arp_validate() is called and dereferences freed memory:

    struct net_test_priv *tdata = pt->af_packet_priv;
    ...
    if (!ether_addr_equal_unaligned(eth_hdr->h_dest, tdata->packet->src))

Should 'goto free' be changed to 'goto cleanup' here to ensure
dev_remove_pack() is called?
-- 
pw-bot: cr