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

[Eric Biggers]

On Wed, Dec 13, 2017 at 06:18:05AM +0100, Steffen Klassert wrote:
> On Tue, Dec 12, 2017 at 01:00:31PM -0800, Eric Biggers wrote:
> > Hi Steffen,
> > 
> > On Fri, Dec 01, 2017 at 08:27:43AM +0100, Steffen Klassert 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 8801ccaa7af8 by task syzkaller231684/3045
> > > 
> > > The patch below should fix this. I plan to apply it to the ipsec tree
> > > after some advanced testing.
> > > 
> > > Subject: [PATCH RFC] xfrm: Fix stack-out-of-bounds with misconfigured 
> > > transport
> > >  mode policies.
> > > 
> > 
> > Are you still planning to apply this?  syzbot is still hitting this bug.
> 
> It is already applied to the ipsec tree, will go upstream by the end of
> this week.
>

Marking this fixed for syzbot:

#syz fix: xfrm: Fix stack-out-of-bounds with misconfigured transport mode 
policies.

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

[Steffen Klassert]

On Tue, Dec 12, 2017 at 01:00:31PM -0800, Eric Biggers wrote:
> Hi Steffen,
> 
> On Fri, Dec 01, 2017 at 08:27:43AM +0100, Steffen Klassert 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 8801ccaa7af8 by task syzkaller231684/3045
> > 
> > The patch below should fix this. I plan to apply it to the ipsec tree
> > after some advanced testing.
> > 
> > Subject: [PATCH RFC] xfrm: Fix stack-out-of-bounds with misconfigured 
> > transport
> >  mode policies.
> > 
> 
> Are you still planning to apply this?  syzbot is still hitting this bug.

It is already applied to the ipsec tree, will go upstream by the end of
this week.

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

[Eric Biggers]

Hi Steffen,

On Fri, Dec 01, 2017 at 08:27:43AM +0100, Steffen Klassert 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 8801ccaa7af8 by task syzkaller231684/3045
> 
> The patch below should fix this. I plan to apply it to the ipsec tree
> after some advanced testing.
> 
> Subject: [PATCH RFC] xfrm: Fix stack-out-of-bounds with misconfigured 
> transport
>  mode policies.
> 

Are you still planning to apply this?  syzbot is still hitting this bug.

Eric

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
 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 8801ccaa7af8 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 
> Signed-off-by: Steffen Klassert 
> ---
>  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+unsubscr...@googlegroups.com.
> 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.

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

[Steffen Klassert]

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 8801ccaa7af8 by task syzkaller231684/3045

The patch below should fix this. I plan to apply it to the ipsec tree
after some advanced testing.

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 
Signed-off-by: Steffen Klassert 
---
 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