All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
To: Xin Long <lucien.xin@gmail.com>
Cc: Andrey Konovalov <andreyknvl@google.com>,
	Vlad Yasevich <vyasevich@gmail.com>,
	Neil Horman <nhorman@tuxdriver.com>,
	"David S. Miller" <davem@davemloft.net>,
	linux-sctp@vger.kernel.org, netdev <netdev@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	Eric Dumazet <edumazet@google.com>,
	Kostya Serebryany <kcc@google.com>,
	syzkaller <syzkaller@googlegroups.com>
Subject: Re: net/sctp: list double add warning in sctp_endpoint_add_asoc
Date: Tue, 4 Apr 2017 18:14:54 -0300	[thread overview]
Message-ID: <20170404211454.GA911@localhost.localdomain> (raw)
In-Reply-To: <CADvbK_f1FEGRnucuMkQT6RKm0G1tKcfHyj=-GqxgzQ4iQoBLCg@mail.gmail.com>

On Wed, Apr 05, 2017 at 01:29:19AM +0800, Xin Long wrote:
> On Tue, Apr 4, 2017 at 9:28 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> > Hi,
> >
> > I've got the following error report while fuzzing the kernel with syzkaller.
> >
> > On commit a71c9a1c779f2499fb2afc0553e543f18aff6edf (4.11-rc5).
> >
> > A reproducer and .config are attached.
> The script is pretty hard to reproduce the issue in my env.

I didn't try running it but I also found the reproducer very complicated
to follow. Do you have any plans on having some PoC optimizer, so we can
have a more readable code?
strace is handy for filtering the noise, yes, but sometimes it doesn't
cut it.

> But there seems a case to cause a use-after-free when out of snd_buf.
> 
> the case is like:
> -----------
> one thread:                       another thread:
>                                   sctp_rcv hold asoc (hold transport)
>                                   enqueue the chunk to backlog queue
>                                   [refcnt=2]
> 
> sctp_close free assoc
> [refcnt=1]
> 
> sctp_sendmsg find asoc
> but not hold it
> 
> out of snd_buf
> hold asoc, schedule out
> [refcnt = 2]
> 
>                                   process backlog and put asoc/transport
>                                   [refcnt=1]
> 
> schedule in, put asoc
> [refcnt=0] <--- destroyed
> 
> sctp_sendmsg continue

It shouldn't be continuing here because sctp_wait_for_sndbuf and
sctp_wait_for_connect functions are checking if the asoc is dead
already when it schedules in, even though sctp_wait_for_connect return
value is ignored and sctp_sendmsg() simply returns after that.
Or the checks for dead asocs in there aren't enough somehow.

> using asoc, panic

WARNING: multiple messages have this Message-ID (diff)
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
To: Xin Long <lucien.xin@gmail.com>
Cc: Andrey Konovalov <andreyknvl@google.com>,
	Vlad Yasevich <vyasevich@gmail.com>,
	Neil Horman <nhorman@tuxdriver.com>,
	"David S. Miller" <davem@davemloft.net>,
	linux-sctp@vger.kernel.org, netdev <netdev@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	Eric Dumazet <edumazet@google.com>,
	Kostya Serebryany <kcc@google.com>,
	syzkaller <syzkaller@googlegroups.com>
Subject: Re: net/sctp: list double add warning in sctp_endpoint_add_asoc
Date: Tue, 04 Apr 2017 21:14:54 +0000	[thread overview]
Message-ID: <20170404211454.GA911@localhost.localdomain> (raw)
In-Reply-To: <CADvbK_f1FEGRnucuMkQT6RKm0G1tKcfHyj=-GqxgzQ4iQoBLCg@mail.gmail.com>

On Wed, Apr 05, 2017 at 01:29:19AM +0800, Xin Long wrote:
> On Tue, Apr 4, 2017 at 9:28 PM, Andrey Konovalov <andreyknvl@google.com> wrote:
> > Hi,
> >
> > I've got the following error report while fuzzing the kernel with syzkaller.
> >
> > On commit a71c9a1c779f2499fb2afc0553e543f18aff6edf (4.11-rc5).
> >
> > A reproducer and .config are attached.
> The script is pretty hard to reproduce the issue in my env.

I didn't try running it but I also found the reproducer very complicated
to follow. Do you have any plans on having some PoC optimizer, so we can
have a more readable code?
strace is handy for filtering the noise, yes, but sometimes it doesn't
cut it.

> But there seems a case to cause a use-after-free when out of snd_buf.
> 
> the case is like:
> -----------
> one thread:                       another thread:
>                                   sctp_rcv hold asoc (hold transport)
>                                   enqueue the chunk to backlog queue
>                                   [refcnt=2]
> 
> sctp_close free assoc
> [refcnt=1]
> 
> sctp_sendmsg find asoc
> but not hold it
> 
> out of snd_buf
> hold asoc, schedule out
> [refcnt = 2]
> 
>                                   process backlog and put asoc/transport
>                                   [refcnt=1]
> 
> schedule in, put asoc
> [refcnt=0] <--- destroyed
> 
> sctp_sendmsg continue

It shouldn't be continuing here because sctp_wait_for_sndbuf and
sctp_wait_for_connect functions are checking if the asoc is dead
already when it schedules in, even though sctp_wait_for_connect return
value is ignored and sctp_sendmsg() simply returns after that.
Or the checks for dead asocs in there aren't enough somehow.

> using asoc, panic



  reply	other threads:[~2017-04-04 21:15 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-04 13:28 net/sctp: list double add warning in sctp_endpoint_add_asoc Andrey Konovalov
2017-04-04 17:29 ` Xin Long
2017-04-04 17:29   ` Xin Long
2017-04-04 21:14   ` Marcelo Ricardo Leitner [this message]
2017-04-04 21:14     ` Marcelo Ricardo Leitner
2017-04-05 10:48     ` Xin Long
2017-04-05 10:48       ` Xin Long
2017-04-05 12:44       ` Marcelo Ricardo Leitner
2017-04-05 12:44         ` Marcelo Ricardo Leitner
2017-04-05 14:03         ` Andrey Konovalov
2017-04-05 14:03           ` Andrey Konovalov
2017-04-05 14:02     ` Andrey Konovalov
2017-04-05 14:02       ` Andrey Konovalov
2017-04-05 14:22       ` Marcelo Ricardo Leitner
2017-04-05 14:22         ` Marcelo Ricardo Leitner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170404211454.GA911@localhost.localdomain \
    --to=marcelo.leitner@gmail.com \
    --cc=andreyknvl@google.com \
    --cc=davem@davemloft.net \
    --cc=dvyukov@google.com \
    --cc=edumazet@google.com \
    --cc=kcc@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sctp@vger.kernel.org \
    --cc=lucien.xin@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=nhorman@tuxdriver.com \
    --cc=syzkaller@googlegroups.com \
    --cc=vyasevich@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.