[PATCH net-next] net: ethtool: avoid OOB accesses in PAUSE_SET

[PATCH net-next] net: ethtool: avoid OOB accesses in PAUSE_SET

[Jakub Kicinski]

We now reuse .parse_request() from GET on SET, so we need to make sure
that the policies for both cover the attributes used for .parse_request().
genetlink will only allocate space in info->attrs for ARRAY_SIZE(policy).

Reported-by: syzbot+430f9f76633641a62217@syzkaller.appspotmail.com
Fixes: 963781bdfe20 ("net: ethtool: call .parse_request for SET handlers")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: andrew@lunn.ch
CC: maxime.chevallier@bootlin.com
---
 net/ethtool/netlink.h | 2 +-
 net/ethtool/pause.c   | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 373a8d5e86ae..94a7eb402022 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -467,7 +467,7 @@ extern const struct nla_policy ethnl_channels_set_policy[ETHTOOL_A_CHANNELS_COMB
 extern const struct nla_policy ethnl_coalesce_get_policy[ETHTOOL_A_COALESCE_HEADER + 1];
 extern const struct nla_policy ethnl_coalesce_set_policy[ETHTOOL_A_COALESCE_MAX + 1];
 extern const struct nla_policy ethnl_pause_get_policy[ETHTOOL_A_PAUSE_STATS_SRC + 1];
-extern const struct nla_policy ethnl_pause_set_policy[ETHTOOL_A_PAUSE_TX + 1];
+extern const struct nla_policy ethnl_pause_set_policy[ETHTOOL_A_PAUSE_STATS_SRC + 1];
 extern const struct nla_policy ethnl_eee_get_policy[ETHTOOL_A_EEE_HEADER + 1];
 extern const struct nla_policy ethnl_eee_set_policy[ETHTOOL_A_EEE_TX_LPI_TIMER + 1];
 extern const struct nla_policy ethnl_tsinfo_get_policy[ETHTOOL_A_TSINFO_MAX + 1];
diff --git a/net/ethtool/pause.c b/net/ethtool/pause.c
index f7c847aeb1a2..0f9af1e66548 100644
--- a/net/ethtool/pause.c
+++ b/net/ethtool/pause.c
@@ -168,6 +168,7 @@ const struct nla_policy ethnl_pause_set_policy[] = {
 	[ETHTOOL_A_PAUSE_AUTONEG]		= { .type = NLA_U8 },
 	[ETHTOOL_A_PAUSE_RX]			= { .type = NLA_U8 },
 	[ETHTOOL_A_PAUSE_TX]			= { .type = NLA_U8 },
+	[ETHTOOL_A_PAUSE_STATS_SRC]		= { .type = NLA_REJECT },
 };
 
 static int
-- 
2.50.0

Re: [PATCH net-next] net: ethtool: avoid OOB accesses in PAUSE_SET

[Ido Schimmel]

On Thu, Jun 26, 2025 at 04:39:26PM -0700, Jakub Kicinski wrote:
> We now reuse .parse_request() from GET on SET, so we need to make sure
> that the policies for both cover the attributes used for .parse_request().
> genetlink will only allocate space in info->attrs for ARRAY_SIZE(policy).
> 
> Reported-by: syzbot+430f9f76633641a62217@syzkaller.appspotmail.com
> Fixes: 963781bdfe20 ("net: ethtool: call .parse_request for SET handlers")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Tested-by: Ido Schimmel <idosch@nvidia.com>

Thanks, we hit that as well.

BTW, shouldn't you also release the reference from the net device if
ethnl_default_parse() fails in ethnl_default_set_doit()?

Re: [PATCH net-next] net: ethtool: avoid OOB accesses in PAUSE_SET

[Jakub Kicinski]

On Sun, 29 Jun 2025 13:04:35 +0300 Ido Schimmel wrote:
> On Thu, Jun 26, 2025 at 04:39:26PM -0700, Jakub Kicinski wrote:
> > We now reuse .parse_request() from GET on SET, so we need to make sure
> > that the policies for both cover the attributes used for .parse_request().
> > genetlink will only allocate space in info->attrs for ARRAY_SIZE(policy).
> > 
> > Reported-by: syzbot+430f9f76633641a62217@syzkaller.appspotmail.com
> > Fixes: 963781bdfe20 ("net: ethtool: call .parse_request for SET handlers")
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>  
> 
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> Tested-by: Ido Schimmel <idosch@nvidia.com>
> 
> Thanks, we hit that as well.
> 
> BTW, shouldn't you also release the reference from the net device if
> ethnl_default_parse() fails in ethnl_default_set_doit()?

Oof, good catch! Let me remove this trap of an API :S

Re: [PATCH net-next] net: ethtool: avoid OOB accesses in PAUSE_SET

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 26 Jun 2025 16:39:26 -0700 you wrote:
> We now reuse .parse_request() from GET on SET, so we need to make sure
> that the policies for both cover the attributes used for .parse_request().
> genetlink will only allocate space in info->attrs for ARRAY_SIZE(policy).
> 
> Reported-by: syzbot+430f9f76633641a62217@syzkaller.appspotmail.com
> Fixes: 963781bdfe20 ("net: ethtool: call .parse_request for SET handlers")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> 
> [...]

Here is the summary with links:
  - [net-next] net: ethtool: avoid OOB accesses in PAUSE_SET
    https://git.kernel.org/netdev/net-next/c/99e3eb454cc4

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html