netdev - Re: [PATCH net-next v2 1/3] openvswitch: allow specifying ifindex of new interfaces

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

Message-ID: <20220823121527.46efd279@kernel.org>
Date:   Tue, 23 Aug 2022 12:15:27 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Andrey Zhadchenko <andrey.zhadchenko@...tuozzo.com>
Cc:     netdev@...r.kernel.org, dev@...nvswitch.org, pshelar@....org,
        davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com,
        ptikhomirov@...tuozzo.com, alexander.mikhalitsyn@...tuozzo.com,
        avagin@...gle.com, brauner@...nel.org, mark.d.gray@...hat.com,
        i.maximets@....org, aconole@...hat.com
Subject: Re: [PATCH net-next v2 1/3] openvswitch: allow specifying ifindex
 of new interfaces

On Tue, 23 Aug 2022 16:50:21 +0300 Andrey Zhadchenko wrote:
> > Are you 100% sure that all user space making this call initializes
> > dp_ifindex to 0? There is no validation in the kernel today that
> > the field is not garbage as far as I can tell.
> > 
> > If you are sure, please add the appropriate analysis to the commit msg.  
> 
> I am not sure that *all* users set dp_ifindex to zero. At least I do not 
> know about them. Primary ovs userspace ovs-vswitchd certainly set to 
> zero, others like WeaveNet do it too. But there can be more?
> What is the policy regarding this? I can add a new attribute, it is not 
> hard.

IDK how many user space components driving OvS exist out there.
We could risk it and reuse the field, but if we get it wrong 
and don't catch the regression before the final release is cut 
the result gets _really_ ugly. We will have _some_ user space
out there expecting us to use ifindex and _some_ which expects
it can pass garbage, so we can neither revert not leave it...
If that makes sense.

If using / adding attribute is not a hassle that'd be my vote.

Using fixed structs like struct ovs_header is strongly discouraged
for new families exactly because of situations like this :(