All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/2] Fix issues in xfrm_migrate
@ 2022-01-06  0:52 Yan Yan
  2022-01-06  0:52 ` [PATCH v1 1/2] xfrm: Check if_id " Yan Yan
  2022-01-06  0:52 ` [PATCH v1 2/2] xfrm: Fix xfrm migrate issues when address family changes Yan Yan
  0 siblings, 2 replies; 15+ messages in thread
From: Yan Yan @ 2022-01-06  0:52 UTC (permalink / raw)
  To: Steffen Klassert
  Cc: netdev, Herbert Xu, David S . Miller, Jakub Kicinski, lorenzo,
	maze, nharold, benedictwong, Yan Yan

This patch series include two patches to fix two issues in xfrm_migrate.

PATCH 1/2 enables distinguishing SAs and SPs based on if_id during the
xfrm_migrate flow. It fixes the problem that when there are multiple
existing SPs with the same direction, the same xfrm_selector and
different endpoint addresses, xfrm_migrate might fail.

PATCH 2/2 enables xfrm_migrate to handle address family change by
breaking the original xfrm_state_clone method into two steps so as to
update the props.family before running xfrm_init_state.

Yan Yan (2):
  xfrm: Check if_id in xfrm_migrate
  xfrm: Fix xfrm migrate issues when address family changes

 include/net/xfrm.h     |  5 +++--
 net/key/af_key.c       |  2 +-
 net/xfrm/xfrm_policy.c | 14 ++++++++------
 net/xfrm/xfrm_state.c  | 40 ++++++++++++++++++++++++++++------------
 net/xfrm/xfrm_user.c   |  6 +++++-
 5 files changed, 45 insertions(+), 22 deletions(-)


base-commit: 18343b80691560f41c3339119a2e9314d4672c77
-- 
2.34.1.448.ga2b2bfdf31-goog


^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH v1 1/2] xfrm: Check if_id in xfrm_migrate
  2022-01-06  0:52 [PATCH v1 0/2] Fix issues in xfrm_migrate Yan Yan
@ 2022-01-06  0:52 ` Yan Yan
  2022-01-06 12:22   ` kernel test robot
  2022-01-06 15:55     ` kernel test robot
  2022-01-06  0:52 ` [PATCH v1 2/2] xfrm: Fix xfrm migrate issues when address family changes Yan Yan
  1 sibling, 2 replies; 15+ messages in thread
From: Yan Yan @ 2022-01-06  0:52 UTC (permalink / raw)
  To: Steffen Klassert
  Cc: netdev, Herbert Xu, David S . Miller, Jakub Kicinski, lorenzo,
	maze, nharold, benedictwong, Yan Yan

This patch enables distinguishing SAs and SPs based on if_id during
the xfrm_migrate flow. This ensures support for xfrm interfaces
throughout the SA/SP lifecycle.

When there are multiple existing SPs with the same direction,
the same xfrm_selector and different endpoint addresses,
xfrm_migrate might fail with ENODATA.

Specifically, the code path for performing xfrm_migrate is:
  Stage 1: find policy to migrate with
    xfrm_migrate_policy_find(sel, dir, type, net)
  Stage 2: find and update state(s) with
    xfrm_migrate_state_find(mp, net)
  Stage 3: update endpoint address(es) of template(s) with
    xfrm_policy_migrate(pol, m, num_migrate)

Currently "Stage 1" always returns the first xfrm_policy that
matches, and "Stage 3" looks for the xfrm_tmpl that matches the
old endpoint address. Thus if there are multiple xfrm_policy
with same selector, direction, type and net, "Stage 1" might
rertun a wrong xfrm_policy and "Stage 3" will fail with ENODATA
because it cannot find a xfrm_tmpl with the matching endpoint
address.

The fix is to allow userspace to pass an if_id and add if_id
to the matching rule in Stage 1 and Stage 2 since if_id is a
unique ID for xfrm_policy and xfrm_state. For compatibility,
if_id will only be checked if the attribute is set.

Tested with additions to Android's kernel unit test suite:
https://android-review.googlesource.com/c/kernel/tests/+/1668886

Signed-off-by: Yan Yan <evitayan@google.com>
---
 include/net/xfrm.h     |  5 +++--
 net/key/af_key.c       |  2 +-
 net/xfrm/xfrm_policy.c | 14 ++++++++------
 net/xfrm/xfrm_state.c  |  7 ++++++-
 net/xfrm/xfrm_user.c   |  6 +++++-
 5 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 83b46da8873d..faee50259fd4 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1676,14 +1676,15 @@ int km_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 	       const struct xfrm_migrate *m, int num_bundles,
 	       const struct xfrm_kmaddress *k,
 	       const struct xfrm_encap_tmpl *encap);
-struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *net);
+struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *net,
+						u32 if_id);
 struct xfrm_state *xfrm_state_migrate(struct xfrm_state *x,
 				      struct xfrm_migrate *m,
 				      struct xfrm_encap_tmpl *encap);
 int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 		 struct xfrm_migrate *m, int num_bundles,
 		 struct xfrm_kmaddress *k, struct net *net,
-		 struct xfrm_encap_tmpl *encap);
+		 struct xfrm_encap_tmpl *encap, u32 if_id);
 #endif
 
 int km_new_mapping(struct xfrm_state *x, xfrm_address_t *ipaddr, __be16 sport);
