netdev - Re: [PATCH 2/2][next] net: ethtool: Avoid thousands of -Wflex-array-member-not-at-end warnings

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0bc27725-cd55-493b-8844-ee2c5baca5f0@embeddedor.com>
Date: Mon, 28 Oct 2024 20:37:13 -0600
From: "Gustavo A. R. Silva" <gustavo@...eddedor.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>,
 Michael Chan <michael.chan@...adcom.com>, Andrew Lunn
 <andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
 Potnuri Bharat Teja <bharat@...lsio.com>,
 Christian Benvenuti <benve@...co.com>, Satish Kharat <satishkh@...co.com>,
 Manish Chopra <manishc@...vell.com>, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH 2/2][next] net: ethtool: Avoid thousands of
 -Wflex-array-member-not-at-end warnings



On 28/10/24 18:32, Jakub Kicinski wrote:
> On Mon, 28 Oct 2024 17:32:53 -0600 Gustavo A. R. Silva wrote:
>>>> Additionally, update the type of some variables in various functions
>>>> that don't access the flexible-array member, changing them to the
>>>> newly created `struct ethtool_link_settings_hdr`.
>>>
>>> Why? Please avoid unnecessary code changes.
>>
>> This is actually necessary. As the type of the conflicting middle members
>> changed, those instances that expect the type to be `struct ethtool_link_settings`
>> should be adjusted to the new type. Another option is to leave the type
>> unchanged and instead use container_of. See below.
> 
> Ah, that makes sense. So they need to be included int the newly split
> patch. Please rephrase the commit message a bit, the current paragraph
> reads as if this was a code cleanup.

After double-checking, it turns out that the patch ends up being basically
the same. The only change that would be split in a separate patch would be
the following.

diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c
index 5cc131cdb1bc..7da94e26ced6 100644
--- a/net/ethtool/ioctl.c
+++ b/net/ethtool/ioctl.c
@@ -425,7 +425,7 @@ convert_link_ksettings_to_legacy_settings(

  /* layout of the struct passed from/to userland */
  struct ethtool_link_usettings {
-       struct ethtool_link_settings base;
+       struct ethtool_link_settings_hdr base;
         struct {
                 __u32 supported[__ETHTOOL_LINK_MODE_MASK_NU32];
                 __u32 advertising[__ETHTOOL_LINK_MODE_MASK_NU32];

The rest will essentially remain the same as the change in
include/linux/ethtool.h triggers a cascade of changes across
the rest of the files in this patch.

So, you tell me if you still want me to split this patch. In any case
I'll update the changelog text.

Thanks
--
Gustavo