[PATCH net] net: ethtool: fix wrong type used in struct kernel_ethtool_ts_info

[PATCH net] net: ethtool: fix wrong type used in struct kernel_ethtool_ts_info

[Russell King (Oracle)]

In C, enumerated types do not have a defined size, apart from being
compatible with one of the standard types. This allows an ABI /
compiler to choose the type of an enum depending on the values it
needs to store, and storing larger values in it can lead to undefined
behaviour.

The tx_type and rx_filters members of struct kernel_ethtool_ts_info
are defined as enumerated types, but are bit arrays, where each bit
is defined by the enumerated type. This means they typically store
values in excess of the maximum value of the enumerated type, in
fact (1 << max_value) and thus must not be declared using the
enumated type.

Fix both of these to use u32, as per the corresponding __u32 UAPI type.

Fixes: 2111375b85ad ("net: Add struct kernel_ethtool_ts_info")
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
 include/linux/ethtool.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index de5bd76a400c..d7d757e72554 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -856,8 +856,8 @@ struct kernel_ethtool_ts_info {
 	enum hwtstamp_provider_qualifier phc_qualifier;
 	enum hwtstamp_source phc_source;
 	int phc_phyindex;
-	enum hwtstamp_tx_types tx_types;
-	enum hwtstamp_rx_filters rx_filters;
+	u32 tx_types;
+	u32 rx_filters;
 };
 
 /**
-- 
2.47.3

Re: [PATCH net] net: ethtool: fix wrong type used in struct kernel_ethtool_ts_info

[Kory Maincent]

On Sun, 07 Sep 2025 21:43:20 +0100
"Russell King (Oracle)" <rmk+kernel@armlinux.org.uk> wrote:

> In C, enumerated types do not have a defined size, apart from being
> compatible with one of the standard types. This allows an ABI /
> compiler to choose the type of an enum depending on the values it
> needs to store, and storing larger values in it can lead to undefined
> behaviour.
> 
> The tx_type and rx_filters members of struct kernel_ethtool_ts_info
> are defined as enumerated types, but are bit arrays, where each bit
> is defined by the enumerated type. This means they typically store
> values in excess of the maximum value of the enumerated type, in
> fact (1 << max_value) and thus must not be declared using the
> enumated type.
> 
> Fix both of these to use u32, as per the corresponding __u32 UAPI type.

Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>

Thank you!
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

Re: [PATCH net] net: ethtool: fix wrong type used in struct kernel_ethtool_ts_info

[Jakub Kicinski]

On Sun, 07 Sep 2025 21:43:20 +0100 Russell King (Oracle) wrote:
> In C, enumerated types do not have a defined size, apart from being
> compatible with one of the standard types. This allows an ABI /
> compiler to choose the type of an enum depending on the values it
> needs to store, and storing larger values in it can lead to undefined
> behaviour.
> 
> The tx_type and rx_filters members of struct kernel_ethtool_ts_info
> are defined as enumerated types, but are bit arrays, where each bit
> is defined by the enumerated type. This means they typically store
> values in excess of the maximum value of the enumerated type, in
> fact (1 << max_value) and thus must not be declared using the
> enumated type.
> 
> Fix both of these to use u32, as per the corresponding __u32 UAPI type.
> 
> Fixes: 2111375b85ad ("net: Add struct kernel_ethtool_ts_info")

Do you feel strongly about this being a fix? (I can adjust when
applying FWIW). It's clearly not great but I don't think storing
a mask of enum values cause functional problems.

Re: [PATCH net] net: ethtool: fix wrong type used in struct kernel_ethtool_ts_info

[Russell King (Oracle)]

On Tue, Sep 09, 2025 at 04:33:02PM -0700, Jakub Kicinski wrote:
> On Sun, 07 Sep 2025 21:43:20 +0100 Russell King (Oracle) wrote:
> > In C, enumerated types do not have a defined size, apart from being
> > compatible with one of the standard types. This allows an ABI /
> > compiler to choose the type of an enum depending on the values it
> > needs to store, and storing larger values in it can lead to undefined
> > behaviour.
> > 
> > The tx_type and rx_filters members of struct kernel_ethtool_ts_info
> > are defined as enumerated types, but are bit arrays, where each bit
> > is defined by the enumerated type. This means they typically store
> > values in excess of the maximum value of the enumerated type, in
> > fact (1 << max_value) and thus must not be declared using the
> > enumated type.
> > 
> > Fix both of these to use u32, as per the corresponding __u32 UAPI type.
> > 
> > Fixes: 2111375b85ad ("net: Add struct kernel_ethtool_ts_info")
> 
> Do you feel strongly about this being a fix? (I can adjust when
> applying FWIW). It's clearly not great but I don't think storing
> a mask of enum values cause functional problems.

Whether or not it causes problems depends whether we have any compilers
that are used for the kernel which select the size of an enum type
based on the value.

From what I recall, when EABI for ARM was being talked about, that
compiler behaviour was certainly on the table. I opposed it - and
we ended up with arm-linux and arm-none variants of EABI. Whether
there's still a difference today, I'm not sure.

Even without Fixes: I think you'll find that the stable autosel bot
will still pick this change up... it uses "AI".

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH net] net: ethtool: fix wrong type used in struct kernel_ethtool_ts_info

[Jakub Kicinski]

On Wed, 10 Sep 2025 13:46:55 +0100 Russell King (Oracle) wrote:
> Even without Fixes: I think you'll find that the stable autosel bot
> will still pick this change up... it uses "AI".

yeah.. probably..

Re: [PATCH net] net: ethtool: fix wrong type used in struct kernel_ethtool_ts_info

[patchwork-bot+netdevbpf]

Hello:

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

On Sun, 07 Sep 2025 21:43:20 +0100 you wrote:
> In C, enumerated types do not have a defined size, apart from being
> compatible with one of the standard types. This allows an ABI /
> compiler to choose the type of an enum depending on the values it
> needs to store, and storing larger values in it can lead to undefined
> behaviour.
> 
> The tx_type and rx_filters members of struct kernel_ethtool_ts_info
> are defined as enumerated types, but are bit arrays, where each bit
> is defined by the enumerated type. This means they typically store
> values in excess of the maximum value of the enumerated type, in
> fact (1 << max_value) and thus must not be declared using the
> enumated type.
> 
> [...]

Here is the summary with links:
  - [net] net: ethtool: fix wrong type used in struct kernel_ethtool_ts_info
    https://git.kernel.org/netdev/net/c/6fef6ae764be

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