@@ -815,10 +815,10 @@ xfrm_init_tempstate(struct xfrm_state *x, const struct flowi *fl,
afinfo->init_temprop(x, tmpl, daddr, saddr);
}
-static struct xfrm_state *__xfrm_state_lookup(struct net *net, u32 mark,
- const xfrm_address_t *daddr,
- __be32 spi, u8 proto,
- unsigned short family)
+static struct xfrm_state *
+__xfrm_state_lookup(struct net *net, u32 mark, u32 mask,
+ const xfrm_address_t *daddr,
+ __be32 spi, u8 proto, unsigned short family)
{
unsigned int h = xfrm_spi_hash(net, daddr, spi, proto, family);
struct xfrm_state *x;
@@ -830,7 +830,7 @@ static struct xfrm_state *__xfrm_state_lookup(struct net *net, u32 mark,
!xfrm_addr_equal(&x->id.daddr, daddr, family))
continue;
- if ((mark & x->mark.m) != x->mark.v)
+ if ((mark ^ x->mark.v) & mask & x->mark.m)
continue;
if (!xfrm_state_hold_rcu(x))
continue;
@@ -840,10 +840,11 @@ static struct xfrm_state *__xfrm_state_lookup(struct net *net, u32 mark,
return NULL;
}
-static struct xfrm_state *__xfrm_state_lookup_byaddr(struct net *net, u32 mark,
- const xfrm_address_t *daddr,
- const xfrm_address_t *saddr,
- u8 proto, unsigned short family)
+static struct xfrm_state *
+__xfrm_state_lookup_byaddr(struct net *net, u32 mark, u32 mask,
+ const xfrm_address_t *daddr,
+ const xfrm_address_t *saddr,
+ u8 proto, unsigned short family)
{
unsigned int h = xfrm_src_hash(net, daddr, saddr, family);
struct xfrm_state *x;
@@ -855,7 +856,7 @@ static struct xfrm_state *__xfrm_state_lookup_byaddr(struct net *net, u32 mark,
!xfrm_addr_equal(&x->props.saddr, saddr, family))
continue;
- if ((mark & x->mark.m) != x->mark.v)
+ if ((mark ^ x->mark.v) & mask & x->mark.m)
continue;
if (!xfrm_state_hold_rcu(x))
continue;
@@ -869,15 +870,14 @@ static inline struct xfrm_state *
__xfrm_state_locate(struct xfrm_state *x, int use_spi, int family)
{
struct net *net = xs_net(x);
- u32 mark = x->mark.v & x->mark.m;
if (use_spi)
- return __xfrm_state_lookup(net, mark, &x->id.daddr,
- x->id.spi, x->id.proto, family);
+ return __xfrm_state_lookup(net, x->mark.v, x->mark.m,
+ &x->id.daddr, x->id.spi,
+ x->id.proto, family);
else
- return __xfrm_state_lookup_byaddr(net, mark,
- &x->id.daddr,
- &x->props.saddr,
+ return __xfrm_state_lookup_byaddr(net, x->mark.v, x->mark.m,
+ &x->id.daddr, &x->props.saddr,
x->id.proto, family);
}
@@ -985,8 +985,10 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
x = best;
if (!x && !error && !acquire_in_progress) {
if (tmpl->id.spi &&
- (x0 = __xfrm_state_lookup(net, mark, daddr, tmpl->id.spi,
- tmpl->id.proto, encap_family)) != NULL) {
+ (x0 = __xfrm_state_lookup(net, mark, 0xFFFFFFFF,
+ daddr, tmpl->id.spi,
+ tmpl->id.proto,
+ encap_family)) != NULL) {
to_put = x0;
error = -EEXIST;
goto out;
@@ -1617,7 +1619,8 @@ xfrm_state_lookup(struct net *net, u32 mark, const xfrm_address_t *daddr, __be32
struct xfrm_state *x;
rcu_read_lock();
- x = __xfrm_state_lookup(net, mark, daddr, spi, proto, family);
+ x = __xfrm_state_lookup(net, mark, 0xFFFFFFFF,
+ daddr, spi, proto, family);
rcu_read_unlock();
return x;
}
@@ -1631,7 +1634,8 @@ xfrm_state_lookup_byaddr(struct net *net, u32 mark,
struct xfrm_state *x;
spin_lock_bh(&net->xfrm.xfrm_state_lock);
- x = __xfrm_state_lookup_byaddr(net, mark, daddr, saddr, proto, family);
+ x = __xfrm_state_lookup_byaddr(net, mark, 0xFFFFFFFF,
+ daddr, saddr, proto, family);
spin_unlock_bh(&net->xfrm.xfrm_state_lock);
return x;
}
It's possible to insert an SA into the SADB that will preclude the lookup of other SAs when using the MARK attribute. The problem occurs based on a particular sequencing with the marks where a new mark matches requests for the SA mark of an existing SA but the inverse is not true. For an example: 1) Add SA with mark=0, mask=0 2) Add an otherwise-identical SA with mark=0x1234, mask=0xFFFFFFFF This will fail; however, if done in the reverse order the second add will succeed: 1) Add an SA with mark=0x1234, mask=0xFFFFFFFF 2) Add an otherwise-identical SA except with mark=0, mask=0 Then: 3) Delete the SA using mark=0x1234, mask=0xFFFFFFF 4) Dump the SADB, and there will be one SA, and it will have mark=0x1234, mask=0xFFFFFFFF; the 0/0 SA will be deleted This patch addresses the problem by performing a reverse-match on the mark and preventing ADDSA for any SA that would 'shadow' an existing SA in the SADB. This patch also address a bug where it was possible to add an SA with a mark broader than its mask, which could never be deleted: 1) ADDSA with mark=0x1234, mask=0xFF 2) DELSA with mark=0x1234, mask=ZZZZ; error=ESRCH By applying both masks to each mark, bits outside the mask of a given SA mark will be ignored, and the match will succeed. This patch does not make any changes to the 'data' path, so SAs with such oddly-defined marks will still be unmatch-able. Signed-off-by: Nathan Harold <nharold@google.com> --- net/xfrm/xfrm_state.c | 44 +++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-)