netdev - Re: KASAN: stack-out-of-bounds Read in xfrm_state

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 6 Nov 2017 11:31:27 +0100
From:   Dmitry Vyukov <dvyukov@...gle.com>
To:     Steffen Klassert <steffen.klassert@...unet.com>
Cc:     Florian Westphal <fw@...len.de>,
        syzbot 
        <bot+19b21aa652248382e2b8cbb81fa1cdc03b4bda01@...kaller.appspotmail.com>,
        David Miller <davem@...emloft.net>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        LKML <linux-kernel@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>, syzkaller-bugs@...glegroups.com,
        thomas.egerer@...unet.com
Subject: Re: KASAN: stack-out-of-bounds Read in xfrm_state_find (2)

On Mon, Nov 6, 2017 at 11:16 AM, Steffen Klassert
<steffen.klassert@...unet.com> wrote:
> On Fri, Nov 03, 2017 at 01:10:12PM +0100, Steffen Klassert wrote:
>> On Thu, Nov 02, 2017 at 01:25:28PM +0100, Florian Westphal wrote:
>> > Steffen Klassert <steffen.klassert@...unet.com> wrote:
>> >
>> > > I'd propose to use the addresses from the template unconditionally,
>> > > like the (untested) patch below does.
>> > >
>> > > Unfortunalely the reproducer does not work with my config,
>> > > sendto returns EAGAIN. Could anybody try this patch?
>> >
>> > The reproducer no longer causes KASAN spew with your patch,
>> > but i don't have a test case that actually creates/uses a tunnel.
>>
>> The patch passed my standard tests, so I tend apply it
>> after a day in the ipsec/testing branch.
>
> FYI: I've just applied the patch below to the ipsec tree.


Thanks

Let's tell the bot what fixes this:

#syz fix: xfrm: Fix stack-out-of-bounds read in xfrm_state_find.

> Subject: [PATCH ipsec] xfrm: Fix stack-out-of-bounds read in xfrm_state_find.
>
> 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 using the addresses
> from the template unconditionally.
>
> Signed-off-by: Steffen Klassert <steffen.klassert@...unet.com>
> ---
>  net/xfrm/xfrm_policy.c | 29 +++++++++++------------------
>  1 file changed, 11 insertions(+), 18 deletions(-)
>
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index a2e531b..6eb228a 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -1361,36 +1361,29 @@ xfrm_tmpl_resolve_one(struct xfrm_policy *policy, const struct flowi *fl,
>         struct net *net = xp_net(policy);
>         int nx;
>         int i, error;
> -       xfrm_address_t *daddr = xfrm_flowi_daddr(fl, family);
> -       xfrm_address_t *saddr = xfrm_flowi_saddr(fl, family);
>         xfrm_address_t tmp;
>
>         for (nx = 0, i = 0; i < policy->xfrm_nr; i++) {
>                 struct xfrm_state *x;
> -               xfrm_address_t *remote = daddr;
> -               xfrm_address_t *local  = saddr;
> +               xfrm_address_t *local;
> +               xfrm_address_t *remote;
>                 struct xfrm_tmpl *tmpl = &policy->xfrm_vec[i];
>
> -               if (tmpl->mode == XFRM_MODE_TUNNEL ||
> -                   tmpl->mode == XFRM_MODE_BEET) {
> -                       remote = &tmpl->id.daddr;
> -                       local = &tmpl->saddr;
> -                       if (xfrm_addr_any(local, tmpl->encap_family)) {
> -                               error = xfrm_get_saddr(net, fl->flowi_oif,
> -                                                      &tmp, remote,
> -                                                      tmpl->encap_family, 0);
> -                               if (error)
> -                                       goto fail;
> -                               local = &tmp;
> -                       }
> +               remote = &tmpl->id.daddr;
> +               local = &tmpl->saddr;
> +               if (xfrm_addr_any(local, tmpl->encap_family)) {
> +                       error = xfrm_get_saddr(net, fl->flowi_oif,
> +                                              &tmp, remote,
> +                                              tmpl->encap_family, 0);
> +                       if (error)
> +                               goto fail;
> +                       local = &tmp;
>                 }
>
>                 x = xfrm_state_find(remote, local, fl, tmpl, policy, &error, family);
>
>                 if (x && x->km.state == XFRM_STATE_VALID) {
>                         xfrm[nx++] = x;
> -                       daddr = remote;
> -                       saddr = local;
>                         continue;
>                 }
>                 if (x) {
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@...glegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20171106101646.GG23855%40secunet.com.
> For more options, visit https://groups.google.com/d/optout.