diff mbox series

[SRU,F,B/hwe] xfrm: policy: match with both mark and mask on user interfaces

Message ID 20200810144447.27902-1-stefan.bader@canonical.com
State New
Headers show
Series [SRU,F,B/hwe] xfrm: policy: match with both mark and mask on user interfaces | expand

Commit Message

Stefan Bader Aug. 10, 2020, 2:44 p.m. UTC
From: Xin Long <lucien.xin@gmail.com>

In commit ed17b8d377ea ("xfrm: fix a warning in xfrm_policy_insert_list"),
it would take 'priority' to make a policy unique, and allow duplicated
policies with different 'priority' to be added, which is not expected
by userland, as Tobias reported in strongswan.

To fix this duplicated policies issue, and also fix the issue in
commit ed17b8d377ea ("xfrm: fix a warning in xfrm_policy_insert_list"),
when doing add/del/get/update on user interfaces, this patch is to change
to look up a policy with both mark and mask by doing:

  mark.v == pol->mark.v && mark.m == pol->mark.m

and leave the check:

  (mark & pol->mark.m) == pol->mark.v

for tx/rx path only.

As the userland expects an exact mark and mask match to manage policies.

v1->v2:
  - make xfrm_policy_mark_match inline and fix the changelog as
    Tobias suggested.

Fixes: 295fae568885 ("xfrm: Allow user space manipulation of SPD mark")
Fixes: ed17b8d377ea ("xfrm: fix a warning in xfrm_policy_insert_list")
Reported-by: Tobias Brunner <tobias@strongswan.org>
Tested-by: Tobias Brunner <tobias@strongswan.org>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

BugLink: https://bugs.launchpad.net/bugs/1890796

(cherry picked from commit 4f47e8ab6ab796b5380f74866fa5287aca4dcc58)
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
---

This is for focal and bionic/hwe first, it seems the offending patch
also went into bionic and xenial but starting with bionic there is some
backport effort required to apply it. So submitting this first for
the easy part.

-Stefan


 include/net/xfrm.h     | 11 +++++++----
 net/key/af_key.c       |  4 ++--
 net/xfrm/xfrm_policy.c | 39 ++++++++++++++++-----------------------
 net/xfrm/xfrm_user.c   | 18 +++++++++++-------
 4 files changed, 36 insertions(+), 36 deletions(-)

Comments

Thadeu Lima de Souza Cascardo Aug. 10, 2020, 3:16 p.m. UTC | #1
On Mon, Aug 10, 2020 at 04:44:47PM +0200, Stefan Bader wrote:
> From: Xin Long <lucien.xin@gmail.com>
> 
> In commit ed17b8d377ea ("xfrm: fix a warning in xfrm_policy_insert_list"),
> it would take 'priority' to make a policy unique, and allow duplicated
> policies with different 'priority' to be added, which is not expected
> by userland, as Tobias reported in strongswan.
> 
> To fix this duplicated policies issue, and also fix the issue in
> commit ed17b8d377ea ("xfrm: fix a warning in xfrm_policy_insert_list"),
> when doing add/del/get/update on user interfaces, this patch is to change
> to look up a policy with both mark and mask by doing:
> 
>   mark.v == pol->mark.v && mark.m == pol->mark.m
> 
> and leave the check:
> 
>   (mark & pol->mark.m) == pol->mark.v
> 
> for tx/rx path only.
> 
> As the userland expects an exact mark and mask match to manage policies.
> 
> v1->v2:
>   - make xfrm_policy_mark_match inline and fix the changelog as
>     Tobias suggested.
> 
> Fixes: 295fae568885 ("xfrm: Allow user space manipulation of SPD mark")
> Fixes: ed17b8d377ea ("xfrm: fix a warning in xfrm_policy_insert_list")
> Reported-by: Tobias Brunner <tobias@strongswan.org>
> Tested-by: Tobias Brunner <tobias@strongswan.org>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1890796
> 
> (cherry picked from commit 4f47e8ab6ab796b5380f74866fa5287aca4dcc58)
> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> ---
> 
> This is for focal and bionic/hwe first, it seems the offending patch
> also went into bionic and xenial but starting with bionic there is some
> backport effort required to apply it. So submitting this first for
> the easy part.
> 
> -Stefan

Clean cherry-pick, simple test case.

