[PATCH net-ipsec v2] xfrm: replay: Fix ESN wrap around for GSO

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...")
Signed-off-by: Christian Langrock <redacted>
---
 include/net/xfrm.h     |  1 +
 net/xfrm/xfrm_output.c |  2 +-
 net/xfrm/xfrm_replay.c | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 6e8fa98f786f..49d6d974f493 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h

@@ -1749,6 +1749,7 @@ void xfrm_replay_advance(struct xfrm_state *x, __be32 net_seq);
 int xfrm_replay_check(struct xfrm_state *x, struct sk_buff *skb, __be32 net_seq);
 void xfrm_replay_notify(struct xfrm_state *x, int event);
 int xfrm_replay_overflow(struct xfrm_state *x, struct sk_buff *skb);
+int xfrm_replay_overflow_check(struct xfrm_state *x, struct sk_buff *skb);
 int xfrm_replay_recheck(struct xfrm_state *x, struct sk_buff *skb, __be32 net_seq);
 
 static inline int xfrm_aevent_is_on(struct net *net)

diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index 9a5e79a38c67..c470a68d9c88 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c

@@ -738,7 +738,7 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb)
 		skb->encapsulation = 1;
 
 		if (skb_is_gso(skb)) {
-			if (skb->inner_protocol)
+			if (skb->inner_protocol || xfrm_replay_overflow_check(x, skb))
 				return xfrm_output_gso(net, sk, skb);
 
 			skb_shinfo(skb)->gso_type |= SKB_GSO_ESP;

diff --git a/net/xfrm/xfrm_replay.c b/net/xfrm/xfrm_replay.c
index 9277d81b344c..991cfc7a091d 100644
--- a/net/xfrm/xfrm_replay.c
+++ b/net/xfrm/xfrm_replay.c

@@ -750,6 +750,34 @@ int xfrm_replay_overflow(struct xfrm_state *x, struct sk_buff *skb)
 
 	return xfrm_replay_overflow_offload(x, skb);
 }
+
+static bool xfrm_replay_overflow_check_offload_esn(struct xfrm_state *x, struct sk_buff *skb)
+{
+	struct xfrm_replay_state_esn *replay_esn = x->replay_esn;
+	__u32 oseq = replay_esn->oseq;
+	bool ret = false;
+
+	/* We assume that this function is called with
+	 * skb_is_gso(skb) == true
+	 */
+
+	if (x->type->flags & XFRM_TYPE_REPLAY_PROT) {
+		oseq = oseq + 1 + skb_shinfo(skb)->gso_segs;
+		if (unlikely(oseq < replay_esn->oseq))
+			ret = true;
+	}
+
+	return ret;
+}
+
+bool xfrm_replay_overflow_check(struct xfrm_state *x, struct sk_buff *skb)
+{
+	if (x->repl_mode == XFRM_REPLAY_MODE_ESN)
+		return xfrm_replay_overflow_check_offload_esn(x, skb);
+
+	return false;
+}
+
 #else
 int xfrm_replay_overflow(struct xfrm_state *x, struct sk_buff *skb)
 {

@@ -764,6 +792,11 @@ int xfrm_replay_overflow(struct xfrm_state *x, struct sk_buff *skb)
 
 	return __xfrm_replay_overflow(x, skb);
 }
+
+int xfrm_replay_overflow_check(struct xfrm_state *x, struct sk_buff *skb)
+{
+	return 0;
+}
 #endif
 
 int xfrm_init_replay(struct xfrm_state *x)

-- 
2.37.1.223.g6a475b71f8

On Tue, 27 Sep 2022 14:59:50 +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.

Does not build but please wait for reviews before reposting:

net/xfrm/xfrm_replay.c:773:6: error: conflicting types for ‘xfrm_replay_overflow_check’; have ‘bool(struct xfrm_state *, struct sk_buff *)’ {aka ‘_Bool(struct xfrm_state *, struct sk_buff *)’}
  773 | bool xfrm_replay_overflow_check(struct xfrm_state *x, struct sk_buff *skb)
      |      ^~~~~~~~~~~~~~~~~~~~~~~~~~

On Tue, Sep 27, 2022 at 02:59:50PM +0200, Christian Langrock wrote:

return true;

+	}
+
+	return ret;

return false;

