* [PATCH net v3] net: sctp: sctp_transport_destroy{,_rcu}: fix potential pointer corruption
@ 2013-08-09 14:25 ` Daniel Borkmann
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Borkmann @ 2013-08-09 14:25 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-sctp
Probably this one is quite unlikely to be triggered, but it's more safe
to do the call_rcu() at the end after we have dropped the reference on
the asoc and freed sctp packet chunks. The reason why is because in
sctp_transport_destroy_rcu() the transport is being kfree()'d, and if
we're unlucky enough we could run into corrupted pointers. Probably
that's more of theoretical nature, but it's safer to have this simple fix.
Introduced by commit 8c98653f ("sctp: sctp_close: fix release of bindings
for deferred call_rcu's"). I also did the 8c98653f regression test and
it's fine that way.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
net/sctp/transport.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index bdbbc3f..8fdd160 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -181,12 +181,12 @@ static void sctp_transport_destroy(struct sctp_transport *transport)
return;
}
- call_rcu(&transport->rcu, sctp_transport_destroy_rcu);
-
sctp_packet_free(&transport->packet);
if (transport->asoc)
sctp_association_put(transport->asoc);
+
+ call_rcu(&transport->rcu, sctp_transport_destroy_rcu);
}
/* Start T3_rtx timer if it is not already running and update the heartbeat
--
1.7.11.7
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net v3] net: sctp: sctp_transport_destroy{,_rcu}: fix potential pointer corruption
@ 2013-08-09 14:25 ` Daniel Borkmann
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Borkmann @ 2013-08-09 14:25 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-sctp
Probably this one is quite unlikely to be triggered, but it's more safe
to do the call_rcu() at the end after we have dropped the reference on
the asoc and freed sctp packet chunks. The reason why is because in
sctp_transport_destroy_rcu() the transport is being kfree()'d, and if
we're unlucky enough we could run into corrupted pointers. Probably
that's more of theoretical nature, but it's safer to have this simple fix.
Introduced by commit 8c98653f ("sctp: sctp_close: fix release of bindings
for deferred call_rcu's"). I also did the 8c98653f regression test and
it's fine that way.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
net/sctp/transport.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index bdbbc3f..8fdd160 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -181,12 +181,12 @@ static void sctp_transport_destroy(struct sctp_transport *transport)
return;
}
- call_rcu(&transport->rcu, sctp_transport_destroy_rcu);
-
sctp_packet_free(&transport->packet);
if (transport->asoc)
sctp_association_put(transport->asoc);
+
+ call_rcu(&transport->rcu, sctp_transport_destroy_rcu);
}
/* Start T3_rtx timer if it is not already running and update the heartbeat
--
1.7.11.7
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net v3] net: sctp: sctp_transport_destroy{,_rcu}: fix potential pointer corruption
2013-08-09 14:25 ` Daniel Borkmann
@ 2013-08-09 14:42 ` Vlad Yasevich
-1 siblings, 0 replies; 6+ messages in thread
From: Vlad Yasevich @ 2013-08-09 14:42 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev, linux-sctp
On 08/09/2013 10:25 AM, Daniel Borkmann wrote:
> Probably this one is quite unlikely to be triggered, but it's more safe
> to do the call_rcu() at the end after we have dropped the reference on
> the asoc and freed sctp packet chunks. The reason why is because in
> sctp_transport_destroy_rcu() the transport is being kfree()'d, and if
> we're unlucky enough we could run into corrupted pointers. Probably
> that's more of theoretical nature, but it's safer to have this simple fix.
>
> Introduced by commit 8c98653f ("sctp: sctp_close: fix release of bindings
> for deferred call_rcu's"). I also did the 8c98653f regression test and
> it's fine that way.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
> ---
> net/sctp/transport.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/transport.c b/net/sctp/transport.c
> index bdbbc3f..8fdd160 100644
> --- a/net/sctp/transport.c
> +++ b/net/sctp/transport.c
> @@ -181,12 +181,12 @@ static void sctp_transport_destroy(struct sctp_transport *transport)
> return;
> }
>
> - call_rcu(&transport->rcu, sctp_transport_destroy_rcu);
> -
> sctp_packet_free(&transport->packet);
>
> if (transport->asoc)
> sctp_association_put(transport->asoc);
> +
> + call_rcu(&transport->rcu, sctp_transport_destroy_rcu);
> }
>
> /* Start T3_rtx timer if it is not already running and update the heartbeat
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net v3] net: sctp: sctp_transport_destroy{,_rcu}: fix potential pointer corruption
@ 2013-08-09 14:42 ` Vlad Yasevich
0 siblings, 0 replies; 6+ messages in thread
From: Vlad Yasevich @ 2013-08-09 14:42 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev, linux-sctp
On 08/09/2013 10:25 AM, Daniel Borkmann wrote:
> Probably this one is quite unlikely to be triggered, but it's more safe
> to do the call_rcu() at the end after we have dropped the reference on
> the asoc and freed sctp packet chunks. The reason why is because in
> sctp_transport_destroy_rcu() the transport is being kfree()'d, and if
> we're unlucky enough we could run into corrupted pointers. Probably
> that's more of theoretical nature, but it's safer to have this simple fix.
>
> Introduced by commit 8c98653f ("sctp: sctp_close: fix release of bindings
> for deferred call_rcu's"). I also did the 8c98653f regression test and
> it's fine that way.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
> ---
> net/sctp/transport.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/transport.c b/net/sctp/transport.c
> index bdbbc3f..8fdd160 100644
> --- a/net/sctp/transport.c
> +++ b/net/sctp/transport.c
> @@ -181,12 +181,12 @@ static void sctp_transport_destroy(struct sctp_transport *transport)
> return;
> }
>
> - call_rcu(&transport->rcu, sctp_transport_destroy_rcu);
> -
> sctp_packet_free(&transport->packet);
>
> if (transport->asoc)
> sctp_association_put(transport->asoc);
> +
> + call_rcu(&transport->rcu, sctp_transport_destroy_rcu);
> }
>
> /* Start T3_rtx timer if it is not already running and update the heartbeat
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net v3] net: sctp: sctp_transport_destroy{,_rcu}: fix potential pointer corruption
2013-08-09 14:42 ` Vlad Yasevich
@ 2013-08-13 5:14 ` David Miller
-1 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-08-13 5:14 UTC (permalink / raw)
To: vyasevich; +Cc: dborkman, netdev, linux-sctp
From: Vlad Yasevich <vyasevich@gmail.com>
Date: Fri, 09 Aug 2013 10:42:37 -0400
> On 08/09/2013 10:25 AM, Daniel Borkmann wrote:
>> Probably this one is quite unlikely to be triggered, but it's more
>> safe
>> to do the call_rcu() at the end after we have dropped the reference on
>> the asoc and freed sctp packet chunks. The reason why is because in
>> sctp_transport_destroy_rcu() the transport is being kfree()'d, and if
>> we're unlucky enough we could run into corrupted pointers. Probably
>> that's more of theoretical nature, but it's safer to have this simple
>> fix.
>>
>> Introduced by commit 8c98653f ("sctp: sctp_close: fix release of
>> bindings
>> for deferred call_rcu's"). I also did the 8c98653f regression test and
>> it's fine that way.
>>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>
> Acked-by: Vlad Yasevich <vyasevich@gmail.com>
Applied, thanks everyone.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net v3] net: sctp: sctp_transport_destroy{,_rcu}: fix potential pointer corruption
@ 2013-08-13 5:14 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-08-13 5:14 UTC (permalink / raw)
To: vyasevich; +Cc: dborkman, netdev, linux-sctp
From: Vlad Yasevich <vyasevich@gmail.com>
Date: Fri, 09 Aug 2013 10:42:37 -0400
> On 08/09/2013 10:25 AM, Daniel Borkmann wrote:
>> Probably this one is quite unlikely to be triggered, but it's more
>> safe
>> to do the call_rcu() at the end after we have dropped the reference on
>> the asoc and freed sctp packet chunks. The reason why is because in
>> sctp_transport_destroy_rcu() the transport is being kfree()'d, and if
>> we're unlucky enough we could run into corrupted pointers. Probably
>> that's more of theoretical nature, but it's safer to have this simple
>> fix.
>>
>> Introduced by commit 8c98653f ("sctp: sctp_close: fix release of
>> bindings
>> for deferred call_rcu's"). I also did the 8c98653f regression test and
>> it's fine that way.
>>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>
> Acked-by: Vlad Yasevich <vyasevich@gmail.com>
Applied, thanks everyone.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-08-13 5:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-09 14:25 [PATCH net v3] net: sctp: sctp_transport_destroy{,_rcu}: fix potential pointer corruption Daniel Borkmann
2013-08-09 14:25 ` Daniel Borkmann
2013-08-09 14:42 ` Vlad Yasevich
2013-08-09 14:42 ` Vlad Yasevich
2013-08-13 5:14 ` David Miller
2013-08-13 5:14 ` David Miller
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.