tipc: set con sock in tipc_conn_alloc
[ Upstream commit 0e5d56c64afcd6fd2d132ea972605b66f8a7d3c4 ]
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: c5fa7b3cf3
("tipc: introduce new TIPC server infrastructure")
Reported-by: Wei Chen <harperchen1110@gmail.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Jon Maloy <jmaloy@redhat.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:

committed by
Greg Kroah-Hartman

parent
891daa95b0
commit
e87a077d09
@@ -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)
|
|||||||
}
|
}
|
||||||
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)
|
|||||||
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)
|
|||||||
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,
|
|||||||
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;
|
||||||
|
Reference in New Issue
Block a user