netdev - Re: [pull request][net-next V2 00/15] mlx5 updates 2023-10-19

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]

Date: Thu, 26 Oct 2023 10:29:58 +0300
From: Leon Romanovsky <leon@...nel.org>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Saeed Mahameed <saeed@...nel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Paolo Abeni <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>,
	Saeed Mahameed <saeedm@...dia.com>, netdev@...r.kernel.org,
	Tariq Toukan <tariqt@...dia.com>
Subject: Re: [pull request][net-next V2 00/15] mlx5 updates 2023-10-19

On Wed, Oct 25, 2023 at 06:25:02PM -0700, Jakub Kicinski wrote:
> On Wed, 25 Oct 2023 11:52:02 +0300 Leon Romanovsky wrote:
> > This patch won't fix much without following patch in that series.
> > https://lore.kernel.org/all/20231021064620.87397-8-saeed@kernel.org/
> > 
> > Yes, users will see their replay window correctly through "ip xfrm state"
> > command, so this is why it has Fixes line, but it won't change anything
> > in the actual behavior without patch 7 and this is the reason why it was
> > sent to net-next.
> 
> Odd ordering of patches, anyone doing backports would totally miss it.
> Neither does the commit message explain the situation nor is it
> possible to grok that fact from the ("pass it to FW") code :(

This is why these patches are bundled together as one series.

> 
> > From patch 3:
> >  Users can configure IPsec replay window size, but mlx5 driver didn't
> >  honor their choice and set always 32bits.
> > 
> > From patch 7:
> >  After IPsec decryption it isn't enough to only check the IPsec syndrome
> >  but need to also check the ASO syndrome in order to verify that the
> >  operation was actually successful.
> 
> Hm, patch 7 looks like an independent but related fix to my uneducated
> eye, should it also have a Fixes tag?
> 
> Is patch 7 needed regardless of what choice of (previously ignored)
> parameters user makes?

Yes, I missed register check and without that replay protection feature
didn't work as one would expect, so it is needed anyway. The patch is
large, complex (IMHO not for -rc7) and I agree with Saeed that "missed register"
sounds like "missed feature".

> One way to deal with the problems from patches 
> 3 and 4 could be to reject the de facto unsupported configurations.
> But if the supported config also doesn't check the "syndrome" correctly
> in all cases, that's no bueno..

Patch 4 was discovered before replay window testing, when we run HW vs.
SW interoperability testing. The feature of setting seq/oseq is orthogonal
to replay window.

Thanks

>