Re: [PATCH net] ipv6: sctp: clone options to avoid use after free

2015-12-11 Thread David Miller
From: Eric Dumazet 
Date: Wed, 09 Dec 2015 07:25:06 -0800

> From: Eric Dumazet 
> 
> SCTP is lacking proper np->opt cloning at accept() time.
> 
> TCP and DCCP use ipv6_dup_options() helper, do the same
> in SCTP.
> 
> We might later factorize this code in a common helper to avoid
> future mistakes.
> 
> Reported-by: Dmitry Vyukov 
> Signed-off-by: Eric Dumazet 

Applied and queued up for -stable, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH net] ipv6: sctp: clone options to avoid use after free

2015-12-10 Thread David Laight
From: Daniel Borkmann
> Sent: 09 December 2015 19:19
> On 12/09/2015 06:11 PM, Marcelo Ricardo Leitner wrote:
> > Em 09-12-2015 14:31, David Laight escreveu:
> >> From: Eric Dumazet [mailto:eric.duma...@gmail.com]
> >>> Sent: 09 December 2015 16:00
> >>> On Wed, 2015-12-09 at 15:49 +, David Laight wrote:
> > SCTP is lacking proper np->opt cloning at accept() time.
> >
> > TCP and DCCP use ipv6_dup_options() helper, do the same in SCTP.
> >
> > We might later factorize this code in a common helper to avoid
> > future mistakes.
> 
>  I'm wondering what the real impact of this and the other recent
>  SCTP bugs/patches is on real workloads?
>  We have enough trouble getting our customers to use kernels
>  later that the 2.6.18 based RHEL5 - without having to persuade
>  them to use kernels that contain very recent fixes.
> >>>
> >>> It all depends if your customers let (hostile ?) people run programs on
> >>> the boxes.
> >>
> >> If they require hostile programs I'm not worried.
> >
> > Not really "require", but "allow", as in: allowing third-party applications 
> > to run on it.
> 
> Yeah :/ given distros enable almost everything anyway, the first unpriv'ed
> socket(..., IPPROTO_SCTP) call auto-loads SCTP module. But to be honest, I'd
> be surprised if Cloud providers allow for this. Most of this might only run
> on dedicated boxes with telco appliances.

Yes, I'm worried about whether our M3UA code is likely to crash customer
systems, not whether hostile applications can crash it.
These boxes ought to be on private networks since the sigtran protocols
themselves have nothing that even gives a hint of security.

David


RE: [PATCH net] ipv6: sctp: clone options to avoid use after free

2015-12-10 Thread David Laight
From: Eric Dumazet
> Sent: 10 December 2015 15:58
>
> BTW, are you even using IPv6 SCTP sessions ?

Our M3UA/SCTP protocol stack supports them and defaults to using
IPv6 listening sockets for IPv4 connections.

I very much doubt than any customers have used them yet.
So most of the IPv6 connections will have been to ::1
during internal regression testing.

We don't even try to set any IPv6 (or IPv4) options.

Just SO_REUSEADDR, TCP/SCTP_NODELAY, SCTP_EVENTS, SCTP_INITMSG,
SO_KEEPALIVE (tcp), IPV6_V6ONLY (if binding separate listeners),
SCTP_SOCKOPT_BINX_ADD (WTF is this a 'socket option') and
SO_LINGER (to get abortive close on SCTP connections on kernels
before 3.18).

David



Re: [PATCH net] ipv6: sctp: clone options to avoid use after free

2015-12-10 Thread Eric Dumazet
On Thu, 2015-12-10 at 12:26 +, David Laight wrote:

> Yes, I'm worried about whether our M3UA code is likely to crash customer
> systems, not whether hostile applications can crash it.
> These boxes ought to be on private networks since the sigtran protocols
> themselves have nothing that even gives a hint of security.

As long as the listener socket is kept as is, meaning that the only use
of it is the poll()/select()/accept() system calls, you are safe.

The bug is about having a fuzzer, specifically playing games with multi
threads so that the listener ipv6 options are changed after accept(). 