diff --git a/net/key/af_key.c b/net/key/af_key.c
index de24a7d474df..9bf52a09b5ff 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -2623,7 +2623,7 @@ static int pfkey_migrate(struct sock *sk, struct sk_buff *skb,
 	}
 
 	return xfrm_migrate(&sel, dir, XFRM_POLICY_TYPE_MAIN, m, i,
-			    kma ? &k : NULL, net, NULL);
+			    kma ? &k : NULL, net, NULL, 0);
 
  out:
 	return err;
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 1a06585022ab..cd656b4ec5b9 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -4235,7 +4235,7 @@ static bool xfrm_migrate_selector_match(const struct xfrm_selector *sel_cmp,
 }
 
 static struct xfrm_policy *xfrm_migrate_policy_find(const struct xfrm_selector *sel,
-						    u8 dir, u8 type, struct net *net)
+						    u8 dir, u8 type, struct net *net, u32 if_id)
 {
 	struct xfrm_policy *pol, *ret = NULL;
 	struct hlist_head *chain;
@@ -4244,7 +4244,8 @@ static struct xfrm_policy *xfrm_migrate_policy_find(const struct xfrm_selector *
 	spin_lock_bh(&net->xfrm.xfrm_policy_lock);
 	chain = policy_hash_direct(net, &sel->daddr, &sel->saddr, sel->family, dir);
 	hlist_for_each_entry(pol, chain, bydst) {
-		if (xfrm_migrate_selector_match(sel, &pol->selector) &&
+		if ((if_id == 0 || pol->if_id == if_id) &&
+		    xfrm_migrate_selector_match(sel, &pol->selector) &&
 		    pol->type == type) {
 			ret = pol;
 			priority = ret->priority;
@@ -4256,7 +4257,8 @@ static struct xfrm_policy *xfrm_migrate_policy_find(const struct xfrm_selector *
 		if ((pol->priority >= priority) && ret)
 			break;
 
-		if (xfrm_migrate_selector_match(sel, &pol->selector) &&
+		if ((if_id == 0 || pol->if_id == if_id) &&
+		    xfrm_migrate_selector_match(sel, &pol->selector) &&
 		    pol->type == type) {
 			ret = pol;
 			break;
@@ -4372,7 +4374,7 @@ static int xfrm_migrate_check(const struct xfrm_migrate *m, int num_migrate)
 int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 		 struct xfrm_migrate *m, int num_migrate,
 		 struct xfrm_kmaddress *k, struct net *net,
-		 struct xfrm_encap_tmpl *encap)
+		 struct xfrm_encap_tmpl *encap, u32 if_id)
 {
 	int i, err, nx_cur = 0, nx_new = 0;
 	struct xfrm_policy *pol = NULL;
@@ -4391,14 +4393,14 @@ int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 	}
 
 	/* Stage 1 - find policy */
-	if ((pol = xfrm_migrate_policy_find(sel, dir, type, net)) == NULL) {
+	if ((pol = xfrm_migrate_policy_find(sel, dir, type, net, if_id)) == NULL) {
 		err = -ENOENT;
 		goto out;
 	}
 
 	/* Stage 2 - find and update state(s) */
 	for (i = 0, mp = m; i < num_migrate; i++, mp++) {
-		if ((x = xfrm_migrate_state_find(mp, net))) {
+		if (x = xfrm_migrate_state_find(mp, net, if_id)) {
 			x_cur[nx_cur] = x;
 			nx_cur++;
 			xc = xfrm_state_migrate(x, mp, encap);
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 0407272a990c..f52767685b13 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1603,7 +1603,8 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig,
 	return NULL;
 }
 
-struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *net)
+struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *net,
+						u32 if_id)
 {
 	unsigned int h;
 	struct xfrm_state *x = NULL;
@@ -1619,6 +1620,8 @@ struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *n
 				continue;
 			if (m->reqid && x->props.reqid != m->reqid)
 				continue;
+			if (if_id != 0 && x->if_id != if_id)
+				continue;
 			if (!xfrm_addr_equal(&x->id.daddr, &m->old_daddr,
 					     m->old_family) ||
 			    !xfrm_addr_equal(&x->props.saddr, &m->old_saddr,
@@ -1634,6 +1637,8 @@ struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *n
 			if (x->props.mode != m->mode ||
 			    x->id.proto != m->proto)
 				continue;
+			if (if_id != 0 && x->if_id != if_id)
+				continue;
 			if (!xfrm_addr_equal(&x->id.daddr, &m->old_daddr,
 					     m->old_family) ||
 			    !xfrm_addr_equal(&x->props.saddr, &m->old_saddr,
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index e3e26f4da6c2..eda02c5544ae 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2580,6 +2580,7 @@ static int xfrm_do_migrate(struct sk_buff *skb, struct nlmsghdr *nlh,
 	int n = 0;
 	struct net *net = sock_net(skb->sk);
 	struct xfrm_encap_tmpl  *encap = NULL;
+	u32 if_id = 0;
 
 	if (attrs[XFRMA_MIGRATE] == NULL)
 		return -EINVAL;
@@ -2604,7 +2605,10 @@ static int xfrm_do_migrate(struct sk_buff *skb, struct nlmsghdr *nlh,
 			return -ENOMEM;
 	}
 
-	err = xfrm_migrate(&pi->sel, pi->dir, type, m, n, kmp, net, encap);
+	if (attrs[XFRMA_IF_ID])
+		if_id = nla_get_u32(attrs[XFRMA_IF_ID]);
+
+	err = xfrm_migrate(&pi->sel, pi->dir, type, m, n, kmp, net, encap, if_id);
 
 	kfree(encap);
 
-- 
2.34.1.448.ga2b2bfdf31-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH v1 2/2] xfrm: Fix xfrm migrate issues when address family changes
  2022-01-06  0:52 [PATCH v1 0/2] Fix issues in xfrm_migrate Yan Yan
  2022-01-06  0:52 ` [PATCH v1 1/2] xfrm: Check if_id " Yan Yan
@ 2022-01-06  0:52 ` Yan Yan
  2022-01-12  7:57   ` Steffen Klassert
  1 sibling, 1 reply; 15+ messages in thread
From: Yan Yan @ 2022-01-06  0:52 UTC (permalink / raw)
  To: Steffen Klassert
  Cc: netdev, Herbert Xu, David S . Miller, Jakub Kicinski, lorenzo,
	maze, nharold, benedictwong, Yan Yan

xfrm_migrate cannot handle address family change of an xfrm_state.
The symptons are the xfrm_state will be migrated to a wrong address,
and sending as well as receiving packets wil be broken.

This commit fixes it by breaking the original xfrm_state_clone
method into two steps so as to update the props.family before
running xfrm_init_state. As the result, xfrm_state's inner mode,
outer mode, type and IP header length in xfrm_state_migrate can
be updated with the new address family.

Tested with additions to Android's kernel unit test suite:
https://android-review.googlesource.com/c/kernel/tests/+/1885354

Signed-off-by: Yan Yan <evitayan@google.com>
---
 net/xfrm/xfrm_state.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index f52767685b13..cac212039bf9 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1504,8 +1504,8 @@ static inline int clone_security(struct xfrm_state *x, struct xfrm_sec_ctx *secu
 	return 0;
 }
 
-static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig,
-					   struct xfrm_encap_tmpl *encap)
+static struct xfrm_state *xfrm_state_clone1(struct xfrm_state *orig,
+					    struct xfrm_encap_tmpl *encap)
 {
 	struct net *net = xs_net(orig);
 	struct xfrm_state *x = xfrm_state_alloc(net);
@@ -1579,8 +1579,20 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig,
 	memcpy(&x->mark, &orig->mark, sizeof(x->mark));
 	memcpy(&x->props.smark, &orig->props.smark, sizeof(x->props.smark));
 
-	if (xfrm_init_state(x) < 0)
-		goto error;
+	return x;
+
+ error:
+	xfrm_state_put(x);
+out:
+	return NULL;
+}
+
+static int xfrm_state_clone2(struct xfrm_state *orig, struct xfrm_state *x)
+{
+	int err = xfrm_init_state(x);
+
+	if (err < 0)
+		return err;
 
 	x->props.flags = orig->props.flags;
 	x->props.extra_flags = orig->props.extra_flags;
@@ -1595,12 +1607,7 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig,
 	x->replay = orig->replay;
 	x->preplay = orig->preplay;
 
-	return x;
-
- error:
-	xfrm_state_put(x);
-out:
-	return NULL;
+	return 0;
 }
 
 struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *net,
@@ -1661,10 +1668,14 @@ struct xfrm_state *xfrm_state_migrate(struct xfrm_state *x,
 {
 	struct xfrm_state *xc;
 
-	xc = xfrm_state_clone(x, encap);
+	xc = xfrm_state_clone1(x, encap);
 	if (!xc)
 		return NULL;
 
+	xc->props.family = m->new_family;
+	if (xfrm_state_clone2(x, xc) < 0)
+		goto error;
+
 	memcpy(&xc->id.daddr, &m->new_daddr, sizeof(xc->id.daddr));
 	memcpy(&xc->props.saddr, &m->new_saddr, sizeof(xc->props.saddr));
 
-- 
2.34.1.448.ga2b2bfdf31-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [PATCH v1 1/2] xfrm: Check if_id in xfrm_migrate
  2022-01-06  0:52 ` [PATCH v1 1/2] xfrm: Check if_id " Yan Yan
@ 2022-01-06 12:22   ` kernel test robot
  2022-01-06 15:55     ` kernel test robot
  1 sibling, 0 replies; 15+ messages in thread
From: kernel test robot @ 2022-01-06 12:22 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2870 bytes --]

Hi Yan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on 18343b80691560f41c3339119a2e9314d4672c77]

url:    https://github.com/0day-ci/linux/commits/Yan-Yan/Fix-issues-in-xfrm_migrate/20220106-085707
base:   18343b80691560f41c3339119a2e9314d4672c77
config: arc-randconfig-r032-20220106 (https://download.01.org/0day-ci/archive/20220106/202201062019.VFveWcCu-lkp(a)intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
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/0day-ci/linux/commit/2be451f5a66efd5951102e188daee7c730f55dc7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yan-Yan/Fix-issues-in-xfrm_migrate/20220106-085707
        git checkout 2be451f5a66efd5951102e188daee7c730f55dc7
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash net/xfrm/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   net/xfrm/xfrm_policy.c: In function 'xfrm_migrate':
>> net/xfrm/xfrm_policy.c:4403:21: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
    4403 |                 if (x = xfrm_migrate_state_find(mp, net, if_id)) {
         |                     ^


vim +4403 net/xfrm/xfrm_policy.c

  4373	
  4374	int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
  4375			 struct xfrm_migrate *m, int num_migrate,
  4376			 struct xfrm_kmaddress *k, struct net *net,
  4377			 struct xfrm_encap_tmpl *encap, u32 if_id)
  4378	{
  4379		int i, err, nx_cur = 0, nx_new = 0;
  4380		struct xfrm_policy *pol = NULL;
  4381		struct xfrm_state *x, *xc;
  4382		struct xfrm_state *x_cur[XFRM_MAX_DEPTH];
  4383		struct xfrm_state *x_new[XFRM_MAX_DEPTH];
  4384		struct xfrm_migrate *mp;
  4385	
  4386		/* Stage 0 - sanity checks */
  4387		if ((err = xfrm_migrate_check(m, num_migrate)) < 0)
  4388			goto out;
  4389	
  4390		if (dir >= XFRM_POLICY_MAX) {
  4391			err = -EINVAL;
  4392			goto out;
  4393		}
  4394	
  4395		/* Stage 1 - find policy */
  4396		if ((pol = xfrm_migrate_policy_find(sel, dir, type, net, if_id)) == NULL) {
  4397			err = -ENOENT;
  4398			goto out;
  4399		}
  4400	
  4401		/* Stage 2 - find and update state(s) */
  4402		for (i = 0, mp = m; i < num_migrate; i++, mp++) {
> 4403			if (x = xfrm_migrate_state_find(mp, net, if_id)) {

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v1 1/2] xfrm: Check if_id in xfrm_migrate
  2022-01-06  0:52 ` [PATCH v1 1/2] xfrm: Check if_id " Yan Yan
@ 2022-01-06 15:55     ` kernel test robot
  2022-01-06 15:55     ` kernel test robot
  1 sibling, 0 replies; 15+ messages in thread
From: kernel test robot @ 2022-01-06 15:55 UTC (permalink / raw)
  To: Yan Yan; +Cc: llvm, kbuild-all

Hi Yan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on 18343b80691560f41c3339119a2e9314d4672c77]

url:    https://github.com/0day-ci/linux/commits/Yan-Yan/Fix-issues-in-xfrm_migrate/20220106-085707
base:   18343b80691560f41c3339119a2e9314d4672c77
config: riscv-randconfig-r002-20220106 (https://download.01.org/0day-ci/archive/20220106/202201062313.QMYcwsyC-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project ca7ffe09dc6e525109e3cd570cc5182ce568be13)
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
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/2be451f5a66efd5951102e188daee7c730f55dc7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yan-Yan/Fix-issues-in-xfrm_migrate/20220106-085707
        git checkout 2be451f5a66efd5951102e188daee7c730f55dc7
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash net/xfrm/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from net/xfrm/xfrm_policy.c:24:
   In file included from include/linux/netdevice.h:37:
   In file included from include/net/net_namespace.h:40:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from net/xfrm/xfrm_policy.c:24:
   In file included from include/linux/netdevice.h:37:
   In file included from include/net/net_namespace.h:40:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from net/xfrm/xfrm_policy.c:24:
   In file included from include/linux/netdevice.h:37:
   In file included from include/net/net_namespace.h:40:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:1024:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
                                                     ~~~~~~~~~~ ^
>> net/xfrm/xfrm_policy.c:4403:9: warning: using the result of an assignment as a condition without parentheses [-Wparentheses]
                   if (x = xfrm_migrate_state_find(mp, net, if_id)) {
                       ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/xfrm/xfrm_policy.c:4403:9: note: place parentheses around the assignment to silence this warning
                   if (x = xfrm_migrate_state_find(mp, net, if_id)) {
                         ^
                       (                                          )
   net/xfrm/xfrm_policy.c:4403:9: note: use '==' to turn this assignment into an equality comparison
                   if (x = xfrm_migrate_state_find(mp, net, if_id)) {
                         ^
                         ==
   8 warnings generated.


vim +4403 net/xfrm/xfrm_policy.c

  4373	
  4374	int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
  4375			 struct xfrm_migrate *m, int num_migrate,
  4376			 struct xfrm_kmaddress *k, struct net *net,
  4377			 struct xfrm_encap_tmpl *encap, u32 if_id)
  4378	{
  4379		int i, err, nx_cur = 0, nx_new = 0;
  4380		struct xfrm_policy *pol = NULL;
  4381		struct xfrm_state *x, *xc;
  4382		struct xfrm_state *x_cur[XFRM_MAX_DEPTH];
  4383		struct xfrm_state *x_new[XFRM_MAX_DEPTH];
  4384		struct xfrm_migrate *mp;
  4385	
  4386		/* Stage 0 - sanity checks */
  4387		if ((err = xfrm_migrate_check(m, num_migrate)) < 0)
  4388			goto out;
  4389	
  4390		if (dir >= XFRM_POLICY_MAX) {
  4391			err = -EINVAL;
  4392			goto out;
  4393		}
  4394	
  4395		/* Stage 1 - find policy */
  4396		if ((pol = xfrm_migrate_policy_find(sel, dir, type, net, if_id)) == NULL) {
  4397			err = -ENOENT;
  4398			goto out;
  4399		}
  4400	
  4401		/* Stage 2 - find and update state(s) */
  4402		for (i = 0, mp = m; i < num_migrate; i++, mp++) {
> 4403			if (x = xfrm_migrate_state_find(mp, net, if_id)) {

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v1 1/2] xfrm: Check if_id in xfrm_migrate
@ 2022-01-06 15:55     ` kernel test robot
  0 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2022-01-06 15:55 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 7961 bytes --]

Hi Yan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on 18343b80691560f41c3339119a2e9314d4672c77]

url:    https://github.com/0day-ci/linux/commits/Yan-Yan/Fix-issues-in-xfrm_migrate/20220106-085707
base:   18343b80691560f41c3339119a2e9314d4672c77
config: riscv-randconfig-r002-20220106 (https://download.01.org/0day-ci/archive/20220106/202201062313.QMYcwsyC-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project ca7ffe09dc6e525109e3cd570cc5182ce568be13)
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
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/2be451f5a66efd5951102e188daee7c730f55dc7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yan-Yan/Fix-issues-in-xfrm_migrate/20220106-085707
        git checkout 2be451f5a66efd5951102e188daee7c730f55dc7
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash net/xfrm/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from net/xfrm/xfrm_policy.c:24:
   In file included from include/linux/netdevice.h:37:
   In file included from include/net/net_namespace.h:40:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from net/xfrm/xfrm_policy.c:24:
   In file included from include/linux/netdevice.h:37:
   In file included from include/net/net_namespace.h:40:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from net/xfrm/xfrm_policy.c:24:
   In file included from include/linux/netdevice.h:37:
   In file included from include/net/net_namespace.h:40:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:1024:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
                                                     ~~~~~~~~~~ ^
>> net/xfrm/xfrm_policy.c:4403:9: warning: using the result of an assignment as a condition without parentheses [-Wparentheses]
                   if (x = xfrm_migrate_state_find(mp, net, if_id)) {
                       ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/xfrm/xfrm_policy.c:4403:9: note: place parentheses around the assignment to silence this warning
                   if (x = xfrm_migrate_state_find(mp, net, if_id)) {
                         ^
                       (                                          )
   net/xfrm/xfrm_policy.c:4403:9: note: use '==' to turn this assignment into an equality comparison
                   if (x = xfrm_migrate_state_find(mp, net, if_id)) {
                         ^
                         ==
   8 warnings generated.


vim +4403 net/xfrm/xfrm_policy.c

  4373	
  4374	int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
  4375			 struct xfrm_migrate *m, int num_migrate,
  4376			 struct xfrm_kmaddress *k, struct net *net,
  4377			 struct xfrm_encap_tmpl *encap, u32 if_id)
  4378	{
  4379		int i, err, nx_cur = 0, nx_new = 0;
  4380		struct xfrm_policy *pol = NULL;
  4381		struct xfrm_state *x, *xc;
  4382		struct xfrm_state *x_cur[XFRM_MAX_DEPTH];
  4383		struct xfrm_state *x_new[XFRM_MAX_DEPTH];
  4384		struct xfrm_migrate *mp;
  4385	
  4386		/* Stage 0 - sanity checks */
  4387		if ((err = xfrm_migrate_check(m, num_migrate)) < 0)
  4388			goto out;
  4389	
  4390		if (dir >= XFRM_POLICY_MAX) {
  4391			err = -EINVAL;
  4392			goto out;
  4393		}
  4394	
  4395		/* Stage 1 - find policy */
  4396		if ((pol = xfrm_migrate_policy_find(sel, dir, type, net, if_id)) == NULL) {
  4397			err = -ENOENT;
  4398			goto out;
  4399		}
  4400	
  4401		/* Stage 2 - find and update state(s) */
  4402		for (i = 0, mp = m; i < num_migrate; i++, mp++) {
> 4403			if (x = xfrm_migrate_state_find(mp, net, if_id)) {

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/2] xfrm: Fix xfrm migrate issues when address family changes
  2022-01-06  0:52 ` [PATCH v1 2/2] xfrm: Fix xfrm migrate issues when address family changes Yan Yan
@ 2022-01-12  7:57   ` Steffen Klassert
  2022-01-19  0:03     ` Yan Yan
  0 siblings, 1 reply; 15+ messages in thread
From: Steffen Klassert @ 2022-01-12  7:57 UTC (permalink / raw)
  To: Yan Yan
  Cc: netdev, Herbert Xu, David S . Miller, Jakub Kicinski, lorenzo,
	maze, nharold, benedictwong

On Wed, Jan 05, 2022 at 04:52:51PM -0800, Yan Yan wrote:

...

> -static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig,
> -					   struct xfrm_encap_tmpl *encap)
> +static struct xfrm_state *xfrm_state_clone1(struct xfrm_state *orig,
> +					    struct xfrm_encap_tmpl *encap)
>  {
>  	struct net *net = xs_net(orig);
>  	struct xfrm_state *x = xfrm_state_alloc(net);
> @@ -1579,8 +1579,20 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig,
>  	memcpy(&x->mark, &orig->mark, sizeof(x->mark));
>  	memcpy(&x->props.smark, &orig->props.smark, sizeof(x->props.smark));
>  
> -	if (xfrm_init_state(x) < 0)
> -		goto error;
> +	return x;
> +
> + error:
> +	xfrm_state_put(x);
> +out:
> +	return NULL;
> +}
> +
> +static int xfrm_state_clone2(struct xfrm_state *orig, struct xfrm_state *x)

I'm not a frind of numbering function names, this just invites to
create xfrm_state_clone3 :)

> +{
> +	int err = xfrm_init_state(x);
> +
> +	if (err < 0)
> +		return err;
>  
>  	x->props.flags = orig->props.flags;
>  	x->props.extra_flags = orig->props.extra_flags;
> @@ -1595,12 +1607,7 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig,
>  	x->replay = orig->replay;
>  	x->preplay = orig->preplay;
>  
> -	return x;
> -
> - error:
> -	xfrm_state_put(x);
> -out:
> -	return NULL;
> +	return 0;
>  }
>  
>  struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *net,
> @@ -1661,10 +1668,14 @@ struct xfrm_state *xfrm_state_migrate(struct xfrm_state *x,
>  {
>  	struct xfrm_state *xc;
>  
> -	xc = xfrm_state_clone(x, encap);
> +	xc = xfrm_state_clone1(x, encap);
>  	if (!xc)
>  		return NULL;
>  
> +	xc->props.family = m->new_family;
> +	if (xfrm_state_clone2(x, xc) < 0)
> +		goto error;

xfrm_state_migrate() is the only function that calls xfrm_state_clone().
Wouldn't it be better to move xfrm_init_state() out of xfrm_state_clone()
and call it afterwards?

This would also fix the replay window initialization on ESN because
currently x->props.flags (which holds XFRM_STATE_ESN) is initialized
after xfrm_init_state().


^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v1 2/2] xfrm: Fix xfrm migrate issues when address family changes
  2022-01-12  7:57   ` Steffen Klassert
@ 2022-01-19  0:03     ` Yan Yan
  0 siblings, 0 replies; 15+ messages in thread
From: Yan Yan @ 2022-01-19  0:03 UTC (permalink / raw)
  To: Steffen Klassert
  Cc: netdev, Herbert Xu, David S . Miller, Jakub Kicinski,
	Lorenzo Colitti, Maciej Żenczykowski, nharold, benedictwong

Thanks Steffen, I will pull out xfrm_init_state() as you suggested.

On Tue, Jan 11, 2022 at 11:57 PM Steffen Klassert
<steffen.klassert@secunet.com> wrote:
>
> On Wed, Jan 05, 2022 at 04:52:51PM -0800, Yan Yan wrote:
>
> ...
>
> > -static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig,
> > -                                        struct xfrm_encap_tmpl *encap)
> > +static struct xfrm_state *xfrm_state_clone1(struct xfrm_state *orig,
> > +                                         struct xfrm_encap_tmpl *encap)
> >  {
> >       struct net *net = xs_net(orig);
> >       struct xfrm_state *x = xfrm_state_alloc(net);
> > @@ -1579,8 +1579,20 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig,
> >       memcpy(&x->mark, &orig->mark, sizeof(x->mark));
> >       memcpy(&x->props.smark, &orig->props.smark, sizeof(x->props.smark));
> >
> > -     if (xfrm_init_state(x) < 0)
> > -             goto error;
> > +     return x;
> > +
> > + error:
> > +     xfrm_state_put(x);
> > +out:
> > +     return NULL;
> > +}
> > +
> > +static int xfrm_state_clone2(struct xfrm_state *orig, struct xfrm_state *x)
>
> I'm not a frind of numbering function names, this just invites to
> create xfrm_state_clone3 :)
>
> > +{
> > +     int err = xfrm_init_state(x);
> > +
> > +     if (err < 0)
> > +             return err;
> >
> >       x->props.flags = orig->props.flags;
> >       x->props.extra_flags = orig->props.extra_flags;
> > @@ -1595,12 +1607,7 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig,
> >       x->replay = orig->replay;
> >       x->preplay = orig->preplay;
> >
> > -     return x;
> > -
> > - error:
> > -     xfrm_state_put(x);
> > -out:
> > -     return NULL;
> > +     return 0;
> >  }
> >
> >  struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *net,
> > @@ -1661,10 +1668,14 @@ struct xfrm_state *xfrm_state_migrate(struct xfrm_state *x,
> >  {
> >       struct xfrm_state *xc;
> >
> > -     xc = xfrm_state_clone(x, encap);
> > +     xc = xfrm_state_clone1(x, encap);
> >       if (!xc)
> >               return NULL;
> >
> > +     xc->props.family = m->new_family;
> > +     if (xfrm_state_clone2(x, xc) < 0)
> > +             goto error;
>
> xfrm_state_migrate() is the only function that calls xfrm_state_clone().
> Wouldn't it be better to move xfrm_init_state() out of xfrm_state_clone()
> and call it afterwards?
>
> This would also fix the replay window initialization on ESN because
> currently x->props.flags (which holds XFRM_STATE_ESN) is initialized
> after xfrm_init_state().
>


-- 
--
Best,
Yan

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v1 1/2] xfrm: Check if_id in xfrm_migrate
  2022-01-12 22:53   ` Yan Yan
@ 2022-01-17 11:46     ` Steffen Klassert
  0 siblings, 0 replies; 15+ messages in thread
From: Steffen Klassert @ 2022-01-17 11:46 UTC (permalink / raw)
  To: Yan Yan
  Cc: netdev, Herbert Xu, David S . Miller, Jakub Kicinski,
	Lorenzo Colitti, Maciej Żenczykowski, Nathan Harold,
	Benedict Wong

Hi Yan,

On Wed, Jan 12, 2022 at 02:53:31PM -0800, Yan Yan wrote:
> Hi Steffen,
> 
> The Jan 7th patch fixes the following warning (reported by the kernel
> test robot) by adding parentheses.
>    net/xfrm/xfrm_policy.c: In function 'xfrm_migrate':
> >> net/xfrm/xfrm_policy.c:4403:21: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
>     4403 |                 if (x = xfrm_migrate_state_find(mp, net, if_id)) {
>          |                     ^
> 
> In the Jan 7th patch, this line becomes "if ((x =
> xfrm_migrate_state_find(mp, net, if_id))) {"

I thought that was already fixed in the previous version.
Please mark updated patches as v2 etc. and describe the
changes you did in the new version.

Please resend your patchset and add a proper 'Fixes:'
tag to the patches.

Thanks!

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v1 1/2] xfrm: Check if_id in xfrm_migrate
  2022-01-12  7:32 ` Steffen Klassert
@ 2022-01-12 22:53   ` Yan Yan
  2022-01-17 11:46     ` Steffen Klassert
  0 siblings, 1 reply; 15+ messages in thread
From: Yan Yan @ 2022-01-12 22:53 UTC (permalink / raw)
  To: Steffen Klassert
  Cc: netdev, Herbert Xu, David S . Miller, Jakub Kicinski,
	Lorenzo Colitti, Maciej Żenczykowski, Nathan Harold,
	Benedict Wong

Hi Steffen,

The Jan 7th patch fixes the following warning (reported by the kernel
test robot) by adding parentheses.
   net/xfrm/xfrm_policy.c: In function 'xfrm_migrate':
>> net/xfrm/xfrm_policy.c:4403:21: warning: suggest parentheses around assignment used as truth value [-Wparentheses]
    4403 |                 if (x = xfrm_migrate_state_find(mp, net, if_id)) {
         |                     ^

In the Jan 7th patch, this line becomes "if ((x =
xfrm_migrate_state_find(mp, net, if_id))) {"


On Tue, Jan 11, 2022 at 11:32 PM Steffen Klassert
<steffen.klassert@secunet.com> wrote:
>
> On Fri, Jan 07, 2022 at 05:32:30PM -0800, Yan Yan wrote:
> > This patch enables distinguishing SAs and SPs based on if_id during
> > the xfrm_migrate flow. This ensures support for xfrm interfaces
> > throughout the SA/SP lifecycle.
> >
> > When there are multiple existing SPs with the same direction,
> > the same xfrm_selector and different endpoint addresses,
> > xfrm_migrate might fail with ENODATA.
> >
> > Specifically, the code path for performing xfrm_migrate is:
> >   Stage 1: find policy to migrate with
> >     xfrm_migrate_policy_find(sel, dir, type, net)
> >   Stage 2: find and update state(s) with
> >     xfrm_migrate_state_find(mp, net)
> >   Stage 3: update endpoint address(es) of template(s) with
> >     xfrm_policy_migrate(pol, m, num_migrate)
> >
> > Currently "Stage 1" always returns the first xfrm_policy that
> > matches, and "Stage 3" looks for the xfrm_tmpl that matches the
> > old endpoint address. Thus if there are multiple xfrm_policy
> > with same selector, direction, type and net, "Stage 1" might
> > rertun a wrong xfrm_policy and "Stage 3" will fail with ENODATA
> > because it cannot find a xfrm_tmpl with the matching endpoint
> > address.
> >
> > The fix is to allow userspace to pass an if_id and add if_id
> > to the matching rule in Stage 1 and Stage 2 since if_id is a
> > unique ID for xfrm_policy and xfrm_state. For compatibility,
> > if_id will only be checked if the attribute is set.
> >
> > Tested with additions to Android's kernel unit test suite:
> > https://android-review.googlesource.com/c/kernel/tests/+/1668886
> >
> > Signed-off-by: Yan Yan <evitayan@google.com>
>
> What is the difference between this patch and the one with
> the same subject you sent on Jan 5th?



-- 
--
Best,
Yan

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v1 1/2] xfrm: Check if_id in xfrm_migrate
  2022-01-08  1:32 [PATCH v1 1/2] xfrm: Check if_id in xfrm_migrate Yan Yan
@ 2022-01-12  7:32 ` Steffen Klassert
  2022-01-12 22:53   ` Yan Yan
  0 siblings, 1 reply; 15+ messages in thread
From: Steffen Klassert @ 2022-01-12  7:32 UTC (permalink / raw)
  To: Yan Yan
  Cc: netdev, Herbert Xu, David S . Miller, Jakub Kicinski, lorenzo,
	maze, nharold, benedictwong

On Fri, Jan 07, 2022 at 05:32:30PM -0800, Yan Yan wrote:
> This patch enables distinguishing SAs and SPs based on if_id during
> the xfrm_migrate flow. This ensures support for xfrm interfaces
> throughout the SA/SP lifecycle.
> 
> When there are multiple existing SPs with the same direction,
> the same xfrm_selector and different endpoint addresses,
> xfrm_migrate might fail with ENODATA.
> 
> Specifically, the code path for performing xfrm_migrate is:
>   Stage 1: find policy to migrate with
>     xfrm_migrate_policy_find(sel, dir, type, net)
>   Stage 2: find and update state(s) with
>     xfrm_migrate_state_find(mp, net)
>   Stage 3: update endpoint address(es) of template(s) with
>     xfrm_policy_migrate(pol, m, num_migrate)
> 
> Currently "Stage 1" always returns the first xfrm_policy that
> matches, and "Stage 3" looks for the xfrm_tmpl that matches the
> old endpoint address. Thus if there are multiple xfrm_policy
> with same selector, direction, type and net, "Stage 1" might
> rertun a wrong xfrm_policy and "Stage 3" will fail with ENODATA
> because it cannot find a xfrm_tmpl with the matching endpoint
> address.
> 
> The fix is to allow userspace to pass an if_id and add if_id
> to the matching rule in Stage 1 and Stage 2 since if_id is a
> unique ID for xfrm_policy and xfrm_state. For compatibility,
> if_id will only be checked if the attribute is set.
> 
> Tested with additions to Android's kernel unit test suite:
> https://android-review.googlesource.com/c/kernel/tests/+/1668886
> 
> Signed-off-by: Yan Yan <evitayan@google.com>

What is the difference between this patch and the one with
the same subject you sent on Jan 5th?

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH v1 1/2] xfrm: Check if_id in xfrm_migrate
@ 2022-01-08  1:32 Yan Yan
  2022-01-12  7:32 ` Steffen Klassert
  0 siblings, 1 reply; 15+ messages in thread
From: Yan Yan @ 2022-01-08  1:32 UTC (permalink / raw)
  To: Steffen Klassert
  Cc: netdev, Herbert Xu, David S . Miller, Jakub Kicinski, lorenzo,
	maze, nharold, benedictwong, Yan Yan

This patch enables distinguishing SAs and SPs based on if_id during
the xfrm_migrate flow. This ensures support for xfrm interfaces
throughout the SA/SP lifecycle.

When there are multiple existing SPs with the same direction,
the same xfrm_selector and different endpoint addresses,
xfrm_migrate might fail with ENODATA.

Specifically, the code path for performing xfrm_migrate is:
  Stage 1: find policy to migrate with
    xfrm_migrate_policy_find(sel, dir, type, net)
  Stage 2: find and update state(s) with
    xfrm_migrate_state_find(mp, net)
  Stage 3: update endpoint address(es) of template(s) with
    xfrm_policy_migrate(pol, m, num_migrate)

Currently "Stage 1" always returns the first xfrm_policy that
matches, and "Stage 3" looks for the xfrm_tmpl that matches the
old endpoint address. Thus if there are multiple xfrm_policy
with same selector, direction, type and net, "Stage 1" might
rertun a wrong xfrm_policy and "Stage 3" will fail with ENODATA
because it cannot find a xfrm_tmpl with the matching endpoint
address.

The fix is to allow userspace to pass an if_id and add if_id
to the matching rule in Stage 1 and Stage 2 since if_id is a
unique ID for xfrm_policy and xfrm_state. For compatibility,
if_id will only be checked if the attribute is set.

Tested with additions to Android's kernel unit test suite:
https://android-review.googlesource.com/c/kernel/tests/+/1668886

Signed-off-by: Yan Yan <evitayan@google.com>
---
 include/net/xfrm.h     |  5 +++--
 net/key/af_key.c       |  2 +-
 net/xfrm/xfrm_policy.c | 14 ++++++++------
 net/xfrm/xfrm_state.c  |  7 ++++++-
 net/xfrm/xfrm_user.c   |  6 +++++-
 5 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 83b46da8873d..faee50259fd4 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1676,14 +1676,15 @@ int km_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 	       const struct xfrm_migrate *m, int num_bundles,
 	       const struct xfrm_kmaddress *k,
 	       const struct xfrm_encap_tmpl *encap);
-struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *net);
+struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *net,
+						u32 if_id);
 struct xfrm_state *xfrm_state_migrate(struct xfrm_state *x,
 				      struct xfrm_migrate *m,
 				      struct xfrm_encap_tmpl *encap);
 int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 		 struct xfrm_migrate *m, int num_bundles,
 		 struct xfrm_kmaddress *k, struct net *net,
-		 struct xfrm_encap_tmpl *encap);
+		 struct xfrm_encap_tmpl *encap, u32 if_id);
 #endif
 
 int km_new_mapping(struct xfrm_state *x, xfrm_address_t *ipaddr, __be16 sport);
diff --git a/net/key/af_key.c b/net/key/af_key.c
index de24a7d474df..9bf52a09b5ff 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -2623,7 +2623,7 @@ static int pfkey_migrate(struct sock *sk, struct sk_buff *skb,
 	}
 
 	return xfrm_migrate(&sel, dir, XFRM_POLICY_TYPE_MAIN, m, i,
-			    kma ? &k : NULL, net, NULL);
+			    kma ? &k : NULL, net, NULL, 0);
 
  out:
 	return err;
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 1a06585022ab..a5e903ec2b3b 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -4235,7 +4235,7 @@ static bool xfrm_migrate_selector_match(const struct xfrm_selector *sel_cmp,
 }
 
 static struct xfrm_policy *xfrm_migrate_policy_find(const struct xfrm_selector *sel,
-						    u8 dir, u8 type, struct net *net)
+						    u8 dir, u8 type, struct net *net, u32 if_id)
 {
 	struct xfrm_policy *pol, *ret = NULL;
 	struct hlist_head *chain;
@@ -4244,7 +4244,8 @@ static struct xfrm_policy *xfrm_migrate_policy_find(const struct xfrm_selector *
 	spin_lock_bh(&net->xfrm.xfrm_policy_lock);
 	chain = policy_hash_direct(net, &sel->daddr, &sel->saddr, sel->family, dir);
 	hlist_for_each_entry(pol, chain, bydst) {
-		if (xfrm_migrate_selector_match(sel, &pol->selector) &&
+		if ((if_id == 0 || pol->if_id == if_id) &&
+		    xfrm_migrate_selector_match(sel, &pol->selector) &&
 		    pol->type == type) {
 			ret = pol;
 			priority = ret->priority;
@@ -4256,7 +4257,8 @@ static struct xfrm_policy *xfrm_migrate_policy_find(const struct xfrm_selector *
 		if ((pol->priority >= priority) && ret)
 			break;
 
-		if (xfrm_migrate_selector_match(sel, &pol->selector) &&
+		if ((if_id == 0 || pol->if_id == if_id) &&
+		    xfrm_migrate_selector_match(sel, &pol->selector) &&
 		    pol->type == type) {
 			ret = pol;
 			break;
@@ -4372,7 +4374,7 @@ static int xfrm_migrate_check(const struct xfrm_migrate *m, int num_migrate)
 int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 		 struct xfrm_migrate *m, int num_migrate,
 		 struct xfrm_kmaddress *k, struct net *net,
-		 struct xfrm_encap_tmpl *encap)
+		 struct xfrm_encap_tmpl *encap, u32 if_id)
 {
 	int i, err, nx_cur = 0, nx_new = 0;
 	struct xfrm_policy *pol = NULL;
@@ -4391,14 +4393,14 @@ int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 	}
 
 	/* Stage 1 - find policy */
-	if ((pol = xfrm_migrate_policy_find(sel, dir, type, net)) == NULL) {
+	if ((pol = xfrm_migrate_policy_find(sel, dir, type, net, if_id)) == NULL) {
 		err = -ENOENT;
 		goto out;
 	}
 
 	/* Stage 2 - find and update state(s) */
 	for (i = 0, mp = m; i < num_migrate; i++, mp++) {
-		if ((x = xfrm_migrate_state_find(mp, net))) {
+		if ((x = xfrm_migrate_state_find(mp, net, if_id))) {
 			x_cur[nx_cur] = x;
 			nx_cur++;
 			xc = xfrm_state_migrate(x, mp, encap);
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 0407272a990c..f52767685b13 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1603,7 +1603,8 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig,
 	return NULL;
 }
 
-struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *net)
+struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *net,
+						u32 if_id)
 {
 	unsigned int h;
 	struct xfrm_state *x = NULL;
@@ -1619,6 +1620,8 @@ struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *n
 				continue;
 			if (m->reqid && x->props.reqid != m->reqid)
 				continue;
+			if (if_id != 0 && x->if_id != if_id)
+				continue;
 			if (!xfrm_addr_equal(&x->id.daddr, &m->old_daddr,
 					     m->old_family) ||
 			    !xfrm_addr_equal(&x->props.saddr, &m->old_saddr,
@@ -1634,6 +1637,8 @@ struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *n
 			if (x->props.mode != m->mode ||
 			    x->id.proto != m->proto)
 				continue;
+			if (if_id != 0 && x->if_id != if_id)
+				continue;
 			if (!xfrm_addr_equal(&x->id.daddr, &m->old_daddr,
 					     m->old_family) ||
 			    !xfrm_addr_equal(&x->props.saddr, &m->old_saddr,
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index e3e26f4da6c2..eda02c5544ae 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2580,6 +2580,7 @@ static int xfrm_do_migrate(struct sk_buff *skb, struct nlmsghdr *nlh,
 	int n = 0;
 	struct net *net = sock_net(skb->sk);
 	struct xfrm_encap_tmpl  *encap = NULL;
+	u32 if_id = 0;
 
 	if (attrs[XFRMA_MIGRATE] == NULL)
 		return -EINVAL;
@@ -2604,7 +2605,10 @@ static int xfrm_do_migrate(struct sk_buff *skb, struct nlmsghdr *nlh,
 			return -ENOMEM;
 	}
 
-	err = xfrm_migrate(&pi->sel, pi->dir, type, m, n, kmp, net, encap);
+	if (attrs[XFRMA_IF_ID])
+		if_id = nla_get_u32(attrs[XFRMA_IF_ID]);
+
+	err = xfrm_migrate(&pi->sel, pi->dir, type, m, n, kmp, net, encap, if_id);
 
 	kfree(encap);
 
-- 
2.34.1.575.g55b058a8bb-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread
* Re: [PATCH v1 1/2] xfrm: Check if_id in xfrm_migrate
  2021-12-23  0:45 ` [PATCH v1 1/2] xfrm: Check if_id " Yan Yan
@ 2021-12-25 18:30     ` kernel test robot
  0 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2021-12-25 18:30 UTC (permalink / raw)
  To: Yan Yan, steffen.klassert
  Cc: kbuild-all, herbert, davem, netdev, nharold, benedictwong, maze,
	lorenzo, Yan Yan

Hi Yan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on klassert-ipsec-next/master]
[also build test ERROR on klassert-ipsec/master net-next/master net/master v5.16-rc6 next-20211224]
[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]

url:    https://github.com/0day-ci/linux/commits/Yan-Yan/Fix-issues-in-xfrm_migrate/20211223-084725
base:   https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20211226/202112260218.oyI4rj2f-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/a1b1a05814d4ac913aa4af753da7e116a3d58342
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yan-Yan/Fix-issues-in-xfrm_migrate/20211223-084725
        git checkout a1b1a05814d4ac913aa4af753da7e116a3d58342
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpio/ net/key/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   net/key/af_key.c: In function 'pfkey_migrate':
>> net/key/af_key.c:2625:9: error: too few arguments to function 'xfrm_migrate'
    2625 |  return xfrm_migrate(&sel, dir, XFRM_POLICY_TYPE_MAIN, m, i,
         |         ^~~~~~~~~~~~
   In file included from net/key/af_key.c:28:
   include/net/xfrm.h:1683:5: note: declared here
    1683 | int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
         |     ^~~~~~~~~~~~


vim +/xfrm_migrate +2625 net/key/af_key.c

08de61beab8a21 Shinta Sugimoto   2007-02-08  2539  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2540  static int pfkey_migrate(struct sock *sk, struct sk_buff *skb,
4c93fbb0626080 David S. Miller   2011-02-25  2541  			 const struct sadb_msg *hdr, void * const *ext_hdrs)
08de61beab8a21 Shinta Sugimoto   2007-02-08  2542  {
08de61beab8a21 Shinta Sugimoto   2007-02-08  2543  	int i, len, ret, err = -EINVAL;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2544  	u8 dir;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2545  	struct sadb_address *sa;
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2546  	struct sadb_x_kmaddress *kma;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2547  	struct sadb_x_policy *pol;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2548  	struct sadb_x_ipsecrequest *rq;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2549  	struct xfrm_selector sel;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2550  	struct xfrm_migrate m[XFRM_MAX_DEPTH];
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2551  	struct xfrm_kmaddress k;
8d549c4f5d92d8 Fan Du            2013-11-07  2552  	struct net *net = sock_net(sk);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2553  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2554  	if (!present_and_same_family(ext_hdrs[SADB_EXT_ADDRESS_SRC - 1],
08de61beab8a21 Shinta Sugimoto   2007-02-08  2555  				     ext_hdrs[SADB_EXT_ADDRESS_DST - 1]) ||
08de61beab8a21 Shinta Sugimoto   2007-02-08  2556  	    !ext_hdrs[SADB_X_EXT_POLICY - 1]) {
08de61beab8a21 Shinta Sugimoto   2007-02-08  2557  		err = -EINVAL;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2558  		goto out;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2559  	}
08de61beab8a21 Shinta Sugimoto   2007-02-08  2560  
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2561  	kma = ext_hdrs[SADB_X_EXT_KMADDRESS - 1];
08de61beab8a21 Shinta Sugimoto   2007-02-08  2562  	pol = ext_hdrs[SADB_X_EXT_POLICY - 1];
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2563  
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2564  	if (pol->sadb_x_policy_dir >= IPSEC_DIR_MAX) {
08de61beab8a21 Shinta Sugimoto   2007-02-08  2565  		err = -EINVAL;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2566  		goto out;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2567  	}
08de61beab8a21 Shinta Sugimoto   2007-02-08  2568  
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2569  	if (kma) {
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2570  		/* convert sadb_x_kmaddress to xfrm_kmaddress */
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2571  		k.reserved = kma->sadb_x_kmaddress_reserved;
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2572  		ret = parse_sockaddr_pair((struct sockaddr *)(kma + 1),
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2573  					  8*(kma->sadb_x_kmaddress_len) - sizeof(*kma),
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2574  					  &k.local, &k.remote, &k.family);
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2575  		if (ret < 0) {
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2576  			err = ret;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2577  			goto out;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2578  		}
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2579  	}
08de61beab8a21 Shinta Sugimoto   2007-02-08  2580  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2581  	dir = pol->sadb_x_policy_dir - 1;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2582  	memset(&sel, 0, sizeof(sel));
08de61beab8a21 Shinta Sugimoto   2007-02-08  2583  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2584  	/* set source address info of selector */
08de61beab8a21 Shinta Sugimoto   2007-02-08  2585  	sa = ext_hdrs[SADB_EXT_ADDRESS_SRC - 1];
08de61beab8a21 Shinta Sugimoto   2007-02-08  2586  	sel.family = pfkey_sadb_addr2xfrm_addr(sa, &sel.saddr);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2587  	sel.prefixlen_s = sa->sadb_address_prefixlen;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2588  	sel.proto = pfkey_proto_to_xfrm(sa->sadb_address_proto);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2589  	sel.sport = ((struct sockaddr_in *)(sa + 1))->sin_port;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2590  	if (sel.sport)
582ee43dad8e41 Al Viro           2007-07-26  2591  		sel.sport_mask = htons(0xffff);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2592  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2593  	/* set destination address info of selector */
47162c0b7e26ef Himangi Saraogi   2014-05-30  2594  	sa = ext_hdrs[SADB_EXT_ADDRESS_DST - 1];
08de61beab8a21 Shinta Sugimoto   2007-02-08  2595  	pfkey_sadb_addr2xfrm_addr(sa, &sel.daddr);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2596  	sel.prefixlen_d = sa->sadb_address_prefixlen;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2597  	sel.proto = pfkey_proto_to_xfrm(sa->sadb_address_proto);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2598  	sel.dport = ((struct sockaddr_in *)(sa + 1))->sin_port;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2599  	if (sel.dport)
582ee43dad8e41 Al Viro           2007-07-26  2600  		sel.dport_mask = htons(0xffff);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2601  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2602  	rq = (struct sadb_x_ipsecrequest *)(pol + 1);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2603  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2604  	/* extract ipsecrequests */
08de61beab8a21 Shinta Sugimoto   2007-02-08  2605  	i = 0;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2606  	len = pol->sadb_x_policy_len * 8 - sizeof(struct sadb_x_policy);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2607  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2608  	while (len > 0 && i < XFRM_MAX_DEPTH) {
08de61beab8a21 Shinta Sugimoto   2007-02-08  2609  		ret = ipsecrequests_to_migrate(rq, len, &m[i]);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2610  		if (ret < 0) {
08de61beab8a21 Shinta Sugimoto   2007-02-08  2611  			err = ret;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2612  			goto out;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2613  		} else {
08de61beab8a21 Shinta Sugimoto   2007-02-08  2614  			rq = (struct sadb_x_ipsecrequest *)((u8 *)rq + ret);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2615  			len -= ret;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2616  			i++;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2617  		}
08de61beab8a21 Shinta Sugimoto   2007-02-08  2618  	}
08de61beab8a21 Shinta Sugimoto   2007-02-08  2619  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2620  	if (!i || len > 0) {
08de61beab8a21 Shinta Sugimoto   2007-02-08  2621  		err = -EINVAL;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2622  		goto out;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2623  	}
08de61beab8a21 Shinta Sugimoto   2007-02-08  2624  
13c1d18931ebb5 Arnaud Ebalard    2008-10-05 @2625  	return xfrm_migrate(&sel, dir, XFRM_POLICY_TYPE_MAIN, m, i,
4ab47d47af20ad Antony Antony     2017-06-06  2626  			    kma ? &k : NULL, net, NULL);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2627  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2628   out:
08de61beab8a21 Shinta Sugimoto   2007-02-08  2629  	return err;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2630  }
08de61beab8a21 Shinta Sugimoto   2007-02-08  2631  #else
08de61beab8a21 Shinta Sugimoto   2007-02-08  2632  static int pfkey_migrate(struct sock *sk, struct sk_buff *skb,
7f6daa635c28ed Stephen Hemminger 2011-03-01  2633  			 const struct sadb_msg *hdr, void * const *ext_hdrs)
08de61beab8a21 Shinta Sugimoto   2007-02-08  2634  {
08de61beab8a21 Shinta Sugimoto   2007-02-08  2635  	return -ENOPROTOOPT;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2636  }
08de61beab8a21 Shinta Sugimoto   2007-02-08  2637  #endif
08de61beab8a21 Shinta Sugimoto   2007-02-08  2638  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2639  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 15+ messages in thread
* Re: [PATCH v1 1/2] xfrm: Check if_id in xfrm_migrate
@ 2021-12-25 18:30     ` kernel test robot
  0 siblings, 0 replies; 15+ messages in thread
From: kernel test robot @ 2021-12-25 18:30 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 9855 bytes --]

Hi Yan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on klassert-ipsec-next/master]
[also build test ERROR on klassert-ipsec/master net-next/master net/master v5.16-rc6 next-20211224]
[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]

url:    https://github.com/0day-ci/linux/commits/Yan-Yan/Fix-issues-in-xfrm_migrate/20211223-084725
base:   https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20211226/202112260218.oyI4rj2f-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/a1b1a05814d4ac913aa4af753da7e116a3d58342
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Yan-Yan/Fix-issues-in-xfrm_migrate/20211223-084725
        git checkout a1b1a05814d4ac913aa4af753da7e116a3d58342
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpio/ net/key/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   net/key/af_key.c: In function 'pfkey_migrate':
>> net/key/af_key.c:2625:9: error: too few arguments to function 'xfrm_migrate'
    2625 |  return xfrm_migrate(&sel, dir, XFRM_POLICY_TYPE_MAIN, m, i,
         |         ^~~~~~~~~~~~
   In file included from net/key/af_key.c:28:
   include/net/xfrm.h:1683:5: note: declared here
    1683 | int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
         |     ^~~~~~~~~~~~


vim +/xfrm_migrate +2625 net/key/af_key.c

08de61beab8a21 Shinta Sugimoto   2007-02-08  2539  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2540  static int pfkey_migrate(struct sock *sk, struct sk_buff *skb,
4c93fbb0626080 David S. Miller   2011-02-25  2541  			 const struct sadb_msg *hdr, void * const *ext_hdrs)
08de61beab8a21 Shinta Sugimoto   2007-02-08  2542  {
08de61beab8a21 Shinta Sugimoto   2007-02-08  2543  	int i, len, ret, err = -EINVAL;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2544  	u8 dir;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2545  	struct sadb_address *sa;
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2546  	struct sadb_x_kmaddress *kma;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2547  	struct sadb_x_policy *pol;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2548  	struct sadb_x_ipsecrequest *rq;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2549  	struct xfrm_selector sel;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2550  	struct xfrm_migrate m[XFRM_MAX_DEPTH];
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2551  	struct xfrm_kmaddress k;
8d549c4f5d92d8 Fan Du            2013-11-07  2552  	struct net *net = sock_net(sk);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2553  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2554  	if (!present_and_same_family(ext_hdrs[SADB_EXT_ADDRESS_SRC - 1],
08de61beab8a21 Shinta Sugimoto   2007-02-08  2555  				     ext_hdrs[SADB_EXT_ADDRESS_DST - 1]) ||
08de61beab8a21 Shinta Sugimoto   2007-02-08  2556  	    !ext_hdrs[SADB_X_EXT_POLICY - 1]) {
08de61beab8a21 Shinta Sugimoto   2007-02-08  2557  		err = -EINVAL;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2558  		goto out;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2559  	}
08de61beab8a21 Shinta Sugimoto   2007-02-08  2560  
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2561  	kma = ext_hdrs[SADB_X_EXT_KMADDRESS - 1];
08de61beab8a21 Shinta Sugimoto   2007-02-08  2562  	pol = ext_hdrs[SADB_X_EXT_POLICY - 1];
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2563  
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2564  	if (pol->sadb_x_policy_dir >= IPSEC_DIR_MAX) {
08de61beab8a21 Shinta Sugimoto   2007-02-08  2565  		err = -EINVAL;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2566  		goto out;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2567  	}
08de61beab8a21 Shinta Sugimoto   2007-02-08  2568  
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2569  	if (kma) {
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2570  		/* convert sadb_x_kmaddress to xfrm_kmaddress */
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2571  		k.reserved = kma->sadb_x_kmaddress_reserved;
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2572  		ret = parse_sockaddr_pair((struct sockaddr *)(kma + 1),
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2573  					  8*(kma->sadb_x_kmaddress_len) - sizeof(*kma),
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2574  					  &k.local, &k.remote, &k.family);
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2575  		if (ret < 0) {
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2576  			err = ret;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2577  			goto out;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2578  		}
13c1d18931ebb5 Arnaud Ebalard    2008-10-05  2579  	}
08de61beab8a21 Shinta Sugimoto   2007-02-08  2580  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2581  	dir = pol->sadb_x_policy_dir - 1;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2582  	memset(&sel, 0, sizeof(sel));
08de61beab8a21 Shinta Sugimoto   2007-02-08  2583  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2584  	/* set source address info of selector */
08de61beab8a21 Shinta Sugimoto   2007-02-08  2585  	sa = ext_hdrs[SADB_EXT_ADDRESS_SRC - 1];
08de61beab8a21 Shinta Sugimoto   2007-02-08  2586  	sel.family = pfkey_sadb_addr2xfrm_addr(sa, &sel.saddr);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2587  	sel.prefixlen_s = sa->sadb_address_prefixlen;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2588  	sel.proto = pfkey_proto_to_xfrm(sa->sadb_address_proto);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2589  	sel.sport = ((struct sockaddr_in *)(sa + 1))->sin_port;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2590  	if (sel.sport)
582ee43dad8e41 Al Viro           2007-07-26  2591  		sel.sport_mask = htons(0xffff);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2592  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2593  	/* set destination address info of selector */
47162c0b7e26ef Himangi Saraogi   2014-05-30  2594  	sa = ext_hdrs[SADB_EXT_ADDRESS_DST - 1];
08de61beab8a21 Shinta Sugimoto   2007-02-08  2595  	pfkey_sadb_addr2xfrm_addr(sa, &sel.daddr);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2596  	sel.prefixlen_d = sa->sadb_address_prefixlen;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2597  	sel.proto = pfkey_proto_to_xfrm(sa->sadb_address_proto);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2598  	sel.dport = ((struct sockaddr_in *)(sa + 1))->sin_port;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2599  	if (sel.dport)
582ee43dad8e41 Al Viro           2007-07-26  2600  		sel.dport_mask = htons(0xffff);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2601  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2602  	rq = (struct sadb_x_ipsecrequest *)(pol + 1);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2603  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2604  	/* extract ipsecrequests */
08de61beab8a21 Shinta Sugimoto   2007-02-08  2605  	i = 0;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2606  	len = pol->sadb_x_policy_len * 8 - sizeof(struct sadb_x_policy);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2607  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2608  	while (len > 0 && i < XFRM_MAX_DEPTH) {
08de61beab8a21 Shinta Sugimoto   2007-02-08  2609  		ret = ipsecrequests_to_migrate(rq, len, &m[i]);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2610  		if (ret < 0) {
08de61beab8a21 Shinta Sugimoto   2007-02-08  2611  			err = ret;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2612  			goto out;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2613  		} else {
08de61beab8a21 Shinta Sugimoto   2007-02-08  2614  			rq = (struct sadb_x_ipsecrequest *)((u8 *)rq + ret);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2615  			len -= ret;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2616  			i++;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2617  		}
08de61beab8a21 Shinta Sugimoto   2007-02-08  2618  	}
08de61beab8a21 Shinta Sugimoto   2007-02-08  2619  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2620  	if (!i || len > 0) {
08de61beab8a21 Shinta Sugimoto   2007-02-08  2621  		err = -EINVAL;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2622  		goto out;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2623  	}
08de61beab8a21 Shinta Sugimoto   2007-02-08  2624  
13c1d18931ebb5 Arnaud Ebalard    2008-10-05 @2625  	return xfrm_migrate(&sel, dir, XFRM_POLICY_TYPE_MAIN, m, i,
4ab47d47af20ad Antony Antony     2017-06-06  2626  			    kma ? &k : NULL, net, NULL);
08de61beab8a21 Shinta Sugimoto   2007-02-08  2627  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2628   out:
08de61beab8a21 Shinta Sugimoto   2007-02-08  2629  	return err;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2630  }
08de61beab8a21 Shinta Sugimoto   2007-02-08  2631  #else
08de61beab8a21 Shinta Sugimoto   2007-02-08  2632  static int pfkey_migrate(struct sock *sk, struct sk_buff *skb,
7f6daa635c28ed Stephen Hemminger 2011-03-01  2633  			 const struct sadb_msg *hdr, void * const *ext_hdrs)
08de61beab8a21 Shinta Sugimoto   2007-02-08  2634  {
08de61beab8a21 Shinta Sugimoto   2007-02-08  2635  	return -ENOPROTOOPT;
08de61beab8a21 Shinta Sugimoto   2007-02-08  2636  }
08de61beab8a21 Shinta Sugimoto   2007-02-08  2637  #endif
08de61beab8a21 Shinta Sugimoto   2007-02-08  2638  
08de61beab8a21 Shinta Sugimoto   2007-02-08  2639  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 15+ messages in thread
* [PATCH v1 1/2] xfrm: Check if_id in xfrm_migrate
  2021-12-23  0:45 [PATCH v1 0/2] Fix issues " Yan Yan
@ 2021-12-23  0:45 ` Yan Yan
  2021-12-25 18:30     ` kernel test robot
  0 siblings, 1 reply; 15+ messages in thread
From: Yan Yan @ 2021-12-23  0:45 UTC (permalink / raw)
  To: steffen.klassert
  Cc: herbert, davem, netdev, nharold, benedictwong, maze, lorenzo, Yan Yan

This patch enables distinguishing SAs and SPs based on if_id during
the xfrm_migrate flow. This ensures support for xfrm interfaces
throughout the SA/SP lifecycle.

When there are multiple existing SPs with the same direction,
the same xfrm_selector and different endpoint addresses,
xfrm_migrate might fail with ENODATA.

Specifically, the code path for performing xfrm_migrate is:
  Stage 1: find policy to migrate with
    xfrm_migrate_policy_find(sel, dir, type, net)
  Stage 2: find and update state(s) with
    xfrm_migrate_state_find(mp, net)
  Stage 3: update endpoint address(es) of template(s) with
    xfrm_policy_migrate(pol, m, num_migrate)

Currently "Stage 1" always returns the first xfrm_policy that
matches, and "Stage 3" looks for the xfrm_tmpl that matches the
old endpoint address. Thus if there are multiple xfrm_policy
with same selector, direction, type and net, "Stage 1" might
rertun a wrong xfrm_policy and "Stage 3" will fail with ENODATA
because it cannot find a xfrm_tmpl with the matching endpoint
address.

The fix is to allow userspace to pass an if_id and add if_id
to the matching rule in Stage 1 and Stage 2 since if_id is a
unique ID for xfrm_policy and xfrm_state. For compatibility,
if_id will only be checked if the attribute is set.

Tested with additions to Android's kernel unit test suite:
https://android-review.googlesource.com/c/kernel/tests/+/1668886

Signed-off-by: Yan Yan <evitayan@google.com>
---
 include/net/xfrm.h     |  5 +++--
 net/xfrm/xfrm_policy.c | 14 ++++++++------
 net/xfrm/xfrm_state.c  |  7 ++++++-
 net/xfrm/xfrm_user.c   |  6 +++++-
 4 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 2308210793a0..4fdd0c44b705 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1675,14 +1675,15 @@ int km_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 	       const struct xfrm_migrate *m, int num_bundles,
 	       const struct xfrm_kmaddress *k,
 	       const struct xfrm_encap_tmpl *encap);
-struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *net);
+struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *net,
+				 	   u32 if_id);
 struct xfrm_state *xfrm_state_migrate(struct xfrm_state *x,
 				      struct xfrm_migrate *m,
 				      struct xfrm_encap_tmpl *encap);
 int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 		 struct xfrm_migrate *m, int num_bundles,
 		 struct xfrm_kmaddress *k, struct net *net,
-		 struct xfrm_encap_tmpl *encap);
+		 struct xfrm_encap_tmpl *encap, u32 if_id);
 #endif
 
 int km_new_mapping(struct xfrm_state *x, xfrm_address_t *ipaddr, __be16 sport);
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 1a06585022ab..cd656b4ec5b9 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -4235,7 +4235,7 @@ static bool xfrm_migrate_selector_match(const struct xfrm_selector *sel_cmp,
 }
 
 static struct xfrm_policy *xfrm_migrate_policy_find(const struct xfrm_selector *sel,
-						    u8 dir, u8 type, struct net *net)
+						    u8 dir, u8 type, struct net *net, u32 if_id)
 {
 	struct xfrm_policy *pol, *ret = NULL;
 	struct hlist_head *chain;
@@ -4244,7 +4244,8 @@ static struct xfrm_policy *xfrm_migrate_policy_find(const struct xfrm_selector *
 	spin_lock_bh(&net->xfrm.xfrm_policy_lock);
 	chain = policy_hash_direct(net, &sel->daddr, &sel->saddr, sel->family, dir);
 	hlist_for_each_entry(pol, chain, bydst) {
-		if (xfrm_migrate_selector_match(sel, &pol->selector) &&
+		if ((if_id == 0 || pol->if_id == if_id) &&
+		    xfrm_migrate_selector_match(sel, &pol->selector) &&
 		    pol->type == type) {
 			ret = pol;
 			priority = ret->priority;
@@ -4256,7 +4257,8 @@ static struct xfrm_policy *xfrm_migrate_policy_find(const struct xfrm_selector *
 		if ((pol->priority >= priority) && ret)
 			break;
 
-		if (xfrm_migrate_selector_match(sel, &pol->selector) &&
+		if ((if_id == 0 || pol->if_id == if_id) &&
+		    xfrm_migrate_selector_match(sel, &pol->selector) &&
 		    pol->type == type) {
 			ret = pol;
 			break;
@@ -4372,7 +4374,7 @@ static int xfrm_migrate_check(const struct xfrm_migrate *m, int num_migrate)
 int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 		 struct xfrm_migrate *m, int num_migrate,
 		 struct xfrm_kmaddress *k, struct net *net,
-		 struct xfrm_encap_tmpl *encap)
+		 struct xfrm_encap_tmpl *encap, u32 if_id)
 {
 	int i, err, nx_cur = 0, nx_new = 0;
 	struct xfrm_policy *pol = NULL;
@@ -4391,14 +4393,14 @@ int xfrm_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 	}
 
 	/* Stage 1 - find policy */
-	if ((pol = xfrm_migrate_policy_find(sel, dir, type, net)) == NULL) {
+	if ((pol = xfrm_migrate_policy_find(sel, dir, type, net, if_id)) == NULL) {
 		err = -ENOENT;
 		goto out;
 	}
 
 	/* Stage 2 - find and update state(s) */
 	for (i = 0, mp = m; i < num_migrate; i++, mp++) {
-		if ((x = xfrm_migrate_state_find(mp, net))) {
+		if (x = xfrm_migrate_state_find(mp, net, if_id)) {
 			x_cur[nx_cur] = x;
 			nx_cur++;
 			xc = xfrm_state_migrate(x, mp, encap);
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index a2f4001221d1..74d4283ed282 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1602,7 +1602,8 @@ static struct xfrm_state *xfrm_state_clone(struct xfrm_state *orig,
 	return NULL;
 }
 
-struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *net)
+struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *net,
+				           u32 if_id)
 {
 	unsigned int h;
 	struct xfrm_state *x = NULL;
@@ -1618,6 +1619,8 @@ struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *n
 				continue;
 			if (m->reqid && x->props.reqid != m->reqid)
 				continue;
+			if (if_id != 0 && x->if_id != if_id)
+				continue;
 			if (!xfrm_addr_equal(&x->id.daddr, &m->old_daddr,
 					     m->old_family) ||
 			    !xfrm_addr_equal(&x->props.saddr, &m->old_saddr,
@@ -1633,6 +1636,8 @@ struct xfrm_state *xfrm_migrate_state_find(struct xfrm_migrate *m, struct net *n
 			if (x->props.mode != m->mode ||
 			    x->id.proto != m->proto)
 				continue;
+			if (if_id != 0 && x->if_id != if_id)
+				continue;
 			if (!xfrm_addr_equal(&x->id.daddr, &m->old_daddr,
 					     m->old_family) ||
 			    !xfrm_addr_equal(&x->props.saddr, &m->old_saddr,
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 7c36cc1f3d79..8cbbe8107543 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -2579,6 +2579,7 @@ static int xfrm_do_migrate(struct sk_buff *skb, struct nlmsghdr *nlh,
 	int n = 0;
 	struct net *net = sock_net(skb->sk);
 	struct xfrm_encap_tmpl  *encap = NULL;
+	u32 if_id = 0;
 
 	if (attrs[XFRMA_MIGRATE] == NULL)
 		return -EINVAL;
@@ -2603,7 +2604,10 @@ static int xfrm_do_migrate(struct sk_buff *skb, struct nlmsghdr *nlh,
 			return -ENOMEM;
 	}
 
-	err = xfrm_migrate(&pi->sel, pi->dir, type, m, n, kmp, net, encap);
+	if (attrs[XFRMA_IF_ID])
+		if_id = nla_get_u32(attrs[XFRMA_IF_ID]);
+
+	err = xfrm_migrate(&pi->sel, pi->dir, type, m, n, kmp, net, encap, if_id);
 
 	kfree(encap);
 
-- 
2.34.1.307.g9b7440fafd-goog


^ permalink raw reply related	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2022-01-19  0:03 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-06  0:52 [PATCH v1 0/2] Fix issues in xfrm_migrate Yan Yan
2022-01-06  0:52 ` [PATCH v1 1/2] xfrm: Check if_id " Yan Yan
2022-01-06 12:22   ` kernel test robot
2022-01-06 15:55   ` kernel test robot
2022-01-06 15:55     ` kernel test robot
2022-01-06  0:52 ` [PATCH v1 2/2] xfrm: Fix xfrm migrate issues when address family changes Yan Yan
2022-01-12  7:57   ` Steffen Klassert
2022-01-19  0:03     ` Yan Yan
  -- strict thread matches above, loose matches on Subject: below --
2022-01-08  1:32 [PATCH v1 1/2] xfrm: Check if_id in xfrm_migrate Yan Yan
2022-01-12  7:32 ` Steffen Klassert
2022-01-12 22:53   ` Yan Yan
2022-01-17 11:46     ` Steffen Klassert
2021-12-23  0:45 [PATCH v1 0/2] Fix issues " Yan Yan
2021-12-23  0:45 ` [PATCH v1 1/2] xfrm: Check if_id " Yan Yan
2021-12-25 18:30   ` kernel test robot
2021-12-25 18:30     ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.