Skip to content

Commit 398ac7a

Browse files
qsngregkh
authored andcommitted
xfrm: fix stack access out of bounds with CONFIG_XFRM_SUB_POLICY
commit 9b3eb54106cf6acd03f07cf0ab01c13676a226c2 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 ca11692 ("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: 9d6ec93 ("ipv4: Use flowi4 in public route lookup interfaces.") Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 736b342 commit 398ac7a

2 files changed

Lines changed: 0 additions & 57 deletions

File tree

include/net/xfrm.h

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -948,10 +948,6 @@ struct xfrm_dst {
948948
struct flow_cache_object flo;
949949
struct xfrm_policy *pols[XFRM_POLICY_TYPE_MAX];
950950
int num_pols, num_xfrms;
951-
#ifdef CONFIG_XFRM_SUB_POLICY
952-
struct flowi *origin;
953-
struct xfrm_selector *partner;
954-
#endif
955951
u32 xfrm_genid;
956952
u32 policy_genid;
957953
u32 route_mtu_cached;
@@ -967,12 +963,6 @@ static inline void xfrm_dst_destroy(struct xfrm_dst *xdst)
967963
dst_release(xdst->route);
968964
if (likely(xdst->u.dst.xfrm))
969965
xfrm_state_put(xdst->u.dst.xfrm);
970-
#ifdef CONFIG_XFRM_SUB_POLICY
971-
kfree(xdst->origin);
972-
xdst->origin = NULL;
973-
kfree(xdst->partner);
974-
xdst->partner = NULL;
975-
#endif
976966
}
977967
#endif
978968

net/xfrm/xfrm_policy.c

Lines changed: 0 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1776,43 +1776,6 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
17761776
goto out;
17771777
}
17781778

1779-
#ifdef CONFIG_XFRM_SUB_POLICY
1780-
static int xfrm_dst_alloc_copy(void **target, const void *src, int size)
1781-
{
1782-
if (!*target) {
1783-
*target = kmalloc(size, GFP_ATOMIC);
1784-
if (!*target)
1785-
return -ENOMEM;
1786-
}
1787-
1788-
memcpy(*target, src, size);
1789-
return 0;
1790-
}
1791-
#endif
1792-
1793-
static int xfrm_dst_update_parent(struct dst_entry *dst,
1794-
const struct xfrm_selector *sel)
1795-
{
1796-
#ifdef CONFIG_XFRM_SUB_POLICY
1797-
struct xfrm_dst *xdst = (struct xfrm_dst *)dst;
1798-
return xfrm_dst_alloc_copy((void **)&(xdst->partner),
1799-
sel, sizeof(*sel));
1800-
#else
1801-
return 0;
1802-
#endif
1803-
}
1804-
1805-
static int xfrm_dst_update_origin(struct dst_entry *dst,
1806-
const struct flowi *fl)
1807-
{
1808-
#ifdef CONFIG_XFRM_SUB_POLICY
1809-
struct xfrm_dst *xdst = (struct xfrm_dst *)dst;
1810-
return xfrm_dst_alloc_copy((void **)&(xdst->origin), fl, sizeof(*fl));
1811-
#else
1812-
return 0;
1813-
#endif
1814-
}
1815-
18161779
static int xfrm_expand_policies(const struct flowi *fl, u16 family,
18171780
struct xfrm_policy **pols,
18181781
int *num_pols, int *num_xfrms)
@@ -1884,16 +1847,6 @@ xfrm_resolve_and_create_bundle(struct xfrm_policy **pols, int num_pols,
18841847

18851848
xdst = (struct xfrm_dst *)dst;
18861849
xdst->num_xfrms = err;
1887-
if (num_pols > 1)
1888-
err = xfrm_dst_update_parent(dst, &pols[1]->selector);
1889-
else
1890-
err = xfrm_dst_update_origin(dst, fl);
1891-
if (unlikely(err)) {
1892-
dst_free(dst);
1893-
XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTBUNDLECHECKERROR);
1894-
return ERR_PTR(err);
1895-
}
1896-
18971850
xdst->num_pols = num_pols;
18981851
memcpy(xdst->pols, pols, sizeof(struct xfrm_policy *) * num_pols);
18991852
xdst->policy_genid = atomic_read(&pols[0]->genid);

0 commit comments

Comments
 (0)