From 456b2f2dc7e585e1a031214c5698f1b00e02448b Mon Sep 17 00:00:00 2001 From: David Howells Date: Mon, 14 Sep 2020 12:57:13 +0100 Subject: [PATCH 1/5] rxrpc: Fix an error goto in rxrpc_connect_call() Fix an error-handling goto in rxrpc_connect_call() whereby it will jump to free the bundle it failed to allocate. Fixes: 245500d853e9 ("rxrpc: Rewrite the client connection manager") Reported-by: kernel test robot Reported-by: Dan Carpenter Signed-off-by: David Howells --- net/rxrpc/conn_client.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c index 0e4e1879c24d..180be4da8d26 100644 --- a/net/rxrpc/conn_client.c +++ b/net/rxrpc/conn_client.c @@ -724,8 +724,9 @@ granted_channel: /* Paired with the write barrier in rxrpc_activate_one_channel(). */ smp_rmb(); -out: +out_put_bundle: rxrpc_put_bundle(bundle); +out: _leave(" = %d", ret); return ret; @@ -742,7 +743,7 @@ wait_failed: trace_rxrpc_client(call->conn, ret, rxrpc_client_chan_wait_failed); rxrpc_set_call_completion(call, RXRPC_CALL_LOCAL_ERROR, 0, ret); rxrpc_disconnect_client_call(bundle, call); - goto out; + goto out_put_bundle; } /* From 96a9c425e234aac193afb9da6ca11fafc679c7e2 Mon Sep 17 00:00:00 2001 From: David Howells Date: Mon, 14 Sep 2020 13:02:58 +0100 Subject: [PATCH 2/5] rxrpc: Fix a missing NULL-pointer check in a trace Fix the rxrpc_client tracepoint to not dereference conn to get the cid if conn is NULL, as it does for other fields. RIP: 0010:trace_event_raw_event_rxrpc_client+0x7e/0xe0 [rxrpc] Call Trace: rxrpc_activate_channels+0x62/0xb0 [rxrpc] rxrpc_connect_call+0x481/0x650 [rxrpc] ? wake_up_q+0xa0/0xa0 ? rxrpc_kernel_begin_call+0x12a/0x1b0 [rxrpc] rxrpc_new_client_call+0x2a5/0x5e0 [rxrpc] Fixes: 245500d853e9 ("rxrpc: Rewrite the client connection manager") Reported-by: Marc Dionne Signed-off-by: David Howells Tested-by: Marc Dionne --- include/trace/events/rxrpc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h index 3b67d5981224..e70c90116eda 100644 --- a/include/trace/events/rxrpc.h +++ b/include/trace/events/rxrpc.h @@ -579,7 +579,7 @@ TRACE_EVENT(rxrpc_client, __entry->channel = channel; __entry->usage = conn ? atomic_read(&conn->usage) : -2; __entry->op = op; - __entry->cid = conn->proto.cid; + __entry->cid = conn ? conn->proto.cid : 0; ), TP_printk("C=%08x h=%2d %s i=%08x u=%d", From 8806245a3e7ad6aafc5bd8c58b0f0dea82a8e4c5 Mon Sep 17 00:00:00 2001 From: David Howells Date: Mon, 14 Sep 2020 13:10:00 +0100 Subject: [PATCH 3/5] rxrpc: Fix rxrpc_bundle::alloc_error to be signed The alloc_error field in the rxrpc_bundle struct should be signed as it has negative error codes assigned to it. Checks directly on it may then fail, and may produce a warning like this: net/rxrpc/conn_client.c:662 rxrpc_wait_for_channel() warn: 'bundle->alloc_error' is unsigned Fixes: 245500d853e9 ("rxrpc: Rewrite the client connection manager") Reported-by Dan Carpenter Signed-off-by: David Howells --- net/rxrpc/ar-internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h index cd5a80b34738..19f714386654 100644 --- a/net/rxrpc/ar-internal.h +++ b/net/rxrpc/ar-internal.h @@ -395,7 +395,7 @@ struct rxrpc_bundle { unsigned int debug_id; bool try_upgrade; /* True if the bundle is attempting upgrade */ bool alloc_conn; /* True if someone's getting a conn */ - unsigned short alloc_error; /* Error from last conn allocation */ + short alloc_error; /* Error from last conn allocation */ spinlock_t channel_lock; struct rb_node local_node; /* Node in local->client_conns */ struct list_head waiting_calls; /* Calls waiting for channels */ From 546a42410bf76075e157dbdd94c097b3bc44b25a Mon Sep 17 00:00:00 2001 From: David Howells Date: Mon, 14 Sep 2020 13:26:47 +0100 Subject: [PATCH 4/5] rxrpc: Fix conn bundle leak in net-namespace exit When the network namespace exits, rxrpc_clean_up_local_conns() needs to unbundle each client connection it evicts. Fix it to do this. kernel BUG at net/rxrpc/conn_object.c:481! RIP: 0010:rxrpc_destroy_all_connections.cold+0x11/0x13 net/rxrpc/conn_object.c:481 Call Trace: rxrpc_exit_net+0x1a4/0x2e0 net/rxrpc/net_ns.c:119 ops_exit_list+0xb0/0x160 net/core/net_namespace.c:186 cleanup_net+0x4ea/0xa00 net/core/net_namespace.c:603 process_one_work+0x94c/0x1670 kernel/workqueue.c:2269 worker_thread+0x64c/0x1120 kernel/workqueue.c:2415 kthread+0x3b5/0x4a0 kernel/kthread.c:292 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294 Fixes: 245500d853e9 ("rxrpc: Rewrite the client connection manager") Reported-by: syzbot+52071f826a617b9c76ed@syzkaller.appspotmail.com Signed-off-by: David Howells --- net/rxrpc/conn_client.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c index 180be4da8d26..0eb36ba52485 100644 --- a/net/rxrpc/conn_client.c +++ b/net/rxrpc/conn_client.c @@ -1112,6 +1112,7 @@ void rxrpc_clean_up_local_conns(struct rxrpc_local *local) conn = list_entry(graveyard.next, struct rxrpc_connection, cache_link); list_del_init(&conn->cache_link); + rxrpc_unbundle_conn(conn); rxrpc_put_connection(conn); } From f1b449748760e333a6aeef2455f3875be07c9b5a Mon Sep 17 00:00:00 2001 From: David Howells Date: Mon, 14 Sep 2020 15:58:14 +0100 Subject: [PATCH 5/5] rxrpc: Fix an overget of the conn bundle when setting up a client conn When setting up a client connection, a second ref is accidentally obtained on the connection bundle (we get one when allocating the conn and a second one when adding the conn to the bundle). Fix it to only use the ref obtained by rxrpc_alloc_client_connection() and not to add a second when adding the candidate conn to the bundle. Fixes: 245500d853e9 ("rxrpc: Rewrite the client connection manager") Signed-off-by: David Howells --- net/rxrpc/conn_client.c | 1 - 1 file changed, 1 deletion(-) diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c index 0eb36ba52485..78c845a4f1ad 100644 --- a/net/rxrpc/conn_client.c +++ b/net/rxrpc/conn_client.c @@ -433,7 +433,6 @@ static void rxrpc_add_conn_to_bundle(struct rxrpc_bundle *bundle, gfp_t gfp) if (!rxrpc_may_reuse_conn(old)) { if (old) trace_rxrpc_client(old, -1, rxrpc_client_replace); - candidate->bundle = rxrpc_get_bundle(bundle); candidate->bundle_shift = shift; bundle->conns[i] = candidate; for (j = 0; j < RXRPC_MAXCALLS; j++)