Acked-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
Colin King Aug. 10, 2020, 3:26 p.m. UTC | #2
On 10/08/2020 15:44, Stefan Bader wrote:
> From: Xin Long <lucien.xin@gmail.com>
> 
> In commit ed17b8d377ea ("xfrm: fix a warning in xfrm_policy_insert_list"),
> it would take 'priority' to make a policy unique, and allow duplicated
> policies with different 'priority' to be added, which is not expected
> by userland, as Tobias reported in strongswan.
> 
> To fix this duplicated policies issue, and also fix the issue in
> commit ed17b8d377ea ("xfrm: fix a warning in xfrm_policy_insert_list"),
> when doing add/del/get/update on user interfaces, this patch is to change
> to look up a policy with both mark and mask by doing:
> 
>   mark.v == pol->mark.v && mark.m == pol->mark.m
> 
> and leave the check:
> 
>   (mark & pol->mark.m) == pol->mark.v
> 
> for tx/rx path only.
> 
> As the userland expects an exact mark and mask match to manage policies.
> 
> v1->v2:
>   - make xfrm_policy_mark_match inline and fix the changelog as
>     Tobias suggested.
> 
> Fixes: 295fae568885 ("xfrm: Allow user space manipulation of SPD mark")
> Fixes: ed17b8d377ea ("xfrm: fix a warning in xfrm_policy_insert_list")
> Reported-by: Tobias Brunner <tobias@strongswan.org>
> Tested-by: Tobias Brunner <tobias@strongswan.org>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1890796
> 
> (cherry picked from commit 4f47e8ab6ab796b5380f74866fa5287aca4dcc58)
> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> ---
> 
> This is for focal and bionic/hwe first, it seems the offending patch
> also went into bionic and xenial but starting with bionic there is some
> backport effort required to apply it. So submitting this first for
> the easy part.
> 
> -Stefan
> 
> 
>  include/net/xfrm.h     | 11 +++++++----
>  net/key/af_key.c       |  4 ++--
>  net/xfrm/xfrm_policy.c | 39 ++++++++++++++++-----------------------
>  net/xfrm/xfrm_user.c   | 18 +++++++++++-------
>  4 files changed, 36 insertions(+), 36 deletions(-)
> 
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index c7d213c9f9d8..5c20953c8deb 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -1630,13 +1630,16 @@ int xfrm_policy_walk(struct net *net, struct xfrm_policy_walk *walk,
>  		     void *);
>  void xfrm_policy_walk_done(struct xfrm_policy_walk *walk, struct net *net);
>  int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl);
> -struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u32 if_id,
> -					  u8 type, int dir,
> +struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net,
> +					  const struct xfrm_mark *mark,
> +					  u32 if_id, u8 type, int dir,
>  					  struct xfrm_selector *sel,
>  					  struct xfrm_sec_ctx *ctx, int delete,
>  					  int *err);
> -struct xfrm_policy *xfrm_policy_byid(struct net *net, u32 mark, u32 if_id, u8,
> -				     int dir, u32 id, int delete, int *err);
> +struct xfrm_policy *xfrm_policy_byid(struct net *net,
> +				     const struct xfrm_mark *mark, u32 if_id,
> +				     u8 type, int dir, u32 id, int delete,
> +				     int *err);
>  int xfrm_policy_flush(struct net *net, u8 type, bool task_valid);
>  void xfrm_policy_hash_rebuild(struct net *net);
>  u32 xfrm_get_acqseq(void);
> diff --git a/net/key/af_key.c b/net/key/af_key.c
> index b67ed3a8486c..979c579afc63 100644
> --- a/net/key/af_key.c
> +++ b/net/key/af_key.c
> @@ -2400,7 +2400,7 @@ static int pfkey_spddelete(struct sock *sk, struct sk_buff *skb, const struct sa
>  			return err;
>  	}
>  
> -	xp = xfrm_policy_bysel_ctx(net, DUMMY_MARK, 0, XFRM_POLICY_TYPE_MAIN,
> +	xp = xfrm_policy_bysel_ctx(net, &dummy_mark, 0, XFRM_POLICY_TYPE_MAIN,
>  				   pol->sadb_x_policy_dir - 1, &sel, pol_ctx,
>  				   1, &err);
>  	security_xfrm_policy_free(pol_ctx);
> @@ -2651,7 +2651,7 @@ static int pfkey_spdget(struct sock *sk, struct sk_buff *skb, const struct sadb_
>  		return -EINVAL;
>  
>  	delete = (hdr->sadb_msg_type == SADB_X_SPDDELETE2);
> -	xp = xfrm_policy_byid(net, DUMMY_MARK, 0, XFRM_POLICY_TYPE_MAIN,
> +	xp = xfrm_policy_byid(net, &dummy_mark, 0, XFRM_POLICY_TYPE_MAIN,
>  			      dir, pol->sadb_x_policy_id, delete, &err);
>  	if (xp == NULL)
>  		return -ENOENT;
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index 564aa6492e7c..6847b3579f54 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -1433,14 +1433,10 @@ static void xfrm_policy_requeue(struct xfrm_policy *old,
>  	spin_unlock_bh(&pq->hold_queue.lock);
>  }
>  
> -static bool xfrm_policy_mark_match(struct xfrm_policy *policy,
> -				   struct xfrm_policy *pol)
> +static inline bool xfrm_policy_mark_match(const struct xfrm_mark *mark,
> +					  struct xfrm_policy *pol)
>  {
> -	if (policy->mark.v == pol->mark.v &&
> -	    policy->priority == pol->priority)
> -		return true;
> -
> -	return false;
> +	return mark->v == pol->mark.v && mark->m == pol->mark.m;
>  }
>  
>  static u32 xfrm_pol_bin_key(const void *data, u32 len, u32 seed)
> @@ -1503,7 +1499,7 @@ static void xfrm_policy_insert_inexact_list(struct hlist_head *chain,
>  		if (pol->type == policy->type &&
>  		    pol->if_id == policy->if_id &&
>  		    !selector_cmp(&pol->selector, &policy->selector) &&
> -		    xfrm_policy_mark_match(policy, pol) &&
> +		    xfrm_policy_mark_match(&policy->mark, pol) &&
>  		    xfrm_sec_ctx_match(pol->security, policy->security) &&
>  		    !WARN_ON(delpol)) {
>  			delpol = pol;
> @@ -1538,7 +1534,7 @@ static struct xfrm_policy *xfrm_policy_insert_list(struct hlist_head *chain,
>  		if (pol->type == policy->type &&
>  		    pol->if_id == policy->if_id &&
>  		    !selector_cmp(&pol->selector, &policy->selector) &&
> -		    xfrm_policy_mark_match(policy, pol) &&
> +		    xfrm_policy_mark_match(&policy->mark, pol) &&
>  		    xfrm_sec_ctx_match(pol->security, policy->security) &&
>  		    !WARN_ON(delpol)) {
>  			if (excl)
> @@ -1610,9 +1606,8 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)
>  EXPORT_SYMBOL(xfrm_policy_insert);
>  
>  static struct xfrm_policy *
> -__xfrm_policy_bysel_ctx(struct hlist_head *chain, u32 mark, u32 if_id,
> -			u8 type, int dir,
> -			struct xfrm_selector *sel,
> +__xfrm_policy_bysel_ctx(struct hlist_head *chain, const struct xfrm_mark *mark,
> +			u32 if_id, u8 type, int dir, struct xfrm_selector *sel,
>  			struct xfrm_sec_ctx *ctx)
>  {
>  	struct xfrm_policy *pol;
> @@ -1623,7 +1618,7 @@ __xfrm_policy_bysel_ctx(struct hlist_head *chain, u32 mark, u32 if_id,
>  	hlist_for_each_entry(pol, chain, bydst) {
>  		if (pol->type == type &&
>  		    pol->if_id == if_id &&
> -		    (mark & pol->mark.m) == pol->mark.v &&
> +		    xfrm_policy_mark_match(mark, pol) &&
>  		    !selector_cmp(sel, &pol->selector) &&
>  		    xfrm_sec_ctx_match(ctx, pol->security))
>  			return pol;
> @@ -1632,11 +1627,10 @@ __xfrm_policy_bysel_ctx(struct hlist_head *chain, u32 mark, u32 if_id,
>  	return NULL;
>  }
>  
> -struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u32 if_id,
> -					  u8 type, int dir,
> -					  struct xfrm_selector *sel,
> -					  struct xfrm_sec_ctx *ctx, int delete,
> -					  int *err)
> +struct xfrm_policy *
> +xfrm_policy_bysel_ctx(struct net *net, const struct xfrm_mark *mark, u32 if_id,
> +		      u8 type, int dir, struct xfrm_selector *sel,
> +		      struct xfrm_sec_ctx *ctx, int delete, int *err)
>  {
>  	struct xfrm_pol_inexact_bin *bin = NULL;
>  	struct xfrm_policy *pol, *ret = NULL;
> @@ -1703,9 +1697,9 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u32 if_id,
>  }
>  EXPORT_SYMBOL(xfrm_policy_bysel_ctx);
>  
> -struct xfrm_policy *xfrm_policy_byid(struct net *net, u32 mark, u32 if_id,
> -				     u8 type, int dir, u32 id, int delete,
> -				     int *err)
> +struct xfrm_policy *
> +xfrm_policy_byid(struct net *net, const struct xfrm_mark *mark, u32 if_id,
> +		 u8 type, int dir, u32 id, int delete, int *err)
>  {
>  	struct xfrm_policy *pol, *ret;
>  	struct hlist_head *chain;
> @@ -1720,8 +1714,7 @@ struct xfrm_policy *xfrm_policy_byid(struct net *net, u32 mark, u32 if_id,
>  	ret = NULL;
>  	hlist_for_each_entry(pol, chain, byidx) {
>  		if (pol->type == type && pol->index == id &&
> -		    pol->if_id == if_id &&
> -		    (mark & pol->mark.m) == pol->mark.v) {
> +		    pol->if_id == if_id && xfrm_policy_mark_match(mark, pol)) {
>  			xfrm_pol_hold(pol);
>  			if (delete) {
>  				*err = security_xfrm_policy_delete(
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index e6cfaa680ef3..fbb7d9d06478 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -1863,7 +1863,6 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
>  	struct km_event c;
>  	int delete;
>  	struct xfrm_mark m;
> -	u32 mark = xfrm_mark_get(attrs, &m);
>  	u32 if_id = 0;
>  
>  	p = nlmsg_data(nlh);
> @@ -1880,8 +1879,11 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
>  	if (attrs[XFRMA_IF_ID])
>  		if_id = nla_get_u32(attrs[XFRMA_IF_ID]);
>  
> +	xfrm_mark_get(attrs, &m);
> +
>  	if (p->index)
> -		xp = xfrm_policy_byid(net, mark, if_id, type, p->dir, p->index, delete, &err);
> +		xp = xfrm_policy_byid(net, &m, if_id, type, p->dir,
> +				      p->index, delete, &err);
>  	else {
>  		struct nlattr *rt = attrs[XFRMA_SEC_CTX];
>  		struct xfrm_sec_ctx *ctx;
> @@ -1898,8 +1900,8 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
>  			if (err)
>  				return err;
>  		}
> -		xp = xfrm_policy_bysel_ctx(net, mark, if_id, type, p->dir, &p->sel,
> -					   ctx, delete, &err);
> +		xp = xfrm_policy_bysel_ctx(net, &m, if_id, type, p->dir,
> +					   &p->sel, ctx, delete, &err);
>  		security_xfrm_policy_free(ctx);
>  	}
>  	if (xp == NULL)
> @@ -2166,7 +2168,6 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
>  	u8 type = XFRM_POLICY_TYPE_MAIN;
>  	int err = -ENOENT;
>  	struct xfrm_mark m;
> -	u32 mark = xfrm_mark_get(attrs, &m);
>  	u32 if_id = 0;
>  
>  	err = copy_from_user_policy_type(&type, attrs);
> @@ -2180,8 +2181,11 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
>  	if (attrs[XFRMA_IF_ID])
>  		if_id = nla_get_u32(attrs[XFRMA_IF_ID]);
>  
> +	xfrm_mark_get(attrs, &m);
> +
>  	if (p->index)
> -		xp = xfrm_policy_byid(net, mark, if_id, type, p->dir, p->index, 0, &err);
> +		xp = xfrm_policy_byid(net, &m, if_id, type, p->dir, p->index,
> +				      0, &err);
>  	else {
>  		struct nlattr *rt = attrs[XFRMA_SEC_CTX];
>  		struct xfrm_sec_ctx *ctx;
> @@ -2198,7 +2202,7 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
>  			if (err)
>  				return err;
>  		}
> -		xp = xfrm_policy_bysel_ctx(net, mark, if_id, type, p->dir,
> +		xp = xfrm_policy_bysel_ctx(net, &m, if_id, type, p->dir,
>  					   &p->sel, ctx, 0, &err);
>  		security_xfrm_policy_free(ctx);
>  	}
> 

Clean upstream cherry pick. Test case looks sane.

Acked-by: Colin Ian King <colin.king@canonical.com>
Kelsey Skunberg Aug. 10, 2020, 7:18 p.m. UTC | #3
Applied to Focal/master-next with the BugLink moved to the top. Thank
you!

-Kelsey

On 2020-08-10 16:44:47 , Stefan Bader wrote:
> From: Xin Long <lucien.xin@gmail.com>
> 
> In commit ed17b8d377ea ("xfrm: fix a warning in xfrm_policy_insert_list"),
> it would take 'priority' to make a policy unique, and allow duplicated
> policies with different 'priority' to be added, which is not expected
> by userland, as Tobias reported in strongswan.
> 
> To fix this duplicated policies issue, and also fix the issue in
> commit ed17b8d377ea ("xfrm: fix a warning in xfrm_policy_insert_list"),
> when doing add/del/get/update on user interfaces, this patch is to change
> to look up a policy with both mark and mask by doing:
> 
>   mark.v == pol->mark.v && mark.m == pol->mark.m
> 
> and leave the check:
> 
>   (mark & pol->mark.m) == pol->mark.v
> 
> for tx/rx path only.
> 
> As the userland expects an exact mark and mask match to manage policies.
> 
> v1->v2:
>   - make xfrm_policy_mark_match inline and fix the changelog as
>     Tobias suggested.
> 
> Fixes: 295fae568885 ("xfrm: Allow user space manipulation of SPD mark")
> Fixes: ed17b8d377ea ("xfrm: fix a warning in xfrm_policy_insert_list")
> Reported-by: Tobias Brunner <tobias@strongswan.org>
> Tested-by: Tobias Brunner <tobias@strongswan.org>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1890796
> 
> (cherry picked from commit 4f47e8ab6ab796b5380f74866fa5287aca4dcc58)
> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> ---
> 
> This is for focal and bionic/hwe first, it seems the offending patch
> also went into bionic and xenial but starting with bionic there is some
> backport effort required to apply it. So submitting this first for
> the easy part.
> 
> -Stefan
> 
> 
>  include/net/xfrm.h     | 11 +++++++----
>  net/key/af_key.c       |  4 ++--
>  net/xfrm/xfrm_policy.c | 39 ++++++++++++++++-----------------------
>  net/xfrm/xfrm_user.c   | 18 +++++++++++-------
>  4 files changed, 36 insertions(+), 36 deletions(-)
> 
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index c7d213c9f9d8..5c20953c8deb 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -1630,13 +1630,16 @@ int xfrm_policy_walk(struct net *net, struct xfrm_policy_walk *walk,
>  		     void *);
>  void xfrm_policy_walk_done(struct xfrm_policy_walk *walk, struct net *net);
>  int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl);
> -struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u32 if_id,
> -					  u8 type, int dir,
> +struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net,
> +					  const struct xfrm_mark *mark,
> +					  u32 if_id, u8 type, int dir,
>  					  struct xfrm_selector *sel,
>  					  struct xfrm_sec_ctx *ctx, int delete,
>  					  int *err);
> -struct xfrm_policy *xfrm_policy_byid(struct net *net, u32 mark, u32 if_id, u8,
> -				     int dir, u32 id, int delete, int *err);
> +struct xfrm_policy *xfrm_policy_byid(struct net *net,
> +				     const struct xfrm_mark *mark, u32 if_id,
> +				     u8 type, int dir, u32 id, int delete,
> +				     int *err);
>  int xfrm_policy_flush(struct net *net, u8 type, bool task_valid);
>  void xfrm_policy_hash_rebuild(struct net *net);
>  u32 xfrm_get_acqseq(void);
> diff --git a/net/key/af_key.c b/net/key/af_key.c
> index b67ed3a8486c..979c579afc63 100644
> --- a/net/key/af_key.c
> +++ b/net/key/af_key.c
> @@ -2400,7 +2400,7 @@ static int pfkey_spddelete(struct sock *sk, struct sk_buff *skb, const struct sa
>  			return err;
>  	}
>  
> -	xp = xfrm_policy_bysel_ctx(net, DUMMY_MARK, 0, XFRM_POLICY_TYPE_MAIN,
> +	xp = xfrm_policy_bysel_ctx(net, &dummy_mark, 0, XFRM_POLICY_TYPE_MAIN,
>  				   pol->sadb_x_policy_dir - 1, &sel, pol_ctx,
>  				   1, &err);
>  	security_xfrm_policy_free(pol_ctx);
> @@ -2651,7 +2651,7 @@ static int pfkey_spdget(struct sock *sk, struct sk_buff *skb, const struct sadb_
>  		return -EINVAL;
>  
>  	delete = (hdr->sadb_msg_type == SADB_X_SPDDELETE2);
> -	xp = xfrm_policy_byid(net, DUMMY_MARK, 0, XFRM_POLICY_TYPE_MAIN,
> +	xp = xfrm_policy_byid(net, &dummy_mark, 0, XFRM_POLICY_TYPE_MAIN,
>  			      dir, pol->sadb_x_policy_id, delete, &err);
>  	if (xp == NULL)
>  		return -ENOENT;
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index 564aa6492e7c..6847b3579f54 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -1433,14 +1433,10 @@ static void xfrm_policy_requeue(struct xfrm_policy *old,
>  	spin_unlock_bh(&pq->hold_queue.lock);
>  }
>  
> -static bool xfrm_policy_mark_match(struct xfrm_policy *policy,
> -				   struct xfrm_policy *pol)
> +static inline bool xfrm_policy_mark_match(const struct xfrm_mark *mark,
> +					  struct xfrm_policy *pol)
>  {
> -	if (policy->mark.v == pol->mark.v &&
> -	    policy->priority == pol->priority)
> -		return true;
> -
> -	return false;
> +	return mark->v == pol->mark.v && mark->m == pol->mark.m;
>  }
>  
>  static u32 xfrm_pol_bin_key(const void *data, u32 len, u32 seed)
> @@ -1503,7 +1499,7 @@ static void xfrm_policy_insert_inexact_list(struct hlist_head *chain,
>  		if (pol->type == policy->type &&
>  		    pol->if_id == policy->if_id &&
>  		    !selector_cmp(&pol->selector, &policy->selector) &&
> -		    xfrm_policy_mark_match(policy, pol) &&
> +		    xfrm_policy_mark_match(&policy->mark, pol) &&
>  		    xfrm_sec_ctx_match(pol->security, policy->security) &&
>  		    !WARN_ON(delpol)) {
>  			delpol = pol;
> @@ -1538,7 +1534,7 @@ static struct xfrm_policy *xfrm_policy_insert_list(struct hlist_head *chain,
>  		if (pol->type == policy->type &&
>  		    pol->if_id == policy->if_id &&
>  		    !selector_cmp(&pol->selector, &policy->selector) &&
> -		    xfrm_policy_mark_match(policy, pol) &&
> +		    xfrm_policy_mark_match(&policy->mark, pol) &&
>  		    xfrm_sec_ctx_match(pol->security, policy->security) &&
>  		    !WARN_ON(delpol)) {
>  			if (excl)
> @@ -1610,9 +1606,8 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)
>  EXPORT_SYMBOL(xfrm_policy_insert);
>  
>  static struct xfrm_policy *
> -__xfrm_policy_bysel_ctx(struct hlist_head *chain, u32 mark, u32 if_id,
> -			u8 type, int dir,
> -			struct xfrm_selector *sel,
> +__xfrm_policy_bysel_ctx(struct hlist_head *chain, const struct xfrm_mark *mark,
> +			u32 if_id, u8 type, int dir, struct xfrm_selector *sel,
>  			struct xfrm_sec_ctx *ctx)
>  {
>  	struct xfrm_policy *pol;
> @@ -1623,7 +1618,7 @@ __xfrm_policy_bysel_ctx(struct hlist_head *chain, u32 mark, u32 if_id,
>  	hlist_for_each_entry(pol, chain, bydst) {
>  		if (pol->type == type &&
>  		    pol->if_id == if_id &&
> -		    (mark & pol->mark.m) == pol->mark.v &&
> +		    xfrm_policy_mark_match(mark, pol) &&
>  		    !selector_cmp(sel, &pol->selector) &&
>  		    xfrm_sec_ctx_match(ctx, pol->security))
>  			return pol;
> @@ -1632,11 +1627,10 @@ __xfrm_policy_bysel_ctx(struct hlist_head *chain, u32 mark, u32 if_id,
>  	return NULL;
>  }
>  
> -struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u32 if_id,
> -					  u8 type, int dir,
> -					  struct xfrm_selector *sel,
> -					  struct xfrm_sec_ctx *ctx, int delete,
> -					  int *err)
> +struct xfrm_policy *
> +xfrm_policy_bysel_ctx(struct net *net, const struct xfrm_mark *mark, u32 if_id,
> +		      u8 type, int dir, struct xfrm_selector *sel,
> +		      struct xfrm_sec_ctx *ctx, int delete, int *err)
>  {
>  	struct xfrm_pol_inexact_bin *bin = NULL;
>  	struct xfrm_policy *pol, *ret = NULL;
> @@ -1703,9 +1697,9 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u32 if_id,
>  }
>  EXPORT_SYMBOL(xfrm_policy_bysel_ctx);
>  
> -struct xfrm_policy *xfrm_policy_byid(struct net *net, u32 mark, u32 if_id,
> -				     u8 type, int dir, u32 id, int delete,
> -				     int *err)
> +struct xfrm_policy *
> +xfrm_policy_byid(struct net *net, const struct xfrm_mark *mark, u32 if_id,
> +		 u8 type, int dir, u32 id, int delete, int *err)
>  {
>  	struct xfrm_policy *pol, *ret;
>  	struct hlist_head *chain;
> @@ -1720,8 +1714,7 @@ struct xfrm_policy *xfrm_policy_byid(struct net *net, u32 mark, u32 if_id,
>  	ret = NULL;
>  	hlist_for_each_entry(pol, chain, byidx) {
>  		if (pol->type == type && pol->index == id &&
> -		    pol->if_id == if_id &&
> -		    (mark & pol->mark.m) == pol->mark.v) {
> +		    pol->if_id == if_id && xfrm_policy_mark_match(mark, pol)) {
>  			xfrm_pol_hold(pol);
>  			if (delete) {
>  				*err = security_xfrm_policy_delete(
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index e6cfaa680ef3..fbb7d9d06478 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -1863,7 +1863,6 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
>  	struct km_event c;
>  	int delete;
>  	struct xfrm_mark m;
> -	u32 mark = xfrm_mark_get(attrs, &m);
>  	u32 if_id = 0;
>  
>  	p = nlmsg_data(nlh);
> @@ -1880,8 +1879,11 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
>  	if (attrs[XFRMA_IF_ID])
>  		if_id = nla_get_u32(attrs[XFRMA_IF_ID]);
>  
> +	xfrm_mark_get(attrs, &m);
> +
>  	if (p->index)
> -		xp = xfrm_policy_byid(net, mark, if_id, type, p->dir, p->index, delete, &err);
> +		xp = xfrm_policy_byid(net, &m, if_id, type, p->dir,
> +				      p->index, delete, &err);
>  	else {
>  		struct nlattr *rt = attrs[XFRMA_SEC_CTX];
>  		struct xfrm_sec_ctx *ctx;
> @@ -1898,8 +1900,8 @@ static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
>  			if (err)
>  				return err;
>  		}
> -		xp = xfrm_policy_bysel_ctx(net, mark, if_id, type, p->dir, &p->sel,
> -					   ctx, delete, &err);
> +		xp = xfrm_policy_bysel_ctx(net, &m, if_id, type, p->dir,
> +					   &p->sel, ctx, delete, &err);
>  		security_xfrm_policy_free(ctx);
>  	}
>  	if (xp == NULL)
> @@ -2166,7 +2168,6 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
>  	u8 type = XFRM_POLICY_TYPE_MAIN;
>  	int err = -ENOENT;
>  	struct xfrm_mark m;
> -	u32 mark = xfrm_mark_get(attrs, &m);
>  	u32 if_id = 0;
>  
>  	err = copy_from_user_policy_type(&type, attrs);
> @@ -2180,8 +2181,11 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
>  	if (attrs[XFRMA_IF_ID])
>  		if_id = nla_get_u32(attrs[XFRMA_IF_ID]);
>  
> +	xfrm_mark_get(attrs, &m);
> +
>  	if (p->index)
> -		xp = xfrm_policy_byid(net, mark, if_id, type, p->dir, p->index, 0, &err);
> +		xp = xfrm_policy_byid(net, &m, if_id, type, p->dir, p->index,
> +				      0, &err);
>  	else {
>  		struct nlattr *rt = attrs[XFRMA_SEC_CTX];
>  		struct xfrm_sec_ctx *ctx;
> @@ -2198,7 +2202,7 @@ static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
>  			if (err)
>  				return err;
>  		}
> -		xp = xfrm_policy_bysel_ctx(net, mark, if_id, type, p->dir,
> +		xp = xfrm_policy_bysel_ctx(net, &m, if_id, type, p->dir,
>  					   &p->sel, ctx, 0, &err);
>  		security_xfrm_policy_free(ctx);
>  	}
> -- 
> 2.25.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff mbox series

Patch

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index c7d213c9f9d8..5c20953c8deb 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1630,13 +1630,16 @@  int xfrm_policy_walk(struct net *net, struct xfrm_policy_walk *walk,
 		     void *);
 void xfrm_policy_walk_done(struct xfrm_policy_walk *walk, struct net *net);
 int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl);
-struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u32 if_id,
-					  u8 type, int dir,
+struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net,
+					  const struct xfrm_mark *mark,
+					  u32 if_id, u8 type, int dir,
 					  struct xfrm_selector *sel,
 					  struct xfrm_sec_ctx *ctx, int delete,
 					  int *err);
-struct xfrm_policy *xfrm_policy_byid(struct net *net, u32 mark, u32 if_id, u8,
-				     int dir, u32 id, int delete, int *err);
+struct xfrm_policy *xfrm_policy_byid(struct net *net,
+				     const struct xfrm_mark *mark, u32 if_id,
+				     u8 type, int dir, u32 id, int delete,
+				     int *err);
 int xfrm_policy_flush(struct net *net, u8 type, bool task_valid);
 void xfrm_policy_hash_rebuild(struct net *net);
 u32 xfrm_get_acqseq(void);
diff --git a/net/key/af_key.c b/net/key/af_key.c
index b67ed3a8486c..979c579afc63 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -2400,7 +2400,7 @@  static int pfkey_spddelete(struct sock *sk, struct sk_buff *skb, const struct sa
 			return err;
 	}
 
-	xp = xfrm_policy_bysel_ctx(net, DUMMY_MARK, 0, XFRM_POLICY_TYPE_MAIN,
+	xp = xfrm_policy_bysel_ctx(net, &dummy_mark, 0, XFRM_POLICY_TYPE_MAIN,
 				   pol->sadb_x_policy_dir - 1, &sel, pol_ctx,
 				   1, &err);
 	security_xfrm_policy_free(pol_ctx);
@@ -2651,7 +2651,7 @@  static int pfkey_spdget(struct sock *sk, struct sk_buff *skb, const struct sadb_
 		return -EINVAL;
 
 	delete = (hdr->sadb_msg_type == SADB_X_SPDDELETE2);
-	xp = xfrm_policy_byid(net, DUMMY_MARK, 0, XFRM_POLICY_TYPE_MAIN,
+	xp = xfrm_policy_byid(net, &dummy_mark, 0, XFRM_POLICY_TYPE_MAIN,
 			      dir, pol->sadb_x_policy_id, delete, &err);
 	if (xp == NULL)
 		return -ENOENT;
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 564aa6492e7c..6847b3579f54 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1433,14 +1433,10 @@  static void xfrm_policy_requeue(struct xfrm_policy *old,
 	spin_unlock_bh(&pq->hold_queue.lock);
 }
 
-static bool xfrm_policy_mark_match(struct xfrm_policy *policy,
-				   struct xfrm_policy *pol)
+static inline bool xfrm_policy_mark_match(const struct xfrm_mark *mark,
+					  struct xfrm_policy *pol)
 {
-	if (policy->mark.v == pol->mark.v &&
-	    policy->priority == pol->priority)
-		return true;
-
-	return false;
+	return mark->v == pol->mark.v && mark->m == pol->mark.m;
 }
 
 static u32 xfrm_pol_bin_key(const void *data, u32 len, u32 seed)
@@ -1503,7 +1499,7 @@  static void xfrm_policy_insert_inexact_list(struct hlist_head *chain,
 		if (pol->type == policy->type &&
 		    pol->if_id == policy->if_id &&
 		    !selector_cmp(&pol->selector, &policy->selector) &&
-		    xfrm_policy_mark_match(policy, pol) &&
+		    xfrm_policy_mark_match(&policy->mark, pol) &&
 		    xfrm_sec_ctx_match(pol->security, policy->security) &&
 		    !WARN_ON(delpol)) {
 			delpol = pol;
@@ -1538,7 +1534,7 @@  static struct xfrm_policy *xfrm_policy_insert_list(struct hlist_head *chain,
 		if (pol->type == policy->type &&
 		    pol->if_id == policy->if_id &&
 		    !selector_cmp(&pol->selector, &policy->selector) &&
-		    xfrm_policy_mark_match(policy, pol) &&
+		    xfrm_policy_mark_match(&policy->mark, pol) &&
 		    xfrm_sec_ctx_match(pol->security, policy->security) &&
 		    !WARN_ON(delpol)) {
 			if (excl)
@@ -1610,9 +1606,8 @@  int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl)
 EXPORT_SYMBOL(xfrm_policy_insert);
 
 static struct xfrm_policy *
-__xfrm_policy_bysel_ctx(struct hlist_head *chain, u32 mark, u32 if_id,
-			u8 type, int dir,
-			struct xfrm_selector *sel,
+__xfrm_policy_bysel_ctx(struct hlist_head *chain, const struct xfrm_mark *mark,
+			u32 if_id, u8 type, int dir, struct xfrm_selector *sel,
 			struct xfrm_sec_ctx *ctx)
 {
 	struct xfrm_policy *pol;
@@ -1623,7 +1618,7 @@  __xfrm_policy_bysel_ctx(struct hlist_head *chain, u32 mark, u32 if_id,
 	hlist_for_each_entry(pol, chain, bydst) {
 		if (pol->type == type &&
 		    pol->if_id == if_id &&
-		    (mark & pol->mark.m) == pol->mark.v &&
+		    xfrm_policy_mark_match(mark, pol) &&
 		    !selector_cmp(sel, &pol->selector) &&
 		    xfrm_sec_ctx_match(ctx, pol->security))
 			return pol;
@@ -1632,11 +1627,10 @@  __xfrm_policy_bysel_ctx(struct hlist_head *chain, u32 mark, u32 if_id,
 	return NULL;
 }
 
-struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u32 if_id,
-					  u8 type, int dir,
-					  struct xfrm_selector *sel,
-					  struct xfrm_sec_ctx *ctx, int delete,
-					  int *err)
+struct xfrm_policy *
+xfrm_policy_bysel_ctx(struct net *net, const struct xfrm_mark *mark, u32 if_id,
+		      u8 type, int dir, struct xfrm_selector *sel,
+		      struct xfrm_sec_ctx *ctx, int delete, int *err)
 {
 	struct xfrm_pol_inexact_bin *bin = NULL;
 	struct xfrm_policy *pol, *ret = NULL;
@@ -1703,9 +1697,9 @@  struct xfrm_policy *xfrm_policy_bysel_ctx(struct net *net, u32 mark, u32 if_id,
 }
 EXPORT_SYMBOL(xfrm_policy_bysel_ctx);
 
-struct xfrm_policy *xfrm_policy_byid(struct net *net, u32 mark, u32 if_id,
-				     u8 type, int dir, u32 id, int delete,
-				     int *err)
+struct xfrm_policy *
+xfrm_policy_byid(struct net *net, const struct xfrm_mark *mark, u32 if_id,
+		 u8 type, int dir, u32 id, int delete, int *err)
 {
 	struct xfrm_policy *pol, *ret;
 	struct hlist_head *chain;
@@ -1720,8 +1714,7 @@  struct xfrm_policy *xfrm_policy_byid(struct net *net, u32 mark, u32 if_id,
 	ret = NULL;
 	hlist_for_each_entry(pol, chain, byidx) {
 		if (pol->type == type && pol->index == id &&
-		    pol->if_id == if_id &&
-		    (mark & pol->mark.m) == pol->mark.v) {
+		    pol->if_id == if_id && xfrm_policy_mark_match(mark, pol)) {
 			xfrm_pol_hold(pol);
 			if (delete) {
 				*err = security_xfrm_policy_delete(
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index e6cfaa680ef3..fbb7d9d06478 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1863,7 +1863,6 @@  static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
 	struct km_event c;
 	int delete;
 	struct xfrm_mark m;
-	u32 mark = xfrm_mark_get(attrs, &m);
 	u32 if_id = 0;
 
 	p = nlmsg_data(nlh);
@@ -1880,8 +1879,11 @@  static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (attrs[XFRMA_IF_ID])
 		if_id = nla_get_u32(attrs[XFRMA_IF_ID]);
 
+	xfrm_mark_get(attrs, &m);
+
 	if (p->index)
-		xp = xfrm_policy_byid(net, mark, if_id, type, p->dir, p->index, delete, &err);
+		xp = xfrm_policy_byid(net, &m, if_id, type, p->dir,
+				      p->index, delete, &err);
 	else {
 		struct nlattr *rt = attrs[XFRMA_SEC_CTX];
 		struct xfrm_sec_ctx *ctx;
@@ -1898,8 +1900,8 @@  static int xfrm_get_policy(struct sk_buff *skb, struct nlmsghdr *nlh,
 			if (err)
 				return err;
 		}
-		xp = xfrm_policy_bysel_ctx(net, mark, if_id, type, p->dir, &p->sel,
-					   ctx, delete, &err);
+		xp = xfrm_policy_bysel_ctx(net, &m, if_id, type, p->dir,
+					   &p->sel, ctx, delete, &err);
 		security_xfrm_policy_free(ctx);
 	}
 	if (xp == NULL)
@@ -2166,7 +2168,6 @@  static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 	u8 type = XFRM_POLICY_TYPE_MAIN;
 	int err = -ENOENT;
 	struct xfrm_mark m;
-	u32 mark = xfrm_mark_get(attrs, &m);
 	u32 if_id = 0;
 
 	err = copy_from_user_policy_type(&type, attrs);
@@ -2180,8 +2181,11 @@  static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (attrs[XFRMA_IF_ID])
 		if_id = nla_get_u32(attrs[XFRMA_IF_ID]);
 
+	xfrm_mark_get(attrs, &m);
+
 	if (p->index)
-		xp = xfrm_policy_byid(net, mark, if_id, type, p->dir, p->index, 0, &err);
+		xp = xfrm_policy_byid(net, &m, if_id, type, p->dir, p->index,
+				      0, &err);
 	else {
 		struct nlattr *rt = attrs[XFRMA_SEC_CTX];
 		struct xfrm_sec_ctx *ctx;
@@ -2198,7 +2202,7 @@  static int xfrm_add_pol_expire(struct sk_buff *skb, struct nlmsghdr *nlh,
 			if (err)
 				return err;
 		}
-		xp = xfrm_policy_bysel_ctx(net, mark, if_id, type, p->dir,
+		xp = xfrm_policy_bysel_ctx(net, &m, if_id, type, p->dir,
 					   &p->sel, ctx, 0, &err);
 		security_xfrm_policy_free(ctx);
 	}