This should not really happen in real world applications : If ipv6
options need to be set on listener, they are set before first accept()
is performed, and not unset until application exits and kill all
sessions.

BTW, are you even using IPv6 SCTP sessions ?



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net] ipv6: sctp: clone options to avoid use after free

2015-12-09 Thread Vlad Yasevich
On 12/09/2015 10:25 AM, Eric Dumazet wrote:
> From: Eric Dumazet 
> 
> SCTP is lacking proper np->opt cloning at accept() time.
> 
> TCP and DCCP use ipv6_dup_options() helper, do the same
> in SCTP.
> 
> We might later factorize this code in a common helper to avoid
> future mistakes.
> 
> Reported-by: Dmitry Vyukov 
> Signed-off-by: Eric Dumazet 

Acked-by: Vlad Yasevich 

This is sufficient for accept() processing, but looks like peeloff is missing
a bunch of ipv6 support.  I'll see if I can cook something up to fix that part.

-vlad

> ---
>  net/sctp/ipv6.c |8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index d28c0b4c9128..ec529121f38a 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -641,6 +641,7 @@ static struct sock *sctp_v6_create_accept_sk(struct sock 
> *sk,
>   struct sock *newsk;
>   struct ipv6_pinfo *newnp, *np = inet6_sk(sk);
>   struct sctp6_sock *newsctp6sk;
> + struct ipv6_txoptions *opt;
>  
>   newsk = sk_alloc(sock_net(sk), PF_INET6, GFP_KERNEL, sk->sk_prot, 0);
>   if (!newsk)
> @@ -660,6 +661,13 @@ static struct sock *sctp_v6_create_accept_sk(struct sock 
> *sk,
>  
>   memcpy(newnp, np, sizeof(struct ipv6_pinfo));
>  
> + rcu_read_lock();
> + opt = rcu_dereference(np->opt);
> + if (opt)
> + opt = ipv6_dup_options(newsk, opt);
> + RCU_INIT_POINTER(newnp->opt, opt);
> + rcu_read_unlock();
> +
>   /* Initialize sk's sport, dport, rcv_saddr and daddr for getsockname()
>* and getpeername().
>*/
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH net] ipv6: sctp: clone options to avoid use after free

2015-12-09 Thread David Laight
> SCTP is lacking proper np->opt cloning at accept() time.
> 
> TCP and DCCP use ipv6_dup_options() helper, do the same in SCTP.
> 
> We might later factorize this code in a common helper to avoid
> future mistakes.

I'm wondering what the real impact of this and the other recent
SCTP bugs/patches is on real workloads?
We have enough trouble getting our customers to use kernels
later that the 2.6.18 based RHEL5 - without having to persuade
them to use kernels that contain very recent fixes.

David



Re: [PATCH net] ipv6: sctp: clone options to avoid use after free

2015-12-09 Thread Eric Dumazet
On Wed, 2015-12-09 at 15:49 +, David Laight wrote:
> > SCTP is lacking proper np->opt cloning at accept() time.
> > 
> > TCP and DCCP use ipv6_dup_options() helper, do the same in SCTP.
> > 
> > We might later factorize this code in a common helper to avoid
> > future mistakes.
> 
> I'm wondering what the real impact of this and the other recent
> SCTP bugs/patches is on real workloads?
> We have enough trouble getting our customers to use kernels
> later that the 2.6.18 based RHEL5 - without having to persuade
> them to use kernels that contain very recent fixes.

It all depends if your customers let (hostile ?) people run programs on
the boxes.

I wont run the program on my laptop just for the fun of making it crash,
but I guess Cloud providers might be worried, once exploits are public.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH net] ipv6: sctp: clone options to avoid use after free

