netdev - Re: [bug report] net: ethtool: Introduce per-PHY DUMP operations

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: <c9c6378a-14a3-440a-80b7-f48cdb06623b@bootlin.com>
Date: Fri, 6 Feb 2026 18:04:36 +0100
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: Simon Horman <horms@...nel.org>, netdev@...r.kernel.org,
 linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [bug report] net: ethtool: Introduce per-PHY DUMP operations

Hi Dan,

On 06/02/2026 14:38, Dan Carpenter wrote:
> [ Smatch checking is paused while we raise funding.  #SadFace
>   https://lore.kernel.org/all/aTaiGSbWZ9DJaGo7@stanley.mountain/ -dan ]
> 
> Hello Maxime Chevallier,
> 
> Commit 172265b44cd3 ("net: ethtool: Introduce per-PHY DUMP
> operations") from May 2, 2025 (linux-next), leads to the following
> Smatch static checker warning:
> 
> 	net/ethtool/netlink.c:714 ethnl_perphy_start()
> 	error: buffer overflow 'ethnl_default_requests' 52 <= 255 user_rl='0-255' uncapped
> 
> net/ethtool/netlink.c
>     700 static int ethnl_perphy_start(struct netlink_callback *cb)
>     701 {
>     702         struct ethnl_perphy_dump_ctx *phy_ctx = ethnl_perphy_dump_context(cb);
>     703         const struct genl_dumpit_info *info = genl_dumpit_info(cb);
>     704         struct ethnl_dump_ctx *ctx = &phy_ctx->ethnl_ctx;
>     705         struct ethnl_reply_data *reply_data;
>     706         const struct ethnl_request_ops *ops;
>     707         struct ethnl_req_info *req_info;
>     708         struct genlmsghdr *ghdr;
>     709         int ret;
>     710 
>     711         BUILD_BUG_ON(sizeof(*ctx) > sizeof(cb->ctx));
>     712 
>     713         ghdr = nlmsg_data(cb->nlh);
> --> 714         ops = ethnl_default_requests[ghdr->cmd];
> 
> Smatch thinks nlmsg_data() is untrusted data, so it could be out of bounds.
> It's a u8, but there are only 52 elements in the ethnl_default_requests[]
> array.

I see, then we also have the same problem in ethnl_default_start().

I'd expect the genl part to validate cmd (I haven't checked yet), but we
do have a WARN_ONCE just below for the case 'cmd' is wrong, so we could
definitely add some more sanity checks before accessing
ethnl_default_requests[].

I'll look further into that and send the relevant fixes :)

Thanks for the report,

Maxime