netdev - Re: [PATCH net] net: ethtool: fix wrong type used in struct kernel_ethtool_ts

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aMFzP6GFj1jVO6Qs@shell.armlinux.org.uk>
Date: Wed, 10 Sep 2025 13:46:55 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Kory Maincent <kory.maincent@...tlin.com>,
	Alexandra Winter <wintera@...ux.ibm.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, netdev@...r.kernel.org,
	Paolo Abeni <pabeni@...hat.com>, Shannon Nelson <sln@...main.com>,
	Simon Horman <horms@...nel.org>
Subject: Re: [PATCH net] net: ethtool: fix wrong type used in struct
 kernel_ethtool_ts_info

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!