netdev - Re: [PATCH ipsec-next] xfrm: delete not-used XFRM_OFFLOAD

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]

Date:   Thu, 3 Feb 2022 07:49:40 +0100
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Leon Romanovsky <leon@...nel.org>
CC:     "David S . Miller" <davem@...emloft.net>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        <netdev@...r.kernel.org>
Subject: Re: [PATCH ipsec-next] xfrm: delete not-used XFRM_OFFLOAD_IPV6 define

On Tue, Feb 01, 2022 at 09:22:17AM +0200, Leon Romanovsky wrote:
> On Tue, Feb 01, 2022 at 07:58:36AM +0100, Steffen Klassert wrote:
> > On Thu, Jan 27, 2022 at 08:24:58PM +0200, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@...dia.com>
> > > 
> > > XFRM_OFFLOAD_IPV6 define was exposed in the commit mentioned in the
> > > fixes line, but it is never been used both in the kernel and in the
> > > user space. So delete it.
> > 
> > How can you be sure that is is not used in userspace? At least some
> > versions of strongswan set that flag. So even if it is meaningless
> > in the kernel, we can't remove it.
> 
> I looked over all net/* and include/uapi/* code with "git log -p" and didn't
> see any use of this flag ever. 
> 
> Looking in strongsswan, I see that they bring kernel header files [1] for the build
> and removal won't break build of old strongsswan versions.
> 
> We just can't use this bit anymore, because of this commit [2]. I have
> no clue why it was used there.
> 
> So yes, we can remove, but worth to add a comment about old strongsswan.

It is always problematic to remove something that was exposed
to userspace by the uapi, that should not happen. We can't
know that nobody uses that, even if unlikely. So please keep
it and maybe mark it as unused with a comment.

> 
> And if we are talking about xfrm_user_offload flags, there is a
> well-known API mistake in xfrm_dev_state_add() of not checking validity
> of flags. So *theoretically* we can find some software in the wild that
> uses other bits too.
> 
> I would like to see it is fixed.

Yes, catching usage of undefined flags should be indeed implemented.

Thanks!