On 06/18/2013 04:55 AM, Daniel Borkmann wrote:
Rather instead of having the endpoint clean the garbage for the
socket, use a sk_destruct handler sctp_destruct_sock(), that does
the job for that when there are no more references on the socket.
With this patch it is possible to run sctp_put_port while the socket
is not locked.
The flow goes something like this:
sctp_close()
sk_bh_lock_sock();
sk_common_release()
sctp_destroy_sock()
endpoint_put()
endpoint_destroy() <-- This is where we used to do sctp_put_port
sk_bh_unlock_sock();
sock_put()
sk_free()
__sk_free()
sctp_destruct_sock()
sctp_put_port()
I haven't found any race conditions yet, but that doesn't mean they
don't exist.
I think an easy solution would be to do sctp_put_port in sctp_unhash(),
but I haven't traced all the paths.
Daniel, please take a look at this.
Thanks
-vlad
Signed-off-by: Daniel Borkmann <dborkman@xxxxxxxxxx>
---
net/sctp/endpointola.c | 7 -------
net/sctp/socket.c | 20 +++++++++++++++++++-
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index a8b2674..9a9ebd2 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -249,9 +249,6 @@ static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
{
SCTP_ASSERT(ep->base.dead, "Endpoint is not dead", return);
- /* Free up the HMAC transform. */
- crypto_free_hash(sctp_sk(ep->base.sk)->hmac);
-
/* Free the digest buffer */
kfree(ep->digest);
@@ -271,10 +268,6 @@ static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
memset(ep->secret_key, 0, sizeof(ep->secret_key));
- /* Remove and free the port */
- if (sctp_sk(ep->base.sk)->bind_hash)
- sctp_put_port(ep->base.sk);
-
/* Give up our hold on the sock. */
if (ep->base.sk)
sock_put(ep->base.sk);
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 68a6b70..67f721e 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -93,6 +93,7 @@ static int sctp_wait_for_packet(struct sock * sk, int *err, long *timeo_p);
static int sctp_wait_for_connect(struct sctp_association *, long *timeo_p);
static int sctp_wait_for_accept(struct sock *sk, long timeo);
static void sctp_wait_for_close(struct sock *sk, long timeo);
+static void sctp_destruct_sock(struct sock *sk);
static struct sctp_af *sctp_sockaddr_af(struct sctp_sock *opt,
union sctp_addr *addr, int len);
static int sctp_bindx_add(struct sock *, struct sockaddr *, int);
@@ -3966,6 +3967,8 @@ static int sctp_init_sock(struct sock *sk)
sp->hmac = NULL;
+ sk->sk_destruct = sctp_destruct_sock;
+
SCTP_DBG_OBJCNT_INC(sock);
local_bh_disable();
@@ -4002,6 +4005,21 @@ static void sctp_destroy_sock(struct sock *sk)
local_bh_enable();
}
+/* Triggered when there are no references on the socket anymore */
+static void sctp_destruct_sock(struct sock *sk)
+{
+ struct sctp_sock *sp = sctp_sk(sk);
+
+ /* Free up the HMAC transform. */
+ crypto_free_hash(sp->hmac);
+
+ /* Remove and free the port */
+ if (sp->bind_hash)
+ sctp_put_port(sk);
+
+ inet_sock_destruct(sk);
+}
+
/* API 4.1.7 shutdown() - TCP Style Syntax
* int shutdown(int socket, int how);
*
@@ -6842,7 +6860,7 @@ void sctp_copy_sock(struct sock *newsk, struct sock *sk,
newsk->sk_reuse = sk->sk_reuse;
newsk->sk_shutdown = sk->sk_shutdown;
- newsk->sk_destruct = inet_sock_destruct;
+ newsk->sk_destruct = sctp_destruct_sock;
newsk->sk_family = sk->sk_family;
newsk->sk_protocol = IPPROTO_SCTP;
newsk->sk_backlog_rcv = sk->sk_prot->backlog_rcv;
--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
