Re: KASAN: stack-out-of-bounds Read in xfrm_state_find (3)

[Dmitry Vyukov]

On Fri, Dec 1, 2017 at 8:27 AM, Steffen Klassert
<steffen.klassert@xxxxxxxxxxx> wrote:
> On Wed, Nov 22, 2017 at 08:05:00AM -0800, syzbot wrote:
>> syzkaller has found reproducer for the following crash on
>> 0c86a6bd85ff0629cd2c5141027fc1c8bb6cde9c
>> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/master
>> compiler: gcc (GCC) 7.1.1 20170620
>> .config is attached
>> Raw console output is attached.
>> C reproducer is attached
>> syzkaller reproducer is attached. See https://goo.gl/kgGztJ
>> for information about syzkaller reproducers
>>
>>
>> BUG: KASAN: stack-out-of-bounds in xfrm_state_find+0x30fc/0x3230
>> net/xfrm/xfrm_state.c:1051
>> Read of size 4 at addr ffff8801ccaa7af8 by task syzkaller231684/3045
>
> The patch below should fix this. I plan to apply it to the ipsec tree
> after some advanced testing.


Please also follow this part:

> Once a fix for this bug is committed, please reply to this email with:
> #syz fix: exact-commit-title
> Note: all commands must start from beginning of the line in the email body.

This will greatly help keep the process running.

Thanks


> Subject: [PATCH RFC] xfrm: Fix stack-out-of-bounds with misconfigured transport
>  mode policies.
>
> On policies with a transport mode template, we pass the addresses
> from the flowi to xfrm_state_find(), assuming that the IP addresses
> (and address family) don't change during transformation.
>
> Unfortunately our policy template validation is not strict enough.
> It is possible to configure policies with transport mode template
> where the address family of the template does not match the selectors
> address family. This lead to stack-out-of-bound reads because
> we compare arddesses of the wrong family. Fix this by refusing
> such a configuration, address family can not change on transport
> mode.
>
> We use the assumption that, on transport mode, the first templates
> address family must match the address family of the policy selector.
> Subsequent transport mode templates must mach the address family of
> the previous template.
>
> Reported-by: syzbot <syzkaller@xxxxxxxxxxxxxxxx>
> Signed-off-by: Steffen Klassert <steffen.klassert@xxxxxxxxxxx>
> ---
>  net/xfrm/xfrm_user.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
> index 983b0233767b..57ad016ae675 100644
> --- a/net/xfrm/xfrm_user.c
> +++ b/net/xfrm/xfrm_user.c
> @@ -1419,11 +1419,14 @@ static void copy_templates(struct xfrm_policy *xp, struct xfrm_user_tmpl *ut,
>
>  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;
> +
>         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;
> +
> +               prev_family = ut[i].family;
> +
>                 switch (ut[i].family) {
>                 case AF_INET:
>                         break;
> --
> 2.14.1
>
> --
> 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@xxxxxxxxxxxxxxxxx
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20171201072743.47ztbmrql7ub327u%40gauss3.secunet.de.
> For more options, visit https://groups.google.com/d/optout.