netdev - Re: [PATCH ipsec v3 0/2] xfrm: fix uapi for the default policy

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: <20210915091908.GA11749@moon.secunet.de>
Date:   Wed, 15 Sep 2021 11:19:08 +0200
From:   Antony Antony <antony.antony@...unet.com>
To:     Nicolas Dichtel <nicolas.dichtel@...nd.com>
CC:     <steffen.klassert@...unet.com>, <davem@...emloft.net>,
        <kuba@...nel.org>, <antony.antony@...unet.com>,
        <netdev@...r.kernel.org>
Subject: Re: [PATCH ipsec v3 0/2] xfrm: fix uapi for the default policy

Hi Nicolas,

On Tue, Sep 14, 2021 at 16:46:32 +0200, Nicolas Dichtel wrote:
> This feature has just been merged after the last release, thus it's still
> time to fix the uapi.
> As stated in the thread, the uapi is based on some magic values (from the
> userland POV).

I like your proposal to make uapi 3 different variables, instead of flags.

This fix leave kernel internal representation as a flags in
struct netns_xfrm
	u8 policy_default;

I have a concern. If your patch is applied, the uapi and xfrm internal representations would be inconsistant. I think they should be the same in this case.
It would easier to follow the code path.
On the other hand we should apply this uapi change ASAP, in 5.15 release cycle, to avoid ABI change.

Could you also change xfrm policy_default to three variables?

> Here is a proposal to simplify this uapi and make it clear how to use it.
> The other problem was the notification: changing the default policy may
> radically change the packets flows.
> 
> v2 -> v3: rebase on top of ipsec tree
> 
> v1 -> v2: fix warnings reported by the kernel test robot
>