netdev - Re: [PATCH ipsec v6] xfrm: replay: Fix ESN wrap around for GSO

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, 13 Oct 2022 08:16:33 +0200
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Herbert Xu <herbert@...dor.apana.org.au>
CC:     Christian Langrock <christian.langrock@...unet.com>,
        <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
        <pabeni@...hat.com>, <netdev@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH ipsec v6] xfrm: replay: Fix ESN wrap around for GSO

On Wed, Oct 12, 2022 at 05:28:57PM +0800, Herbert Xu wrote:
> On Fri, Oct 07, 2022 at 04:50:15PM +0200, Christian Langrock wrote:
> > When using GSO it can happen that the wrong seq_hi is used for the last
> > packets before the wrap around. This can lead to double usage of a
> > sequence number. To avoid this, we should serialize this last GSO
> > packet.
> > 
> > Fixes: d7dbefc45cf5 ("xfrm: Add xfrm_replay_overflow functions for offloading")
> > Co-developed-by: Steffen Klassert <steffen.klassert@...unet.com>
> > Signed-off-by: Christian Langrock <christian.langrock@...unet.com>
> > ---
> > Changes in v6:
> >  - move overflow check to offloading path to avoid locking issues
> > 
> > Changes in v5:
> >  - Fix build
> > 
> > Changes in v4:
> >  - move changelog within comment
> >  - add reviewer
> > 
> > Changes in v3:
> > - fix build
> > - remove wrapper function
> > 
> > Changes in v2:
> > - switch to bool as return value
> > - remove switch case in wrapper function
> > ---
> >  net/ipv4/esp4_offload.c |  3 +++
> >  net/ipv6/esp6_offload.c |  3 +++
> >  net/xfrm/xfrm_device.c  | 15 ++++++++++++++-
> >  net/xfrm/xfrm_replay.c  |  2 +-
> >  4 files changed, 21 insertions(+), 2 deletions(-)
> 
> Could you please explain how this code restructure makes it safe
> with respect to multiple users of the same xfrm_state?

That is because with this patch, the sequence number from the xfrm_state
is assigned to the skb and advanced by the number of segments while
holding the state lock, as it was before. The sequence numbers this
patch operates on are exclusive and private to that skb (and its
segments). The next skb will checkout the correct number from the
xfrm_state regardless on which cpu it comes.