| Message ID | fcceeb2a58ce2054cedb091f0e251393b36418ba.1449250981.git.marcelo.leitner@gmail.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Fri, Dec 4, 2015 at 6:48 PM, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote: > Hi Dmitry, > > Can you please test this patch? > I'll re-post with proper subject if it works. Still happening with the same stacks. > ---8<--- > > Dmitry Vyukov reported a use-after-free in the code expanded by the > macro debug_post_sfx, which is caused by the use of the asoc pointer > after it was freed within sctp_side_effect() scope. > > This patch fixes it by allowing sctp_side_effect to clear that asoc > pointer when the TCB is freed. > > The macro is already prepared to handle such NULL pointer. > > Reported-by: Dmitry Vyukov <dvyukov@google.com> > Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > --- > net/sctp/sm_sideeffect.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c > index 6098d4c42fa91287d3cde36ac05d860f76d4fe32..05594dcd93e0d649cace5215d225bef2713f9310 100644 > --- a/net/sctp/sm_sideeffect.c > +++ b/net/sctp/sm_sideeffect.c > @@ -63,7 +63,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type, > static int sctp_side_effects(sctp_event_t event_type, sctp_subtype_t subtype, > sctp_state_t state, > struct sctp_endpoint *ep, > - struct sctp_association *asoc, > + struct sctp_association **asoc, > void *event_arg, > sctp_disposition_t status, > sctp_cmd_seq_t *commands, > @@ -1123,7 +1123,7 @@ int sctp_do_sm(struct net *net, sctp_event_t event_type, sctp_subtype_t subtype, > debug_post_sfn(); > > error = sctp_side_effects(event_type, subtype, state, > - ep, asoc, event_arg, status, > + ep, &asoc, event_arg, status, > &commands, gfp); > debug_post_sfx(); > > @@ -1136,7 +1136,7 @@ int sctp_do_sm(struct net *net, sctp_event_t event_type, sctp_subtype_t subtype, > static int sctp_side_effects(sctp_event_t event_type, sctp_subtype_t subtype, > sctp_state_t state, > struct sctp_endpoint *ep, > - struct sctp_association *asoc, > + struct sctp_association **asoc, > void *event_arg, > sctp_disposition_t status, > sctp_cmd_seq_t *commands, > @@ -1151,7 +1151,7 @@ static int sctp_side_effects(sctp_event_t event_type, sctp_subtype_t subtype, > * disposition SCTP_DISPOSITION_CONSUME. > */ > if (0 != (error = sctp_cmd_interpreter(event_type, subtype, state, > - ep, asoc, > + ep, *asoc, > event_arg, status, > commands, gfp))) > goto bail; > @@ -1175,6 +1175,7 @@ static int sctp_side_effects(sctp_event_t event_type, sctp_subtype_t subtype, > > case SCTP_DISPOSITION_DELETE_TCB: > /* This should now be a command. */ > + *asoc = NULL; > break; > > case SCTP_DISPOSITION_CONSUME: > -- > 2.5.0 > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 04, 2015 at 09:25:35PM +0100, Dmitry Vyukov wrote: > On Fri, Dec 4, 2015 at 6:48 PM, Marcelo Ricardo Leitner > <marcelo.leitner@gmail.com> wrote: > > Hi Dmitry, > > > > Can you please test this patch? > > I'll re-post with proper subject if it works. > > Still happening with the same stacks. Then there may be another one, I'm afraid. I'm using the testapp you shared in the first email, with that debug line enabled and added a new one: + pr_debug("%p %d\n", asoc, asoc ? asoc->state : 0); debug_post_sfx(); (should have used %x, but ok) Also enabled slub_debug=PUZ, and I get: without the patch: [ 87.873640] sctp: ffff8800b71533d8 1 [ 87.873647] sctp: sctp_do_sm[post-sfx]: error:0, asoc:ffff8800b71533d8[STATE_CLOSED] [ 87.873739] sctp: ffff8800b71533d8 1 [ 87.873742] sctp: sctp_do_sm[post-sfx]: error:0, asoc:ffff8800b71533d8[STATE_CLOSED] [ 87.875149] sctp: ffff8800b71533d8 1802201963 [ 87.875238] sctp: sctp_do_sm[post-sfx]: error:0, asoc:ffff8800b71533d8[STATE_CLOSED] 1802201963 = 0x6b6b6b6b, poison with the patch: [ 81.071265] sctp: ffff880137571148 1 [ 81.071273] sctp: sctp_do_sm[post-sfx]: error:0, asoc:ffff880137571148[STATE_CLOSED] [ 81.071372] sctp: ffff880137571148 1 [ 81.071375] sctp: sctp_do_sm[post-sfx]: error:0, asoc:ffff880137571148[STATE_CLOSED] [ 81.072423] sctp: (null) 0 [ 81.072427] sctp: sctp_do_sm[post-sfx]: error:0, asoc: (null)[STATE_CLOSED] This one, at least, is gone with this patch. Marcelo -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 4, 2015 at 10:34 PM, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote: > On Fri, Dec 04, 2015 at 09:25:35PM +0100, Dmitry Vyukov wrote: >> On Fri, Dec 4, 2015 at 6:48 PM, Marcelo Ricardo Leitner >> <marcelo.leitner@gmail.com> wrote: >> > Hi Dmitry, >> > >> > Can you please test this patch? >> > I'll re-post with proper subject if it works. >> >> Still happening with the same stacks. > > Then there may be another one, I'm afraid. > > I'm using the testapp you shared in the first email, with that debug line > enabled and added a new one: > + pr_debug("%p %d\n", asoc, asoc ? asoc->state : 0); > debug_post_sfx(); > (should have used %x, but ok) > > Also enabled slub_debug=PUZ, and I get: > > without the patch: > [ 87.873640] sctp: ffff8800b71533d8 1 > [ 87.873647] sctp: sctp_do_sm[post-sfx]: error:0, > asoc:ffff8800b71533d8[STATE_CLOSED] > [ 87.873739] sctp: ffff8800b71533d8 1 > [ 87.873742] sctp: sctp_do_sm[post-sfx]: error:0, > asoc:ffff8800b71533d8[STATE_CLOSED] > [ 87.875149] sctp: ffff8800b71533d8 1802201963 > [ 87.875238] sctp: sctp_do_sm[post-sfx]: error:0, > asoc:ffff8800b71533d8[STATE_CLOSED] > > 1802201963 = 0x6b6b6b6b, poison > > with the patch: > [ 81.071265] sctp: ffff880137571148 1 > [ 81.071273] sctp: sctp_do_sm[post-sfx]: error:0, > asoc:ffff880137571148[STATE_CLOSED] > [ 81.071372] sctp: ffff880137571148 1 > [ 81.071375] sctp: sctp_do_sm[post-sfx]: error:0, > asoc:ffff880137571148[STATE_CLOSED] > [ 81.072423] sctp: (null) 0 > [ 81.072427] sctp: sctp_do_sm[post-sfx]: error:0, asoc: > (null)[STATE_CLOSED] > > This one, at least, is gone with this patch. I will try to extract reproducer next week. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/04/2015 04:34 PM, Marcelo Ricardo Leitner wrote: > On Fri, Dec 04, 2015 at 09:25:35PM +0100, Dmitry Vyukov wrote: >> On Fri, Dec 4, 2015 at 6:48 PM, Marcelo Ricardo Leitner >> <marcelo.leitner@gmail.com> wrote: >>> Hi Dmitry, >>> >>> Can you please test this patch? >>> I'll re-post with proper subject if it works. >> >> Still happening with the same stacks. > > Then there may be another one, I'm afraid. > > I'm using the testapp you shared in the first email, with that debug line > enabled and added a new one: > + pr_debug("%p %d\n", asoc, asoc ? asoc->state : 0); > debug_post_sfx(); > (should have used %x, but ok) > > Also enabled slub_debug=PUZ, and I get: > > without the patch: > [ 87.873640] sctp: ffff8800b71533d8 1 > [ 87.873647] sctp: sctp_do_sm[post-sfx]: error:0, > asoc:ffff8800b71533d8[STATE_CLOSED] > [ 87.873739] sctp: ffff8800b71533d8 1 > [ 87.873742] sctp: sctp_do_sm[post-sfx]: error:0, > asoc:ffff8800b71533d8[STATE_CLOSED] > [ 87.875149] sctp: ffff8800b71533d8 1802201963 > [ 87.875238] sctp: sctp_do_sm[post-sfx]: error:0, > asoc:ffff8800b71533d8[STATE_CLOSED] > > 1802201963 = 0x6b6b6b6b, poison > > with the patch: > [ 81.071265] sctp: ffff880137571148 1 > [ 81.071273] sctp: sctp_do_sm[post-sfx]: error:0, > asoc:ffff880137571148[STATE_CLOSED] > [ 81.071372] sctp: ffff880137571148 1 > [ 81.071375] sctp: sctp_do_sm[post-sfx]: error:0, > asoc:ffff880137571148[STATE_CLOSED] > [ 81.072423] sctp: (null) 0 > [ 81.072427] sctp: sctp_do_sm[post-sfx]: error:0, asoc: > (null)[STATE_CLOSED] > > This one, at least, is gone with this patch. > > Marcelo > Hi Marcelo I think you also need to catch the SCTP_DISPOSITION_ABORT and update the pointer. There are some issues there though as some functions report that code without actually destroying the association. This happens when the ABORT chunk may be dropped. I think this might be why we still see the issue. -vlad -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Dec 5, 2015 at 5:39 PM, Vlad Yasevich <vyasevich@gmail.com> wrote: > On 12/04/2015 04:34 PM, Marcelo Ricardo Leitner wrote: >> On Fri, Dec 04, 2015 at 09:25:35PM +0100, Dmitry Vyukov wrote: >>> On Fri, Dec 4, 2015 at 6:48 PM, Marcelo Ricardo Leitner >>> <marcelo.leitner@gmail.com> wrote: >>>> Hi Dmitry, >>>> >>>> Can you please test this patch? >>>> I'll re-post with proper subject if it works. >>> >>> Still happening with the same stacks. >> >> Then there may be another one, I'm afraid. >> >> I'm using the testapp you shared in the first email, with that debug line >> enabled and added a new one: >> + pr_debug("%p %d\n", asoc, asoc ? asoc->state : 0); >> debug_post_sfx(); >> (should have used %x, but ok) >> >> Also enabled slub_debug=PUZ, and I get: >> >> without the patch: >> [ 87.873640] sctp: ffff8800b71533d8 1 >> [ 87.873647] sctp: sctp_do_sm[post-sfx]: error:0, >> asoc:ffff8800b71533d8[STATE_CLOSED] >> [ 87.873739] sctp: ffff8800b71533d8 1 >> [ 87.873742] sctp: sctp_do_sm[post-sfx]: error:0, >> asoc:ffff8800b71533d8[STATE_CLOSED] >> [ 87.875149] sctp: ffff8800b71533d8 1802201963 >> [ 87.875238] sctp: sctp_do_sm[post-sfx]: error:0, >> asoc:ffff8800b71533d8[STATE_CLOSED] >> >> 1802201963 = 0x6b6b6b6b, poison >> >> with the patch: >> [ 81.071265] sctp: ffff880137571148 1 >> [ 81.071273] sctp: sctp_do_sm[post-sfx]: error:0, >> asoc:ffff880137571148[STATE_CLOSED] >> [ 81.071372] sctp: ffff880137571148 1 >> [ 81.071375] sctp: sctp_do_sm[post-sfx]: error:0, >> asoc:ffff880137571148[STATE_CLOSED] >> [ 81.072423] sctp: (null) 0 >> [ 81.072427] sctp: sctp_do_sm[post-sfx]: error:0, asoc: >> (null)[STATE_CLOSED] >> >> This one, at least, is gone with this patch. >> >> Marcelo >> > > Hi Marcelo > > I think you also need to catch the SCTP_DISPOSITION_ABORT and update > the pointer. There are some issues there though as some functions report > that code without actually destroying the association. This happens when > the ABORT chunk may be dropped. > > I think this might be why we still see the issue. Marcelo, Is this info enough for you to cook another fix? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 07, 2015 at 12:26:09PM +0100, Dmitry Vyukov wrote: > On Sat, Dec 5, 2015 at 5:39 PM, Vlad Yasevich <vyasevich@gmail.com> wrote: > > On 12/04/2015 04:34 PM, Marcelo Ricardo Leitner wrote: > >> On Fri, Dec 04, 2015 at 09:25:35PM +0100, Dmitry Vyukov wrote: > >>> On Fri, Dec 4, 2015 at 6:48 PM, Marcelo Ricardo Leitner > >>> <marcelo.leitner@gmail.com> wrote: > >>>> Hi Dmitry, > >>>> > >>>> Can you please test this patch? > >>>> I'll re-post with proper subject if it works. > >>> > >>> Still happening with the same stacks. > >> > >> Then there may be another one, I'm afraid. > >> > >> I'm using the testapp you shared in the first email, with that debug line > >> enabled and added a new one: > >> + pr_debug("%p %d\n", asoc, asoc ? asoc->state : 0); > >> debug_post_sfx(); > >> (should have used %x, but ok) > >> > >> Also enabled slub_debug=PUZ, and I get: > >> > >> without the patch: > >> [ 87.873640] sctp: ffff8800b71533d8 1 > >> [ 87.873647] sctp: sctp_do_sm[post-sfx]: error:0, > >> asoc:ffff8800b71533d8[STATE_CLOSED] > >> [ 87.873739] sctp: ffff8800b71533d8 1 > >> [ 87.873742] sctp: sctp_do_sm[post-sfx]: error:0, > >> asoc:ffff8800b71533d8[STATE_CLOSED] > >> [ 87.875149] sctp: ffff8800b71533d8 1802201963 > >> [ 87.875238] sctp: sctp_do_sm[post-sfx]: error:0, > >> asoc:ffff8800b71533d8[STATE_CLOSED] > >> > >> 1802201963 = 0x6b6b6b6b, poison > >> > >> with the patch: > >> [ 81.071265] sctp: ffff880137571148 1 > >> [ 81.071273] sctp: sctp_do_sm[post-sfx]: error:0, > >> asoc:ffff880137571148[STATE_CLOSED] > >> [ 81.071372] sctp: ffff880137571148 1 > >> [ 81.071375] sctp: sctp_do_sm[post-sfx]: error:0, > >> asoc:ffff880137571148[STATE_CLOSED] > >> [ 81.072423] sctp: (null) 0 > >> [ 81.072427] sctp: sctp_do_sm[post-sfx]: error:0, asoc: > >> (null)[STATE_CLOSED] > >> > >> This one, at least, is gone with this patch. > >> > >> Marcelo > >> > > > > Hi Marcelo > > > > I think you also need to catch the SCTP_DISPOSITION_ABORT and update > > the pointer. There are some issues there though as some functions report > > that code without actually destroying the association. This happens when > > the ABORT chunk may be dropped. > > > > I think this might be why we still see the issue. > > > Marcelo, > > Is this info enough for you to cook another fix? Hi, I think so. I was really wondering how you could trigger that issue without the timestamp fix and Vlad's comment does shed some light on it. I'll do more tests later today, but what did you have connecting to the listening socket? Somehow you made that accept() call to return.. Marcelo -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 7, 2015 at 2:15 PM, Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> wrote: > On Mon, Dec 07, 2015 at 12:26:09PM +0100, Dmitry Vyukov wrote: >> On Sat, Dec 5, 2015 at 5:39 PM, Vlad Yasevich <vyasevich@gmail.com> wrote: >> > On 12/04/2015 04:34 PM, Marcelo Ricardo Leitner wrote: >> >> On Fri, Dec 04, 2015 at 09:25:35PM +0100, Dmitry Vyukov wrote: >> >>> On Fri, Dec 4, 2015 at 6:48 PM, Marcelo Ricardo Leitner >> >>> <marcelo.leitner@gmail.com> wrote: >> >>>> Hi Dmitry, >> >>>> >> >>>> Can you please test this patch? >> >>>> I'll re-post with proper subject if it works. >> >>> >> >>> Still happening with the same stacks. >> >> >> >> Then there may be another one, I'm afraid. >> >> >> >> I'm using the testapp you shared in the first email, with that debug line >> >> enabled and added a new one: >> >> + pr_debug("%p %d\n", asoc, asoc ? asoc->state : 0); >> >> debug_post_sfx(); >> >> (should have used %x, but ok) >> >> >> >> Also enabled slub_debug=PUZ, and I get: >> >> >> >> without the patch: >> >> [ 87.873640] sctp: ffff8800b71533d8 1 >> >> [ 87.873647] sctp: sctp_do_sm[post-sfx]: error:0, >> >> asoc:ffff8800b71533d8[STATE_CLOSED] >> >> [ 87.873739] sctp: ffff8800b71533d8 1 >> >> [ 87.873742] sctp: sctp_do_sm[post-sfx]: error:0, >> >> asoc:ffff8800b71533d8[STATE_CLOSED] >> >> [ 87.875149] sctp: ffff8800b71533d8 1802201963 >> >> [ 87.875238] sctp: sctp_do_sm[post-sfx]: error:0, >> >> asoc:ffff8800b71533d8[STATE_CLOSED] >> >> >> >> 1802201963 = 0x6b6b6b6b, poison >> >> >> >> with the patch: >> >> [ 81.071265] sctp: ffff880137571148 1 >> >> [ 81.071273] sctp: sctp_do_sm[post-sfx]: error:0, >> >> asoc:ffff880137571148[STATE_CLOSED] >> >> [ 81.071372] sctp: ffff880137571148 1 >> >> [ 81.071375] sctp: sctp_do_sm[post-sfx]: error:0, >> >> asoc:ffff880137571148[STATE_CLOSED] >> >> [ 81.072423] sctp: (null) 0 >> >> [ 81.072427] sctp: sctp_do_sm[post-sfx]: error:0, asoc: >> >> (null)[STATE_CLOSED] >> >> >> >> This one, at least, is gone with this patch. >> >> >> >> Marcelo >> >> >> > >> > Hi Marcelo >> > >> > I think you also need to catch the SCTP_DISPOSITION_ABORT and update >> > the pointer. There are some issues there though as some functions report >> > that code without actually destroying the association. This happens when >> > the ABORT chunk may be dropped. >> > >> > I think this might be why we still see the issue. >> >> >> Marcelo, >> >> Is this info enough for you to cook another fix? > > Hi, I think so. I was really wondering how you could trigger that issue > without the timestamp fix and Vlad's comment does shed some light on it. > > I'll do more tests later today, but what did you have connecting to the > listening socket? Somehow you made that accept() call to return.. Local connect in another thread I guess. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Dec 07, 2015 at 02:20:47PM +0100, Dmitry Vyukov wrote: > On Mon, Dec 7, 2015 at 2:15 PM, Marcelo Ricardo Leitner > <marcelo.leitner@gmail.com> wrote: > > On Mon, Dec 07, 2015 at 12:26:09PM +0100, Dmitry Vyukov wrote: > >> On Sat, Dec 5, 2015 at 5:39 PM, Vlad Yasevich <vyasevich@gmail.com> wrote: ... > >> > Hi Marcelo > >> > > >> > I think you also need to catch the SCTP_DISPOSITION_ABORT and update > >> > the pointer. There are some issues there though as some functions report > >> > that code without actually destroying the association. This happens when > >> > the ABORT chunk may be dropped. > >> > > >> > I think this might be why we still see the issue. > >> > >> > >> Marcelo, > >> > >> Is this info enough for you to cook another fix? > > > > Hi, I think so. I was really wondering how you could trigger that issue > > without the timestamp fix and Vlad's comment does shed some light on it. > > > > I'll do more tests later today, but what did you have connecting to the > > listening socket? Somehow you made that accept() call to return.. > > Local connect in another thread I guess. 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. 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. Dmitry, still no luck here, cannot reproduce another hit. I'm using sctp_test and a custom test of mine, both on localhost so I would catch it in server or client side, nothing.. I need more info. Please enable the pr_debug() on debug_post_sfn() macro and see which status is being reported when you trigger the issue. And/or share a traffic capture so we can see what's going on with the association. Marcelo -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/07/2015 01:52 PM, Marcelo Ricardo Leitner wrote: > On Mon, Dec 07, 2015 at 02:20:47PM +0100, Dmitry Vyukov wrote: >> On Mon, Dec 7, 2015 at 2:15 PM, Marcelo Ricardo Leitner >> <marcelo.leitner@gmail.com> wrote: >>> On Mon, Dec 07, 2015 at 12:26:09PM +0100, Dmitry Vyukov wrote: >>>> On Sat, Dec 5, 2015 at 5:39 PM, Vlad Yasevich <vyasevich@gmail.com> wrote: > ... >>>>> Hi Marcelo >>>>> >>>>> I think you also need to catch the SCTP_DISPOSITION_ABORT and update >>>>> the pointer. There are some issues there though as some functions report >>>>> that code without actually destroying the association. This happens when >>>>> the ABORT chunk may be dropped. >>>>> >>>>> I think this might be why we still see the issue. >>>> >>>> >>>> Marcelo, >>>> >>>> Is this info enough for you to cook another fix? >>> >>> Hi, I think so. I was really wondering how you could trigger that issue >>> without the timestamp fix and Vlad's comment does shed some light on it. >>> >>> I'll do more tests later today, but what did you have connecting to the >>> listening socket? Somehow you made that accept() call to return.. >> >> Local connect in another thread I guess. > > 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. > 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. -vlad > > Dmitry, still no luck here, cannot reproduce another hit. > I'm using sctp_test and a custom test of mine, both on localhost so I > would catch it in server or client side, nothing.. > > I need more info. Please enable the pr_debug() on debug_post_sfn() macro > and see which status is being reported when you trigger the issue. > And/or share a traffic capture so we can see what's going on with the > association. > > Marcelo > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c index 6098d4c42fa91287d3cde36ac05d860f76d4fe32..05594dcd93e0d649cace5215d225bef2713f9310 100644 --- a/net/sctp/sm_sideeffect.c +++ b/net/sctp/sm_sideeffect.c @@ -63,7 +63,7 @@ static int sctp_cmd_interpreter(sctp_event_t event_type, static int sctp_side_effects(sctp_event_t event_type, sctp_subtype_t subtype, sctp_state_t state, struct sctp_endpoint *ep, - struct sctp_association *asoc, + struct sctp_association **asoc, void *event_arg, sctp_disposition_t status, sctp_cmd_seq_t *commands, @@ -1123,7 +1123,7 @@ int sctp_do_sm(struct net *net, sctp_event_t event_type, sctp_subtype_t subtype, debug_post_sfn(); error = sctp_side_effects(event_type, subtype, state, - ep, asoc, event_arg, status, + ep, &asoc, event_arg, status, &commands, gfp); debug_post_sfx(); @@ -1136,7 +1136,7 @@ int sctp_do_sm(struct net *net, sctp_event_t event_type, sctp_subtype_t subtype, static int sctp_side_effects(sctp_event_t event_type, sctp_subtype_t subtype, sctp_state_t state, struct sctp_endpoint *ep, - struct sctp_association *asoc, + struct sctp_association **asoc, void *event_arg, sctp_disposition_t status, sctp_cmd_seq_t *commands, @@ -1151,7 +1151,7 @@ static int sctp_side_effects(sctp_event_t event_type, sctp_subtype_t subtype, * disposition SCTP_DISPOSITION_CONSUME. */ if (0 != (error = sctp_cmd_interpreter(event_type, subtype, state, - ep, asoc, + ep, *asoc, event_arg, status, commands, gfp))) goto bail; @@ -1175,6 +1175,7 @@ static int sctp_side_effects(sctp_event_t event_type, sctp_subtype_t subtype, case SCTP_DISPOSITION_DELETE_TCB: /* This should now be a command. */ + *asoc = NULL; break; case SCTP_DISPOSITION_CONSUME:
Hi Dmitry, Can you please test this patch? I'll re-post with proper subject if it works. Thanks. ---8<--- Dmitry Vyukov reported a use-after-free in the code expanded by the macro debug_post_sfx, which is caused by the use of the asoc pointer after it was freed within sctp_side_effect() scope. This patch fixes it by allowing sctp_side_effect to clear that asoc pointer when the TCB is freed. The macro is already prepared to handle such NULL pointer. Reported-by: Dmitry Vyukov <dvyukov@google.com> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> --- net/sctp/sm_sideeffect.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)