On Wed, Aug 06, 2025 at 05:29:55PM +0200, Alexander Lobakin wrote: > From: Nathan Chancellor <nat...@kernel.org>
Thank you for the fixes Nathan! I'll dig through these and get them sent out before I try to land this patch again -- "But COMPILE_TEST is never wrong!" ;) > > [...] > > descriptions expected to be stable once they are released or are we able > > Ethtool private stats are not "ABI" at all. Moreover, if they result in > incorrect code, this needs to be fixed no matter if someone already > wrote scripts dependent on these names or not. Internally there isn't an ABI, but the userspace interface effectively has an ABI: the strings are fixed-size and NUL-padded but not NUL-terminated. > > to adjust them? We could maybe shave an 'o' from 'unknown' to easily > > resolve this without losing much in the way of quick visual processing. > > I've no idea why it's popular to define Ethtool stats names in drivers > using a fixed array of ETH_GSTRING_LEN and then do memcpy(). The above is why: they are fixed-size, non-NUL-terminated strings, so many drivers use this memcpy pattern. But not all. > I've been always using just `const char * const[]` + strscpy() (then > switched the latter to ethtool_puts()/ethtool_sprintf() -- we even have > special helpers for that). In case some name goes past ETH_GSTRING_LEN, > it would just be truncated, but always have \0 at the end. Unfortunately this is not true: not all sources are NUL terminated. > Plus most of the names are shorter than 32, so defining such arrays of > 32 just wastes space in .rodata. That IS true, but many drivers just keep giant blocks of data they can memcpy. :( Regardless, I will double-check this and see what needs to happen here. I've fixed a lot of these already[1]. -Kees [1] https://lore.kernel.org/lkml/20250416010210.work.904-k...@kernel.org/ -- Kees Cook
