On Mon, Nov 27, 2017 at 11:39:40AM +0100, Florian Westphal wrote: > Steffen Klassert <steffen.klassert@xxxxxxxxxxx> wrote: > > > > In between I think our template validation is not strict enough. > > It is possible to configure policies with transport mode template > > where the selector address family does not match the templates > > address family. The address family can not change on a transport > > mode transformation, so this configuration does not make much > > sense but lead to problems because we use the assumption that > > the address family can not change on thransport mode later on. > > > > Unfortunately the reproducer provided by syzcaller does not > > trigger anything on my test setup, so I don't even know if > > this fixes this exact problem. I had to update my compliler, now I can reproduce the problem. > > > > Florian, could you please give the patch blelow a try? > > Doesn't help. It does not fix this problem, but the one referenced here: https://lkml.org/lkml/2017/11/22/414 I knew it fixes something :-) The bug we are talking about here should be fixed with the patch below. Subject: [PATCH] xfrm: Fix stack-out-of-bounds read on socket policy lookup. When we do tunnel or beet mode, we pass saddr and daddr from the template to xfrm_state_find(), this is ok. On transport mode, we pass the addresses from the flowi, assuming that the IP addresses (and address family) don't change during transformation. This assumption is wrong in the IPv4 mapped IPv6 case, packet is IPv4 and template is IPv6. Fix this by catching address family missmatches of the policy and the flow already before we do the lookup. Signed-off-by: Steffen Klassert <steffen.klassert@xxxxxxxxxxx> --- net/xfrm/xfrm_policy.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index 9542975eb2f9..038ec68f6901 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -1168,9 +1168,15 @@ static struct xfrm_policy *xfrm_sk_policy_lookup(const struct sock *sk, int dir, again: pol = rcu_dereference(sk->sk_policy[dir]); if (pol != NULL) { - bool match = xfrm_selector_match(&pol->selector, fl, family); + bool match; int err = 0; + if (pol->family != family) { + pol = NULL; + goto out; + } + + match = xfrm_selector_match(&pol->selector, fl, family); if (match) { if ((sk->sk_mark & pol->mark.m) != pol->mark.v) { pol = NULL; -- 2.14.1