From: Marcelo Ricardo Leitner <marcelo.leitner@gmail•com>
To: Vlad Yasevich <vyasevich@gmail•com>
Cc: Dmitry Vyukov <dvyukov@google•com>,
netdev <netdev@vger•kernel.org>,
Eric Dumazet <eric.dumazet@gmail•com>,
syzkaller <syzkaller@googlegroups•com>,
linux-sctp@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: Mon, 7 Dec 2015 18:52:11 -0200 [thread overview]
Message-ID: <5665F17B.5030908@gmail.com> (raw)
In-Reply-To: <5665EE26.3000706@gmail.com>
Em 07-12-2015 18:37, Vlad Yasevich escreveu:
> On 12/07/2015 02:50 PM, Marcelo Ricardo Leitner wrote:
>> On Mon, Dec 07, 2015 at 02:33:52PM -0500, Vlad Yasevich wrote:
>>> On 12/07/2015 01:52 PM, Marcelo Ricardo Leitner wrote:
>>>> Vlad, I reviewed the places on which it returns SCTP_DISPOSITION_ABORT,
>>>> and if I didn't miss something in there all of them either issue
>>>> SCTP_CMD_ASSOC_FAILED or SCTP_CMD_INIT_FAILED before returning it, thus
>>>> delaying DELETE_TCB and with that the asoc free.
>>>
>>> They delay it from the perspective of the command interpreter since the command
>>> to delete the TCB happens a little later, but status code is checked after all
>>> commands are processed and command processing doesn't change it. So the 'status'
>>> code would still be SCTP_DISPOSITION_ABORT after DELETE_TCB command was processed.
>>> So, I think we may still have an use-after-free issue here.
>>
>> Gotcha! That's pretty much it then. From that point of view now, there
>> shouldn't be a case that it returns _ABORT without freeing the asoc in
>> the same loop. (more below)
>>
>>>> There is one place,
>>>> though, that may not do it that way, it's sctp_sf_abort_violation(), but
>>>> then that code only runs if asoc is already NULL by then.
>>>
>>> I don't believe so. The violation state function can run with a non-NULL association
>>> if we are encountering protocol violations after the association is established.
>>
>> Yup, that's correct. I just tried to reference one case on which it
>> would return _ABORT without issuing any of those _FAILEDs before doing
>> so (meaning the association could still be valid) but that in that case,
>> the asoc was already NULL.
>
> I think it is possible to hit the 'discard:' tag in that function while still
> having a valid association. That happens when ABORT chunk is required to be
> authenticated. This that case, instead of generating an ABORT and terminating the
> current association, we just drop the packet, but still report an _ABORT disposition code.
>
> This probably need to change if we are going to catch the _ABORT disposition and
> clear the asoc pointer.
Oups. Nice one. I'll switch it to SCTP_DISPOSITION_DISCARD if it hits
that if() then. Thanks Vlad.
Marcelo
next prev parent reply other threads:[~2015-12-07 20:52 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
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 [this message]
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=5665F17B.5030908@gmail.com \
--to=marcelo.leitner@gmail$(echo .)com \
--cc=dvyukov@google$(echo .)com \
--cc=eric.dumazet@gmail$(echo .)com \
--cc=glider@google$(echo .)com \
--cc=kcc@google$(echo .)com \
--cc=linux-sctp@vger$(echo .)kernel.org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=sasha.levin@oracle$(echo .)com \
--cc=syzkaller@googlegroups$(echo .)com \
--cc=vyasevich@gmail$(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