+}
+
+bool xfrm_replay_overflow_check(struct xfrm_state *x, struct sk_buff *skb)
+{
+	if (x->repl_mode == XFRM_REPLAY_MODE_ESN)
+		return xfrm_replay_overflow_check_offload_esn(x, skb);
+
+	return false;
+}

I still think that functions above should be merged into one. This is
called only if xfrm_dev_offload_ok() success -> in crypto offload path.

Thanks

int -> bool

+{
+	return 0;
+}
 #endif
 
 int xfrm_init_replay(struct xfrm_state *x)
-- 
2.37.1.223.g6a475b71f8

On Tue, Sep 27, 2022 at 02:59:50PM +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...")
Signed-off-by: Christian Langrock <redacted>

Some minor nits:

This is already v3, not v2 as stated in the subject line.
Also, please explain the changes between the versions
(see 'git log' for examples).

The target tree is 'ipsec', not 'net-ipsec'.

Otherwise this is a fix for a real bug. So fix the build,
incorporate the review from Leon and send a v4.

Thanks!

Hi Christian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on klassert-ipsec/master]
[also build test ERROR on net-next/master net/master linus/master v6.0-rc7 next-20220927]
[cannot apply to klassert-ipsec-next/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Christian-Langrock/xfrm-replay-Fix-ESN-wrap-around-for-GSO/20220927-210053
base:   https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git master
config: x86_64-rhel-8.3-func
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/bb1661466e741ea02b82c9d57c6cee6cee07ba7e
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Christian-Langrock/xfrm-replay-Fix-ESN-wrap-around-for-GSO/20220927-210053
        git checkout bb1661466e741ea02b82c9d57c6cee6cee07ba7e
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash net/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot [off-list ref]

All errors (new ones prefixed by >>):

quoted

net/xfrm/xfrm_replay.c:773:6: error: conflicting types for 'xfrm_replay_overflow_check'; have 'bool(struct xfrm_state *, struct sk_buff *)' {aka '_Bool(struct xfrm_state *, struct sk_buff *)'}

     773 | bool xfrm_replay_overflow_check(struct xfrm_state *x, struct sk_buff *skb)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from net/xfrm/xfrm_replay.c:10:
   include/net/xfrm.h:1752:5: note: previous declaration of 'xfrm_replay_overflow_check' with type 'int(struct xfrm_state *, struct sk_buff *)'
    1752 | int xfrm_replay_overflow_check(struct xfrm_state *x, struct sk_buff *skb);
         |     ^~~~~~~~~~~~~~~~~~~~~~~~~~


vim +773 net/xfrm/xfrm_replay.c

   772	
 > 773	bool xfrm_replay_overflow_check(struct xfrm_state *x, struct sk_buff *skb)
   774	{
   775		if (x->repl_mode == XFRM_REPLAY_MODE_ESN)
   776			return xfrm_replay_overflow_check_offload_esn(x, skb);
   777	
   778		return false;
   779	}
   780	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Hi Christian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on klassert-ipsec/master]
[also build test ERROR on net-next/master net/master linus/master v6.0-rc7 next-20220927]
[cannot apply to klassert-ipsec-next/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Christian-Langrock/xfrm-replay-Fix-ESN-wrap-around-for-GSO/20220927-210053
base:   https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git master
config: i386-randconfig-a013
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/bb1661466e741ea02b82c9d57c6cee6cee07ba7e
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Christian-Langrock/xfrm-replay-Fix-ESN-wrap-around-for-GSO/20220927-210053
        git checkout bb1661466e741ea02b82c9d57c6cee6cee07ba7e
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash net/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot [off-list ref]

All errors (new ones prefixed by >>):

quoted

net/xfrm/xfrm_replay.c:773:6: error: conflicting types for 'xfrm_replay_overflow_check'

   bool xfrm_replay_overflow_check(struct xfrm_state *x, struct sk_buff *skb)
        ^
   include/net/xfrm.h:1752:5: note: previous declaration is here
   int xfrm_replay_overflow_check(struct xfrm_state *x, struct sk_buff *skb);
       ^
   1 error generated.


vim +/xfrm_replay_overflow_check +773 net/xfrm/xfrm_replay.c

   772	
 > 773	bool xfrm_replay_overflow_check(struct xfrm_state *x, struct sk_buff *skb)
   774	{
   775		if (x->repl_mode == XFRM_REPLAY_MODE_ESN)
   776			return xfrm_replay_overflow_check_offload_esn(x, skb);
   777	
   778		return false;
   779	}
   780	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp