From: Aaron Conole <aconole@redhat.com> To: Vlad Yasevich <vyasevich@gmail.com> Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>, Dmitry Vyukov <dvyukov@google.com>, Joe Perches <joe@perches.com>, Jason Baron <jbaron@akamai.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, 04 Dec 2015 10:51:38 -0500 [thread overview] Message-ID: <f7ttwnyus8l.fsf@aconole.bos.csb> (raw) In-Reply-To: <5661B336.8090007@gmail.com> (Vlad Yasevich's message of "Fri, 4 Dec 2015 10:37:26 -0500") Vlad Yasevich <vyasevich@gmail.com> writes: > 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. I've proposed a patch to linux-kernel to fix them, but I don't think it's really as bad as folks imagine. Ubuntu, RHEL, and Fedora all use DYNAMIC_DEBUG configuration option, which means that the code is getting emitted anyway (correctly, I'll add) and is shunted out by a dynamic debug flag. So for the average user, it's not even really a blip. That does mean there's a cool side-effect of the entire print-macro setup which implies we execute less code when running with DYNAMIC_DEBUG=y in the "normal" case. "Turn on the dynamic debugging config and watch everything get better" isn't the worst mantra, is it? :) > -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 >>
WARNING: multiple messages have this Message-ID (diff)
From: Aaron Conole <aconole@redhat.com> To: Vlad Yasevich <vyasevich@gmail.com> Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>, Dmitry Vyukov <dvyukov@google.com>, Joe Perches <joe@perches.com>, Jason Baron <jbaron@akamai.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, 04 Dec 2015 15:51:38 +0000 [thread overview] Message-ID: <f7ttwnyus8l.fsf@aconole.bos.csb> (raw) In-Reply-To: <5661B336.8090007@gmail.com> (Vlad Yasevich's message of "Fri, 4 Dec 2015 10:37:26 -0500") Vlad Yasevich <vyasevich@gmail.com> writes: > 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. I've proposed a patch to linux-kernel to fix them, but I don't think it's really as bad as folks imagine. Ubuntu, RHEL, and Fedora all use DYNAMIC_DEBUG configuration option, which means that the code is getting emitted anyway (correctly, I'll add) and is shunted out by a dynamic debug flag. So for the average user, it's not even really a blip. That does mean there's a cool side-effect of the entire print-macro setup which implies we execute less code when running with DYNAMIC_DEBUG=y in the "normal" case. "Turn on the dynamic debugging config and watch everything get better" isn't the worst mantra, is it? :) > -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:51 UTC|newest] Thread overview: 153+ 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:15 ` Dmitry Vyukov 2015-11-24 9:31 ` Dmitry Vyukov 2015-11-24 9:31 ` Dmitry Vyukov 2015-11-24 10:10 ` Dmitry Vyukov 2015-11-24 10:10 ` Dmitry Vyukov 2015-11-24 20:45 ` Neil Horman 2015-11-24 20:45 ` Neil Horman 2015-11-24 21:08 ` Eric Dumazet 2015-11-24 21:08 ` Eric Dumazet 2015-11-24 21:12 ` David Miller 2015-11-24 21:12 ` David Miller 2015-11-25 15:12 ` Vlad Yasevich 2015-11-25 15:12 ` Vlad Yasevich 2015-11-28 15:50 ` Dmitry Vyukov 2015-11-28 15:50 ` Dmitry Vyukov 2015-12-03 16:51 ` Marcelo Ricardo Leitner 2015-12-03 16:51 ` Marcelo Ricardo Leitner 2015-12-03 17:43 ` Marcelo Ricardo Leitner 2015-12-03 17:43 ` Marcelo Ricardo Leitner 2015-12-03 17:59 ` Eric Dumazet 2015-12-03 17:59 ` Eric Dumazet 2015-12-03 18:06 ` Marcelo 2015-12-03 18:06 ` Marcelo 2015-12-03 18:35 ` Vlad Yasevich 2015-12-03 18:35 ` Vlad Yasevich 2015-12-03 18:43 ` Marcelo 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 ` 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 17:14 ` Marcelo Ricardo Leitner 2015-12-04 20:31 ` Vlad Yasevich 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 17:14 ` Marcelo Ricardo Leitner 2015-12-04 20:33 ` Vlad Yasevich 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 17:14 ` Marcelo Ricardo Leitner 2015-12-04 20:33 ` Vlad Yasevich 2015-12-04 20:33 ` Vlad Yasevich 2015-12-06 3:24 ` [PATCH net 0/3] sctp: packet timestamp fixes David Miller 2015-12-06 3:24 ` David Miller 2015-12-03 13:05 ` use-after-free in sctp_do_sm Marcelo Ricardo Leitner 2015-12-03 13:05 ` Marcelo Ricardo Leitner 2015-12-03 13:45 ` Dmitry Vyukov 2015-12-03 13:45 ` Dmitry Vyukov 2015-12-03 14:48 ` Eric Dumazet 2015-12-03 14:48 ` Eric Dumazet 2015-12-03 15:55 ` Dmitry Vyukov 2015-12-03 15:55 ` Dmitry Vyukov 2015-12-03 16:15 ` Marcelo Ricardo Leitner 2015-12-03 16:15 ` Marcelo Ricardo Leitner 2015-12-03 17:02 ` Eric Dumazet 2015-12-03 17:02 ` Eric Dumazet 2015-12-03 17:12 ` Dmitry Vyukov 2015-12-03 17:12 ` Dmitry Vyukov 2015-12-03 18:52 ` Aaron Conole 2015-12-03 18:52 ` Aaron Conole 2015-12-03 19:06 ` Joe Perches 2015-12-03 19:06 ` Joe Perches 2015-12-03 19:32 ` Jason Baron 2015-12-03 19:32 ` Jason Baron 2015-12-03 20:03 ` Joe Perches 2015-12-03 20:03 ` Joe Perches 2015-12-03 20:10 ` Jason Baron 2015-12-03 20:10 ` Jason Baron 2015-12-03 20:24 ` Joe Perches 2015-12-03 20:24 ` Joe Perches 2015-12-03 20:42 ` Jason Baron 2015-12-03 20:42 ` Jason Baron 2015-12-03 20:51 ` Joe Perches 2015-12-03 20:51 ` Joe Perches 2015-12-04 10:40 ` Dmitry Vyukov 2015-12-04 10:40 ` Dmitry Vyukov 2015-12-04 12:55 ` Marcelo Ricardo Leitner 2015-12-04 12:55 ` Marcelo Ricardo Leitner 2015-12-04 15:37 ` Vlad Yasevich 2015-12-04 15:37 ` Vlad Yasevich 2015-12-04 15:51 ` Aaron Conole [this message] 2015-12-04 15:51 ` Aaron Conole 2015-12-04 16:12 ` Dmitry Vyukov 2015-12-04 16:12 ` Dmitry Vyukov 2015-12-04 16:47 ` Jason Baron 2015-12-04 16:47 ` Jason Baron 2015-12-04 17:03 ` Joe Perches 2015-12-04 17:03 ` Joe Perches 2015-12-04 17:11 ` Jason Baron 2015-12-04 17:11 ` Jason Baron 2015-12-04 10:41 ` Dmitry Vyukov 2015-12-04 10:41 ` Dmitry Vyukov 2015-12-04 17:48 ` Marcelo Ricardo Leitner 2015-12-04 17:48 ` Marcelo Ricardo Leitner 2015-12-04 20:25 ` Dmitry Vyukov 2015-12-04 20:25 ` Dmitry Vyukov 2015-12-04 21:34 ` Marcelo Ricardo Leitner 2015-12-04 21:34 ` Marcelo Ricardo Leitner 2015-12-04 21:38 ` Dmitry Vyukov 2015-12-04 21:38 ` Dmitry Vyukov 2015-12-05 16:39 ` Vlad Yasevich 2015-12-05 16:39 ` Vlad Yasevich 2015-12-07 11:26 ` Dmitry Vyukov 2015-12-07 11:26 ` Dmitry Vyukov 2015-12-07 13:15 ` Marcelo Ricardo Leitner 2015-12-07 13:15 ` Marcelo Ricardo Leitner 2015-12-07 13:20 ` Dmitry Vyukov 2015-12-07 13:20 ` Dmitry Vyukov 2015-12-07 18:52 ` Marcelo Ricardo Leitner 2015-12-07 18:52 ` Marcelo Ricardo Leitner 2015-12-07 19:33 ` Vlad Yasevich 2015-12-07 19:33 ` Vlad Yasevich 2015-12-07 19:50 ` Marcelo Ricardo Leitner 2015-12-07 19:50 ` Marcelo Ricardo Leitner 2015-12-07 20:37 ` Vlad Yasevich 2015-12-07 20:37 ` Vlad Yasevich 2015-12-07 20:52 ` Marcelo Ricardo Leitner 2015-12-07 20:52 ` Marcelo Ricardo Leitner 2015-12-08 17:30 ` Dmitry Vyukov 2015-12-08 17:30 ` Dmitry Vyukov 2015-12-08 17:40 ` Marcelo Ricardo Leitner 2015-12-08 17:40 ` Marcelo Ricardo Leitner 2015-12-08 19:22 ` Dmitry Vyukov 2015-12-08 19:22 ` Dmitry Vyukov 2015-12-09 14:41 ` Dmitry Vyukov 2015-12-09 14:41 ` Dmitry Vyukov 2015-12-09 15:03 ` Marcelo Ricardo Leitner 2015-12-09 15:03 ` Marcelo Ricardo Leitner 2015-12-09 16:41 ` Marcelo Ricardo Leitner 2015-12-09 16:41 ` Marcelo Ricardo Leitner 2015-12-11 13:35 ` Dmitry Vyukov 2015-12-11 13:35 ` Dmitry Vyukov 2015-12-11 13:51 ` Marcelo Ricardo Leitner 2015-12-11 13:51 ` Marcelo Ricardo Leitner 2015-12-11 14:03 ` Marcelo Ricardo Leitner 2015-12-11 14:03 ` Marcelo Ricardo Leitner 2015-12-11 14:30 ` Dmitry Vyukov 2015-12-11 14:30 ` Dmitry Vyukov 2015-12-11 15:55 ` Marcelo Ricardo Leitner 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-08 13:00 ` Marcelo Ricardo Leitner 2016-01-11 17:00 ` Vlad Yasevich 2016-01-11 17:00 ` Vlad Yasevich 2016-01-11 22:13 ` David Miller 2016-01-11 22:13 ` David Miller 2016-01-12 8:41 ` Dmitry Vyukov 2016-01-12 8:41 ` Dmitry Vyukov 2015-12-11 18:37 ` use-after-free in sctp_do_sm Vlad Yasevich 2015-12-11 18:37 ` Vlad Yasevich 2015-12-14 9:50 ` David Laight 2015-12-14 14:25 ` Vlad Yasevich 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=f7ttwnyus8l.fsf@aconole.bos.csb \ --to=aconole@redhat.com \ --cc=akpm@linux-foundation.org \ --cc=dvyukov@google.com \ --cc=edumazet@google.com \ --cc=glider@google.com \ --cc=jbaron@akamai.com \ --cc=joe@perches.com \ --cc=kcc@google.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-sctp@vger.kernel.org \ --cc=marcelo.leitner@gmail.com \ --cc=netdev@vger.kernel.org \ --cc=sasha.levin@oracle.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: linkBe 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.