Steffen Klassert <steffen.klassert@xxxxxxxxxxx> wrote:
> On Sat, Nov 25, 2017 at 04:50:31AM +0900, David Miller wrote:
> > From: Florian Westphal <fw@xxxxxxxxx>
> > Date: Fri, 24 Nov 2017 20:32:12 +0100
> >
> > > Tomas Charvat <tc@xxxxxxxxxx> wrote:
> > >
> > > [ CC stable, Steffen ]
> > >
> > >> Hi Florian and David, I'm running several servers that use XFRM ipsec.
> > >> It do work well on all kernels bellow 4.14.0.
> > >>
> > >> It doesnt work on 4.14.0-2. There is no any error in dmesg or in
> > >> userspace when I do configure policies.
> > >>
> > >> Since there is not much info about XFRM in dmesg I have no clue, where
> > >> to start when I want to debug this issue.
> > >
> > > David, please consider picking up
> > > 94802151894d482e82c324edf2c658f8e6b96508
> > > ("Revert "xfrm: Fix stack-out-of-bounds read in xfrm_state_find.")
> > >
> > > for the 4.14.y stable queue.
> > >
> > > I think its a pretty safe bet that this fixes the problem, it broke
> > > transport mode wildcard policy lookup.
> >
> > Ok, once we have confirmation that this fixes it I also need to pair
> > it up with Steffen's alternative fix for the bug that commit was
> > trying to fix.
>
> We need this revert in the 4.14.y stable tree anyway as it broke
> transport mode IPsec.
>
> I thought quite a lot about the original problem that I tried
> to fix. It is a rather subtile thing, like almost all bugs
> reported from syzcaller I have seen.
>
> 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.
>
> Florian, could you please give the patch blelow a try?
Doesn't help.
> static int validate_tmpl(int nr, struct xfrm_user_tmpl *ut, u16 family)
> {
> + u16 prev_family;
> int i;
>
> if (nr > XFRM_MAX_DEPTH)
> return -EINVAL;
>
> + prev_family = family;
family is AF_INET6 (10), nr is 1.
> for (i = 0; i < nr; i++) {
> /* We never validated the ut->family value, so many
> * applications simply leave it at zero. The check was
> @@ -1435,6 +1438,12 @@ static int validate_tmpl(int nr, struct xfrm_user_tmpl *ut, u16 family)
> if (!ut[i].family)
> ut[i].family = family;
>
> + if ((ut[i].mode == XFRM_MODE_TRANSPORT) &&
> + (ut[i].family != prev_family))
> + return -EINVAL;
mode is XFRM_MODE_TRANSPORT, family == prev_family (10), so no
-EINVAL here.
Socket is AF_INET6.
setsockopt(3, SOL_IPV6, 0x23 /* IPV6_??? */, ...
sendto(3, "", 0, MSG_EOR|MSG_NOSIGNAL|0x10, {sa_family=AF_INET,
sin_port=htons(20002), sin_addr=inet_addr("0.0.0.0")}, 16) = -1
EAGAIN (Resource temporarily unavailable)
and then used with AF_INET address, which causes the KASAN warning
as we feed xfrm_state_find with on-stack ipv4 addresses.