2015-12-09 Thread David Laight
From: Eric Dumazet [mailto:eric.duma...@gmail.com]
> Sent: 09 December 2015 16:00
> On Wed, 2015-12-09 at 15:49 +, David Laight wrote:
> > > SCTP is lacking proper np->opt cloning at accept() time.
> > >
> > > TCP and DCCP use ipv6_dup_options() helper, do the same in SCTP.
> > >
> > > We might later factorize this code in a common helper to avoid
> > > future mistakes.
> >
> > I'm wondering what the real impact of this and the other recent
> > SCTP bugs/patches is on real workloads?
> > We have enough trouble getting our customers to use kernels
> > later that the 2.6.18 based RHEL5 - without having to persuade
> > them to use kernels that contain very recent fixes.
> 
> It all depends if your customers let (hostile ?) people run programs on
> the boxes.

If they require hostile programs I'm not worried.

But it isn't entirely clear from these oops reports what the
test program is actually doing.
Some of them might be valid scenarios.
Not that our code does anything clever.

David



Re: [PATCH net] ipv6: sctp: clone options to avoid use after free

2015-12-09 Thread Daniel Borkmann

On 12/09/2015 06:11 PM, Marcelo Ricardo Leitner wrote:

Em 09-12-2015 14:31, David Laight escreveu:

From: Eric Dumazet [mailto:eric.duma...@gmail.com]

Sent: 09 December 2015 16:00
On Wed, 2015-12-09 at 15:49 +, David Laight wrote:

SCTP is lacking proper np->opt cloning at accept() time.

TCP and DCCP use ipv6_dup_options() helper, do the same in SCTP.

We might later factorize this code in a common helper to avoid
future mistakes.


I'm wondering what the real impact of this and the other recent
SCTP bugs/patches is on real workloads?
We have enough trouble getting our customers to use kernels
later that the 2.6.18 based RHEL5 - without having to persuade
them to use kernels that contain very recent fixes.


It all depends if your customers let (hostile ?) people run programs on
the boxes.


If they require hostile programs I'm not worried.


Not really "require", but "allow", as in: allowing third-party applications to 
run on it.


Yeah :/ given distros enable almost everything anyway, the first unpriv'ed
socket(..., IPPROTO_SCTP) call auto-loads SCTP module. But to be honest, I'd
be surprised if Cloud providers allow for this. Most of this might only run
on dedicated boxes with telco appliances.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net] ipv6: sctp: clone options to avoid use after free

2015-12-09 Thread Marcelo Ricardo Leitner

Em 09-12-2015 14:31, David Laight escreveu:

From: Eric Dumazet [mailto:eric.duma...@gmail.com]

Sent: 09 December 2015 16:00
On Wed, 2015-12-09 at 15:49 +, David Laight wrote:

SCTP is lacking proper np->opt cloning at accept() time.

TCP and DCCP use ipv6_dup_options() helper, do the same in SCTP.

We might later factorize this code in a common helper to avoid
future mistakes.


I'm wondering what the real impact of this and the other recent
SCTP bugs/patches is on real workloads?
We have enough trouble getting our customers to use kernels
later that the 2.6.18 based RHEL5 - without having to persuade
them to use kernels that contain very recent fixes.


It all depends if your customers let (hostile ?) people run programs on
the boxes.


If they require hostile programs I'm not worried.


Not really "require", but "allow", as in: allowing third-party 
applications to run on it.



But it isn't entirely clear from these oops reports what the
test program is actually doing.
Some of them might be valid scenarios.
Not that our code does anything clever.


This one patched by Eric is a rather common scenario and can lead to 
memory corruption and even double-frees because it will call txopt_put() 
on a buffer that is long gone due to the blind pointer copy. Note that 
Vlad is checking peeloff operation yet regarding this.
You just have to use the right SOL_IPV6 options and accept a socket to 
trigger it.


The timestamps I patched are also not an unusual scenario and allow one 
to be able to disable packet stamping for the entire system. Another 
effect would be to one enable stamping on listening socket and not gett 
it automatically on accepted ones, but that's probably a minor compared 
to the others because developers probably would have caught this already.


And this last one I'm still fixing, is mostly harmless so far. The 
use-after-free happens only in a debug statement (read: not propagated 
anywhere else) and it's just a read.


HTH!

  Marcelo

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html