From: Vlad Yasevich <vyasevich@gmail•com>
To: Marcelo Ricardo Leitner <marcelo.leitner@gmail•com>,
Dmitry Vyukov <dvyukov@google•com>
Cc: Joe Perches <joe@perches•com>, Jason Baron <jbaron@akamai•com>,
Aaron Conole <aconole@redhat•com>,
Andrew Morton <akpm@linux-foundation•org>,
LKML <linux-kernel@vger•kernel.org>,
Eric Dumazet <edumazet@google•com>,
syzkaller <syzkaller@googlegroups•com>,
linux-sctp@vger•kernel.org, netdev <netdev@vger•kernel.org>,
Kostya Serebryany <kcc@google•com>,
Alexander Potapenko <glider@google•com>,
Sasha Levin <sasha.levin@oracle•com>
Subject: Re: use-after-free in sctp_do_sm
Date: Fri, 4 Dec 2015 10:37:26 -0500 [thread overview]
Message-ID: <5661B336.8090007@gmail.com> (raw)
In-Reply-To: <20151204125543.GG4164@mrl.redhat.com>
On 12/04/2015 07:55 AM, Marcelo Ricardo Leitner wrote:
> On Fri, Dec 04, 2015 at 11:40:02AM +0100, Dmitry Vyukov wrote:
>> On Thu, Dec 3, 2015 at 9:51 PM, Joe Perches <joe@perches•com> wrote:
>>> (adding lkml as this is likely better discussed there)
>>>
>>> On Thu, 2015-12-03 at 15:42 -0500, Jason Baron wrote:
>>>> On 12/03/2015 03:24 PM, Joe Perches wrote:
>>>>> On Thu, 2015-12-03 at 15:10 -0500, Jason Baron wrote:
>>>>>> On 12/03/2015 03:03 PM, Joe Perches wrote:
>>>>>>> On Thu, 2015-12-03 at 14:32 -0500, Jason Baron wrote:
>>>>>>>> On 12/03/2015 01:52 PM, Aaron Conole wrote:
>>>>>>>>> I think that as a minimum, the following patch should be evaluted,
>>>>>>>>> but am unsure to whom I should submit it (after I test):
>>>>>>> []
>>>>>>>> Agreed - the intention here is certainly to have no side effects. It
>>>>>>>> looks like 'no_printk()' is used in quite a few other places that would
>>>>>>>> benefit from this change. So we probably want a generic
>>>>>>>> 'really_no_printk()' macro.
>>>>>>>
>>>>>>> https://lkml.org/lkml/2012/6/17/231
>>>>>>
>>>>>> I don't see this in the tree.
>>>>>
>>>>> It never got applied.
>>>>>
>>>>>> Also maybe we should just convert
>>>>>> no_printk() to do what your 'eliminated_printk()'.
>>>>>
>>>>> Some of them at least.
>>>>>
>>>>>> So we can convert all users with this change?
>>>>>
>>>>> I don't think so, I think there are some
>>>>> function evaluation/side effects that are
>>>>> required. I believe some do hardware I/O.
>>>>>
>>>>> It'd be good to at least isolate them.
>>>>>
>>>>> I'm not sure how to find them via some
>>>>> automated tool/mechanism though.
>>>>>
>>>>> I asked Julia Lawall about it once in this
>>>>> thread: https://lkml.org/lkml/2014/12/3/696
>>>>>
>>>>
>>>> Seems rather fragile to have side effects that we rely
>>>> upon hidden in a printk().
>>>
>>> Yup.
>>>
>>>> Just convert them and see what breaks :)
>>>
>>> I appreciate your optimism. It's very 1995.
>>> Try it and see what happens.
>>
>>
>> Whatever is the resolution for pr_debug, we still need to fix this
>> particular use-after-free. It affects stability of debug builds, gives
>> invalid debug output, prevents us from finding more bugs in SCTP. And
>> maybe somebody uses CONFIG_DYNAMIC_DEBUG in production.
>
> Agreed. I'm already working on a fix for this particular use-after-free.
>
> Another interesting thing about this is that sctp_do_sm() is called for
> nearly every movement that happens on a sctp socket. Said that, that
> always-running IDR search hidden on that debug statement do have some
> nasty performance impact, specially because it's serialized on a
> spinlock.
YUCK! I didn't really pay much attention to those debug macros before, but
debug_post_sfx() is truly awful.
This wasn't such a bad thing where these macros depended on CONFIG_SCTP_DEBUG,
but now that they are always built, we need fix them.
-vlad
> This wouldn't be happening if it was fully ellided and would
> be ok if that pr_debug() was really being printed, but not as it is.
> Kudos to this report that I could notice this. I'm trying to fix this on
> SCTP-side as well.
>
> Marcelo
>
next prev parent reply other threads:[~2015-12-04 15:37 UTC|newest]
Thread overview: 77+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-24 9:15 use-after-free in sctp_do_sm Dmitry Vyukov
2015-11-24 9:31 ` Dmitry Vyukov
2015-11-24 10:10 ` Dmitry Vyukov
2015-11-24 20:45 ` Neil Horman
2015-11-24 21:08 ` Eric Dumazet
2015-11-24 21:12 ` David Miller
2015-11-25 15:12 ` Vlad Yasevich
2015-11-28 15:50 ` Dmitry Vyukov
2015-12-03 16:51 ` Marcelo Ricardo Leitner
2015-12-03 17:43 ` Marcelo Ricardo Leitner
2015-12-03 17:59 ` Eric Dumazet
2015-12-03 18:06 ` Marcelo
2015-12-03 18:35 ` Vlad Yasevich
2015-12-03 18:43 ` Marcelo
2015-12-04 17:14 ` [PATCH net 0/3] sctp: packet timestamp fixes Marcelo Ricardo Leitner
2015-12-04 17:14 ` [PATCH net 1/3] sctp: use the same clock as if sock source timestamps were on Marcelo Ricardo Leitner
2015-12-04 20:31 ` Vlad Yasevich
2015-12-04 17:14 ` [PATCH net 2/3] sctp: update the netstamp_needed counter when copying sockets Marcelo Ricardo Leitner
2015-12-04 20:33 ` Vlad Yasevich
2015-12-04 17:14 ` [PATCH net 3/3] sctp: also copy sk_tsflags when copying the socket Marcelo Ricardo Leitner
2015-12-04 20:33 ` Vlad Yasevich
2015-12-06 3:24 ` [PATCH net 0/3] sctp: packet timestamp fixes David Miller
2015-12-03 13:05 ` use-after-free in sctp_do_sm Marcelo Ricardo Leitner
2015-12-03 13:45 ` Dmitry Vyukov
2015-12-03 14:48 ` Eric Dumazet
2015-12-03 15:55 ` Dmitry Vyukov
2015-12-03 16:15 ` Marcelo Ricardo Leitner
2015-12-03 17:02 ` Eric Dumazet
2015-12-03 17:12 ` Dmitry Vyukov
2015-12-03 18:52 ` Aaron Conole
2015-12-03 19:06 ` Joe Perches
2015-12-03 19:32 ` Jason Baron
2015-12-03 20:03 ` Joe Perches
2015-12-03 20:10 ` Jason Baron
2015-12-03 20:24 ` Joe Perches
2015-12-03 20:42 ` Jason Baron
2015-12-03 20:51 ` Joe Perches
2015-12-04 10:40 ` Dmitry Vyukov
2015-12-04 12:55 ` Marcelo Ricardo Leitner
2015-12-04 15:37 ` Vlad Yasevich [this message]
2015-12-04 15:51 ` Aaron Conole
2015-12-04 16:12 ` Dmitry Vyukov
2015-12-04 16:47 ` Jason Baron
2015-12-04 17:03 ` Joe Perches
2015-12-04 17:11 ` Jason Baron
2015-12-04 10:41 ` Dmitry Vyukov
2015-12-04 17:48 ` Marcelo Ricardo Leitner
2015-12-04 20:25 ` Dmitry Vyukov
2015-12-04 21:34 ` Marcelo Ricardo Leitner
2015-12-04 21:38 ` Dmitry Vyukov
2015-12-05 16:39 ` Vlad Yasevich
2015-12-07 11:26 ` Dmitry Vyukov
2015-12-07 13:15 ` Marcelo Ricardo Leitner
2015-12-07 13:20 ` Dmitry Vyukov
2015-12-07 18:52 ` Marcelo Ricardo Leitner
2015-12-07 19:33 ` Vlad Yasevich
2015-12-07 19:50 ` Marcelo Ricardo Leitner
2015-12-07 20:37 ` Vlad Yasevich
2015-12-07 20:52 ` Marcelo Ricardo Leitner
2015-12-08 17:30 ` Dmitry Vyukov
2015-12-08 17:40 ` Marcelo Ricardo Leitner
2015-12-08 19:22 ` Dmitry Vyukov
2015-12-09 14:41 ` Dmitry Vyukov
2015-12-09 15:03 ` Marcelo Ricardo Leitner
2015-12-09 16:41 ` Marcelo Ricardo Leitner
2015-12-11 13:35 ` Dmitry Vyukov
2015-12-11 13:51 ` Marcelo Ricardo Leitner
2015-12-11 14:03 ` Marcelo Ricardo Leitner
2015-12-11 14:30 ` Dmitry Vyukov
2015-12-11 15:55 ` Marcelo Ricardo Leitner
2016-01-08 13:00 ` [PATCH] sctp: fix use-after-free in pr_debug statement Marcelo Ricardo Leitner
2016-01-11 17:00 ` Vlad Yasevich
2016-01-11 22:13 ` David Miller
2016-01-12 8:41 ` Dmitry Vyukov
2015-12-11 18:37 ` use-after-free in sctp_do_sm Vlad Yasevich
2015-12-14 9:50 ` David Laight
2015-12-14 14:25 ` Vlad Yasevich
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=5661B336.8090007@gmail.com \
--to=vyasevich@gmail$(echo .)com \
--cc=aconole@redhat$(echo .)com \
--cc=akpm@linux-foundation$(echo .)org \
--cc=dvyukov@google$(echo .)com \
--cc=edumazet@google$(echo .)com \
--cc=glider@google$(echo .)com \
--cc=jbaron@akamai$(echo .)com \
--cc=joe@perches$(echo .)com \
--cc=kcc@google$(echo .)com \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-sctp@vger$(echo .)kernel.org \
--cc=marcelo.leitner@gmail$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=sasha.levin@oracle$(echo .)com \
--cc=syzkaller@googlegroups$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox