From 7ef43ebaa538e0cc9063cbf84593a05091bcace2 Mon Sep 17 00:00:00 2001 From: Allan Stephens Date: Mon, 12 May 2008 15:42:28 -0700 Subject: tipc: Fix race condition when creating socket or native port This patch eliminates the (very remote) chance of a crash resulting from a partially initialized socket or native port unexpectedly receiving a message. Now, during the creation of a socket or native port, the underlying generic port's lock is not released until all initialization required to handle incoming messages has been done. Signed-off-by: Allan Stephens Signed-off-by: David S. Miller --- net/tipc/socket.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'net/tipc/socket.c') diff --git a/net/tipc/socket.c b/net/tipc/socket.c index 230f9ca2ad6..38f48795b40 100644 --- a/net/tipc/socket.c +++ b/net/tipc/socket.c @@ -188,6 +188,7 @@ static int tipc_create(struct net *net, struct socket *sock, int protocol) const struct proto_ops *ops; socket_state state; struct sock *sk; + struct tipc_port *tp_ptr; u32 portref; /* Validate arguments */ @@ -225,7 +226,7 @@ static int tipc_create(struct net *net, struct socket *sock, int protocol) /* Allocate TIPC port for socket to use */ portref = tipc_createport_raw(sk, &dispatch, &wakeupdispatch, - TIPC_LOW_IMPORTANCE); + TIPC_LOW_IMPORTANCE, &tp_ptr); if (unlikely(portref == 0)) { sk_free(sk); return -ENOMEM; @@ -241,6 +242,8 @@ static int tipc_create(struct net *net, struct socket *sock, int protocol) sk->sk_backlog_rcv = backlog_rcv; tipc_sk(sk)->p = tipc_get_port(portref); + spin_unlock_bh(tp_ptr->lock); + if (sock->state == SS_READY) { tipc_set_portunreturnable(portref, 1); if (sock->type == SOCK_DGRAM) -- cgit v1.2.3-70-g09d2 From 0ea522416b658dedfc9d565b331624a55a6260ad Mon Sep 17 00:00:00 2001 From: Allan Stephens Date: Mon, 14 Jul 2008 22:42:19 -0700 Subject: tipc: Remove unneeded parameter to tipc_createport_raw() This patch eliminates an unneeded parameter when creating a low-level TIPC port object. Instead of returning both the pointer to the port structure and the port's reference ID, it now returns only the pointer since the port structure contains the reference ID as one of its fields. Signed-off-by: Allan Stephens Signed-off-by: David S. Miller --- include/net/tipc/tipc_port.h | 13 +++---------- net/tipc/port.c | 28 ++++++++++------------------ net/tipc/socket.c | 15 +++++++-------- 3 files changed, 20 insertions(+), 36 deletions(-) (limited to 'net/tipc/socket.c') diff --git a/include/net/tipc/tipc_port.h b/include/net/tipc/tipc_port.h index 9923e41a821..c54917cbfa4 100644 --- a/include/net/tipc/tipc_port.h +++ b/include/net/tipc/tipc_port.h @@ -2,7 +2,7 @@ * include/net/tipc/tipc_port.h: Include file for privileged access to TIPC ports * * Copyright (c) 1994-2007, Ericsson AB - * Copyright (c) 2005-2007, Wind River Systems + * Copyright (c) 2005-2008, Wind River Systems * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -75,17 +75,10 @@ struct tipc_port { }; -/** - * tipc_createport_raw - create a native TIPC port and return it's reference - * - * Note: 'dispatcher' and 'wakeup' deliver a locked port. - */ - -u32 tipc_createport_raw(void *usr_handle, +struct tipc_port *tipc_createport_raw(void *usr_handle, u32 (*dispatcher)(struct tipc_port *, struct sk_buff *), void (*wakeup)(struct tipc_port *), - const u32 importance, - struct tipc_port **tp_ptr); + const u32 importance); int tipc_reject_msg(struct sk_buff *buf, u32 err); diff --git a/net/tipc/port.c b/net/tipc/port.c index 2e0cff408ff..ffba1e7f06d 100644 --- a/net/tipc/port.c +++ b/net/tipc/port.c @@ -2,7 +2,7 @@ * net/tipc/port.c: TIPC port code * * Copyright (c) 1992-2007, Ericsson AB - * Copyright (c) 2004-2007, Wind River Systems + * Copyright (c) 2004-2008, Wind River Systems * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -213,16 +213,13 @@ exit: /** * tipc_createport_raw - create a generic TIPC port * - * Returns port reference, or 0 if unable to create it - * - * Note: The newly created port is returned in the locked state. + * Returns pointer to (locked) TIPC port, or NULL if unable to create it */ -u32 tipc_createport_raw(void *usr_handle, +struct tipc_port *tipc_createport_raw(void *usr_handle, u32 (*dispatcher)(struct tipc_port *, struct sk_buff *), void (*wakeup)(struct tipc_port *), - const u32 importance, - struct tipc_port **tp_ptr) + const u32 importance) { struct port *p_ptr; struct tipc_msg *msg; @@ -231,13 +228,13 @@ u32 tipc_createport_raw(void *usr_handle, p_ptr = kzalloc(sizeof(*p_ptr), GFP_ATOMIC); if (!p_ptr) { warn("Port creation failed, no memory\n"); - return 0; + return NULL; } ref = tipc_ref_acquire(p_ptr, &p_ptr->publ.lock); if (!ref) { warn("Port creation failed, reference table exhausted\n"); kfree(p_ptr); - return 0; + return NULL; } p_ptr->publ.usr_handle = usr_handle; @@ -260,8 +257,7 @@ u32 tipc_createport_raw(void *usr_handle, INIT_LIST_HEAD(&p_ptr->port_list); list_add_tail(&p_ptr->port_list, &ports); spin_unlock_bh(&tipc_port_list_lock); - *tp_ptr = &p_ptr->publ; - return ref; + return &(p_ptr->publ); } int tipc_deleteport(u32 ref) @@ -1044,21 +1040,18 @@ int tipc_createport(u32 user_ref, { struct user_port *up_ptr; struct port *p_ptr; - struct tipc_port *tp_ptr; - u32 ref; up_ptr = kmalloc(sizeof(*up_ptr), GFP_ATOMIC); if (!up_ptr) { warn("Port creation failed, no memory\n"); return -ENOMEM; } - ref = tipc_createport_raw(NULL, port_dispatcher, port_wakeup, - importance, &tp_ptr); - if (ref == 0) { + p_ptr = (struct port *)tipc_createport_raw(NULL, port_dispatcher, + port_wakeup, importance); + if (!p_ptr) { kfree(up_ptr); return -ENOMEM; } - p_ptr = (struct port *)tp_ptr; p_ptr->user_port = up_ptr; up_ptr->user_ref = user_ref; @@ -1074,7 +1067,6 @@ int tipc_createport(u32 user_ref, INIT_LIST_HEAD(&up_ptr->uport_list); tipc_reg_add_port(up_ptr); *portref = p_ptr->publ.ref; - dbg(" tipc_createport: %x with ref %u\n", p_ptr, p_ptr->publ.ref); tipc_port_unlock(p_ptr); return TIPC_OK; } diff --git a/net/tipc/socket.c b/net/tipc/socket.c index 38f48795b40..9c362c5759b 100644 --- a/net/tipc/socket.c +++ b/net/tipc/socket.c @@ -2,7 +2,7 @@ * net/tipc/socket.c: TIPC socket API * * Copyright (c) 2001-2007, Ericsson AB - * Copyright (c) 2004-2007, Wind River Systems + * Copyright (c) 2004-2008, Wind River Systems * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -189,7 +189,6 @@ static int tipc_create(struct net *net, struct socket *sock, int protocol) socket_state state; struct sock *sk; struct tipc_port *tp_ptr; - u32 portref; /* Validate arguments */ @@ -225,9 +224,9 @@ static int tipc_create(struct net *net, struct socket *sock, int protocol) /* Allocate TIPC port for socket to use */ - portref = tipc_createport_raw(sk, &dispatch, &wakeupdispatch, - TIPC_LOW_IMPORTANCE, &tp_ptr); - if (unlikely(portref == 0)) { + tp_ptr = tipc_createport_raw(sk, &dispatch, &wakeupdispatch, + TIPC_LOW_IMPORTANCE); + if (unlikely(!tp_ptr)) { sk_free(sk); return -ENOMEM; } @@ -240,14 +239,14 @@ static int tipc_create(struct net *net, struct socket *sock, int protocol) sock_init_data(sock, sk); sk->sk_rcvtimeo = msecs_to_jiffies(CONN_TIMEOUT_DEFAULT); sk->sk_backlog_rcv = backlog_rcv; - tipc_sk(sk)->p = tipc_get_port(portref); + tipc_sk(sk)->p = tp_ptr; spin_unlock_bh(tp_ptr->lock); if (sock->state == SS_READY) { - tipc_set_portunreturnable(portref, 1); + tipc_set_portunreturnable(tp_ptr->ref, 1); if (sock->type == SOCK_DGRAM) - tipc_set_portunreliable(portref, 1); + tipc_set_portunreliable(tp_ptr->ref, 1); } atomic_inc(&tipc_user_count); -- cgit v1.2.3-70-g09d2 From 8642bd9e04f51980b2b6293c66acf7e388c9a6e7 Mon Sep 17 00:00:00 2001 From: Allan Stephens Date: Mon, 14 Jul 2008 22:42:51 -0700 Subject: tipc: Optimize pointer dereferencing when receiving stream data This patch eliminates an unnecessary pointer dereference when accessing a stream-based socket's receive queue. Signed-off-by: Allan Stephens Signed-off-by: David S. Miller --- net/tipc/socket.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/tipc/socket.c') diff --git a/net/tipc/socket.c b/net/tipc/socket.c index 9c362c5759b..ddcb2a753aa 100644 --- a/net/tipc/socket.c +++ b/net/tipc/socket.c @@ -1133,7 +1133,7 @@ restart: /* Loop around if more data is required */ if ((sz_copied < buf_len) /* didn't get all requested data */ - && (!skb_queue_empty(&sock->sk->sk_receive_queue) || + && (!skb_queue_empty(&sk->sk_receive_queue) || (flags & MSG_WAITALL)) /* ... and more is ready or required */ && (!(flags & MSG_PEEK)) /* ... and aren't just peeking at data */ -- cgit v1.2.3-70-g09d2 From 2da59918e26837f305131cfac9c0f1b3b42bb8ae Mon Sep 17 00:00:00 2001 From: Allan Stephens Date: Mon, 14 Jul 2008 22:43:32 -0700 Subject: tipc: Fix race condition that could cause accept() to fail This patch ensurs that accept() returns successfully even when the newly created socket is immediately disconnected by its peer. Previously, accept() would fail if it was unable to pass back the optional address info for the socket's peer before the socket became disconnected; TIPC now allows accept() to gather peer address information after disconnection. As a bonus, the revised code accesses the socket's port more efficiently, without the overhead incurred by a reference table lookup. Signed-off-by: Allan Stephens Signed-off-by: David S. Miller --- net/tipc/socket.c | 42 ++++++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 20 deletions(-) (limited to 'net/tipc/socket.c') diff --git a/net/tipc/socket.c b/net/tipc/socket.c index ddcb2a753aa..1848693ebb8 100644 --- a/net/tipc/socket.c +++ b/net/tipc/socket.c @@ -63,6 +63,7 @@ struct tipc_sock { struct sock sk; struct tipc_port *p; + struct tipc_portid peer_name; }; #define tipc_sk(sk) ((struct tipc_sock *)(sk)) @@ -377,27 +378,29 @@ static int bind(struct socket *sock, struct sockaddr *uaddr, int uaddr_len) * @sock: socket structure * @uaddr: area for returned socket address * @uaddr_len: area for returned length of socket address - * @peer: 0 to obtain socket name, 1 to obtain peer socket name + * @peer: 0 = own ID, 1 = current peer ID, 2 = current/former peer ID * * Returns 0 on success, errno otherwise * - * NOTE: This routine doesn't need to take the socket lock since it doesn't - * access any non-constant socket information. + * NOTE: This routine doesn't need to take the socket lock since it only + * accesses socket information that is unchanging (or which changes in + * a completely predictable manner). */ static int get_name(struct socket *sock, struct sockaddr *uaddr, int *uaddr_len, int peer) { struct sockaddr_tipc *addr = (struct sockaddr_tipc *)uaddr; - u32 portref = tipc_sk_port(sock->sk)->ref; - u32 res; + struct tipc_sock *tsock = tipc_sk(sock->sk); if (peer) { - res = tipc_peer(portref, &addr->addr.id); - if (res) - return res; + if ((sock->state != SS_CONNECTED) && + ((peer != 2) || (sock->state != SS_DISCONNECTING))) + return -ENOTCONN; + addr->addr.id.ref = tsock->peer_name.ref; + addr->addr.id.node = tsock->peer_name.node; } else { - tipc_ownidentity(portref, &addr->addr.id); + tipc_ownidentity(tsock->p->ref, &addr->addr.id); } *uaddr_len = sizeof(*addr); @@ -766,18 +769,17 @@ exit: static int auto_connect(struct socket *sock, struct tipc_msg *msg) { - struct tipc_port *tport = tipc_sk_port(sock->sk); - struct tipc_portid peer; + struct tipc_sock *tsock = tipc_sk(sock->sk); if (msg_errcode(msg)) { sock->state = SS_DISCONNECTING; return -ECONNREFUSED; } - peer.ref = msg_origport(msg); - peer.node = msg_orignode(msg); - tipc_connect2port(tport->ref, &peer); - tipc_set_portimportance(tport->ref, msg_importance(msg)); + tsock->peer_name.ref = msg_origport(msg); + tsock->peer_name.node = msg_orignode(msg); + tipc_connect2port(tsock->p->ref, &tsock->peer_name); + tipc_set_portimportance(tsock->p->ref, msg_importance(msg)); sock->state = SS_CONNECTED; return 0; } @@ -1529,9 +1531,9 @@ static int accept(struct socket *sock, struct socket *new_sock, int flags) res = tipc_create(sock_net(sock->sk), new_sock, 0); if (!res) { struct sock *new_sk = new_sock->sk; - struct tipc_port *new_tport = tipc_sk_port(new_sk); + struct tipc_sock *new_tsock = tipc_sk(new_sk); + struct tipc_port *new_tport = new_tsock->p; u32 new_ref = new_tport->ref; - struct tipc_portid id; struct tipc_msg *msg = buf_msg(buf); lock_sock(new_sk); @@ -1545,9 +1547,9 @@ static int accept(struct socket *sock, struct socket *new_sock, int flags) /* Connect new socket to it's peer */ - id.ref = msg_origport(msg); - id.node = msg_orignode(msg); - tipc_connect2port(new_ref, &id); + new_tsock->peer_name.ref = msg_origport(msg); + new_tsock->peer_name.node = msg_orignode(msg); + tipc_connect2port(new_ref, &new_tsock->peer_name); new_sock->state = SS_CONNECTED; tipc_set_portimportance(new_ref, msg_importance(msg)); -- cgit v1.2.3-70-g09d2