Commit 0e5d56c6 authored by Xin Long's avatar Xin Long Committed by Jakub Kicinski

tipc: set con sock in tipc_conn_alloc

A crash was reported by Wei Chen:

  BUG: kernel NULL pointer dereference, address: 0000000000000018
  RIP: 0010:tipc_conn_close+0x12/0x100
  Call Trace:
   tipc_topsrv_exit_net+0x139/0x320
   ops_exit_list.isra.9+0x49/0x80
   cleanup_net+0x31a/0x540
   process_one_work+0x3fa/0x9f0
   worker_thread+0x42/0x5c0

It was caused by !con->sock in tipc_conn_close(). In tipc_topsrv_accept(),
con is allocated in conn_idr then its sock is set:

  con = tipc_conn_alloc();
  ...                    <----[1]
  con->sock = newsock;

If tipc_conn_close() is called in anytime of [1], the null-pointer-def
is triggered by con->sock->sk due to con->sock is not yet set.

This patch fixes it by moving the con->sock setting to tipc_conn_alloc()
under s->idr_lock. So that con->sock can never be NULL when getting the
con from s->conn_idr. It will be also safer to move con->server and flag
CF_CONNECTED setting under s->idr_lock, as they should all be set before
tipc_conn_alloc() is called.

Fixes: c5fa7b3c ("tipc: introduce new TIPC server infrastructure")
Reported-by: default avatarWei Chen <harperchen1110@gmail.com>
Signed-off-by: default avatarXin Long <lucien.xin@gmail.com>
Acked-by: default avatarJon Maloy <jmaloy@redhat.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 1f0dd412
...@@ -176,7 +176,7 @@ static void tipc_conn_close(struct tipc_conn *con) ...@@ -176,7 +176,7 @@ static void tipc_conn_close(struct tipc_conn *con)
conn_put(con); conn_put(con);
} }
static struct tipc_conn *tipc_conn_alloc(struct tipc_topsrv *s) static struct tipc_conn *tipc_conn_alloc(struct tipc_topsrv *s, struct socket *sock)
{ {
struct tipc_conn *con; struct tipc_conn *con;
int ret; int ret;
...@@ -202,10 +202,11 @@ static struct tipc_conn *tipc_conn_alloc(struct tipc_topsrv *s) ...@@ -202,10 +202,11 @@ static struct tipc_conn *tipc_conn_alloc(struct tipc_topsrv *s)
} }
con->conid = ret; con->conid = ret;
s->idr_in_use++; s->idr_in_use++;
spin_unlock_bh(&s->idr_lock);
set_bit(CF_CONNECTED, &con->flags); set_bit(CF_CONNECTED, &con->flags);
con->server = s; con->server = s;
con->sock = sock;
spin_unlock_bh(&s->idr_lock);
return con; return con;
} }
...@@ -467,7 +468,7 @@ static void tipc_topsrv_accept(struct work_struct *work) ...@@ -467,7 +468,7 @@ static void tipc_topsrv_accept(struct work_struct *work)
ret = kernel_accept(lsock, &newsock, O_NONBLOCK); ret = kernel_accept(lsock, &newsock, O_NONBLOCK);
if (ret < 0) if (ret < 0)
return; return;
con = tipc_conn_alloc(srv); con = tipc_conn_alloc(srv, newsock);
if (IS_ERR(con)) { if (IS_ERR(con)) {
ret = PTR_ERR(con); ret = PTR_ERR(con);
sock_release(newsock); sock_release(newsock);
...@@ -479,7 +480,6 @@ static void tipc_topsrv_accept(struct work_struct *work) ...@@ -479,7 +480,6 @@ static void tipc_topsrv_accept(struct work_struct *work)
newsk->sk_data_ready = tipc_conn_data_ready; newsk->sk_data_ready = tipc_conn_data_ready;
newsk->sk_write_space = tipc_conn_write_space; newsk->sk_write_space = tipc_conn_write_space;
newsk->sk_user_data = con; newsk->sk_user_data = con;
con->sock = newsock;
write_unlock_bh(&newsk->sk_callback_lock); write_unlock_bh(&newsk->sk_callback_lock);
/* Wake up receive process in case of 'SYN+' message */ /* Wake up receive process in case of 'SYN+' message */
...@@ -577,12 +577,11 @@ bool tipc_topsrv_kern_subscr(struct net *net, u32 port, u32 type, u32 lower, ...@@ -577,12 +577,11 @@ bool tipc_topsrv_kern_subscr(struct net *net, u32 port, u32 type, u32 lower,
sub.filter = filter; sub.filter = filter;
*(u64 *)&sub.usr_handle = (u64)port; *(u64 *)&sub.usr_handle = (u64)port;
con = tipc_conn_alloc(tipc_topsrv(net)); con = tipc_conn_alloc(tipc_topsrv(net), NULL);
if (IS_ERR(con)) if (IS_ERR(con))
return false; return false;
*conid = con->conid; *conid = con->conid;
con->sock = NULL;
rc = tipc_conn_rcv_sub(tipc_topsrv(net), con, &sub); rc = tipc_conn_rcv_sub(tipc_topsrv(net), con, &sub);
if (rc >= 0) if (rc >= 0)
return true; return true;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment