Commit 1ab141d6 authored by Sabrina Dubroca's avatar Sabrina Dubroca Committed by Ben Hutchings

xfrm: fix stack access out of bounds with CONFIG_XFRM_SUB_POLICY

commit 9b3eb541 upstream.

When CONFIG_XFRM_SUB_POLICY=y, xfrm_dst stores a copy of the flowi for
that dst. Unfortunately, the code that allocates and fills this copy
doesn't care about what type of flowi (flowi, flowi4, flowi6) gets
passed. In multiple code paths (from raw_sendmsg, from TCP when
replying to a FIN, in vxlan, geneve, and gre), the flowi that gets
passed to xfrm is actually an on-stack flowi4, so we end up reading
stuff from the stack past the end of the flowi4 struct.

Since xfrm_dst->origin isn't used anywhere following commit
ca116922 ("xfrm: Eliminate "fl" and "pol" args to
xfrm_bundle_ok()."), just get rid of it.  xfrm_dst->partner isn't used
either, so get rid of that too.

Fixes: 9d6ec938 ("ipv4: Use flowi4 in public route lookup interfaces.")
Signed-off-by: default avatarSabrina Dubroca <sd@queasysnail.net>
Signed-off-by: default avatarSteffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: default avatarBen Hutchings <ben@decadent.org.uk>
parent 0d45844a
...@@ -949,10 +949,6 @@ struct xfrm_dst { ...@@ -949,10 +949,6 @@ struct xfrm_dst {
struct flow_cache_object flo; struct flow_cache_object flo;
struct xfrm_policy *pols[XFRM_POLICY_TYPE_MAX]; struct xfrm_policy *pols[XFRM_POLICY_TYPE_MAX];
int num_pols, num_xfrms; int num_pols, num_xfrms;
#ifdef CONFIG_XFRM_SUB_POLICY
struct flowi *origin;
struct xfrm_selector *partner;
#endif
u32 xfrm_genid; u32 xfrm_genid;
u32 policy_genid; u32 policy_genid;
u32 route_mtu_cached; u32 route_mtu_cached;
...@@ -968,12 +964,6 @@ static inline void xfrm_dst_destroy(struct xfrm_dst *xdst) ...@@ -968,12 +964,6 @@ static inline void xfrm_dst_destroy(struct xfrm_dst *xdst)
dst_release(xdst->route); dst_release(xdst->route);
if (likely(xdst->u.dst.xfrm)) if (likely(xdst->u.dst.xfrm))
xfrm_state_put(xdst->u.dst.xfrm); xfrm_state_put(xdst->u.dst.xfrm);
#ifdef CONFIG_XFRM_SUB_POLICY
kfree(xdst->origin);
xdst->origin = NULL;
kfree(xdst->partner);
xdst->partner = NULL;
#endif
} }
#endif #endif
......
...@@ -1634,43 +1634,6 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy, ...@@ -1634,43 +1634,6 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
goto out; goto out;
} }
#ifdef CONFIG_XFRM_SUB_POLICY
static int xfrm_dst_alloc_copy(void **target, const void *src, int size)
{
if (!*target) {
*target = kmalloc(size, GFP_ATOMIC);
if (!*target)
return -ENOMEM;
}
memcpy(*target, src, size);
return 0;
}
#endif
static int xfrm_dst_update_parent(struct dst_entry *dst,
const struct xfrm_selector *sel)
{
#ifdef CONFIG_XFRM_SUB_POLICY
struct xfrm_dst *xdst = (struct xfrm_dst *)dst;
return xfrm_dst_alloc_copy((void **)&(xdst->partner),
sel, sizeof(*sel));
#else
return 0;
#endif
}
static int xfrm_dst_update_origin(struct dst_entry *dst,
const struct flowi *fl)
{
#ifdef CONFIG_XFRM_SUB_POLICY
struct xfrm_dst *xdst = (struct xfrm_dst *)dst;
return xfrm_dst_alloc_copy((void **)&(xdst->origin), fl, sizeof(*fl));
#else
return 0;
#endif
}
static int xfrm_expand_policies(const struct flowi *fl, u16 family, static int xfrm_expand_policies(const struct flowi *fl, u16 family,
struct xfrm_policy **pols, struct xfrm_policy **pols,
int *num_pols, int *num_xfrms) int *num_pols, int *num_xfrms)
...@@ -1742,16 +1705,6 @@ xfrm_resolve_and_create_bundle(struct xfrm_policy **pols, int num_pols, ...@@ -1742,16 +1705,6 @@ xfrm_resolve_and_create_bundle(struct xfrm_policy **pols, int num_pols,
xdst = (struct xfrm_dst *)dst; xdst = (struct xfrm_dst *)dst;
xdst->num_xfrms = err; xdst->num_xfrms = err;
if (num_pols > 1)
err = xfrm_dst_update_parent(dst, &pols[1]->selector);
else
err = xfrm_dst_update_origin(dst, fl);
if (unlikely(err)) {
dst_free(dst);
XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTBUNDLECHECKERROR);
return ERR_PTR(err);
}
xdst->num_pols = num_pols; xdst->num_pols = num_pols;
memcpy(xdst->pols, pols, sizeof(struct xfrm_policy *) * num_pols); memcpy(xdst->pols, pols, sizeof(struct xfrm_policy *) * num_pols);
xdst->policy_genid = atomic_read(&pols[0]->genid); xdst->policy_genid = atomic_read(&pols[0]->genid);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment