| From d6a2d598292c8d9430bcfa17b4a60f7a4ee73398 Mon Sep 17 00:00:00 2001 |
| From: David Howells <dhowells@redhat.com> |
| Date: Thu, 6 Feb 2020 13:57:40 +0000 |
| Subject: [PATCH] rxrpc: Fix call RCU cleanup using non-bh-safe locks |
| |
| commit 963485d436ccc2810177a7b08af22336ec2af67b upstream. |
| |
| rxrpc_rcu_destroy_call(), which is called as an RCU callback to clean up a |
| put call, calls rxrpc_put_connection() which, deep in its bowels, takes a |
| number of spinlocks in a non-BH-safe way, including rxrpc_conn_id_lock and |
| local->client_conns_lock. RCU callbacks, however, are normally called from |
| softirq context, which can cause lockdep to notice the locking |
| inconsistency. |
| |
| To get lockdep to detect this, it's necessary to have the connection |
| cleaned up on the put at the end of the last of its calls, though normally |
| the clean up is deferred. This can be induced, however, by starting a call |
| on an AF_RXRPC socket and then closing the socket without reading the |
| reply. |
| |
| Fix this by having rxrpc_rcu_destroy_call() punt the destruction to a |
| workqueue if in softirq-mode and defer the destruction to process context. |
| |
| Note that another way to fix this could be to add a bunch of bh-disable |
| annotations to the spinlocks concerned - and there might be more than just |
| those two - but that means spending more time with BHs disabled. |
| |
| Note also that some of these places were covered by bh-disable spinlocks |
| belonging to the rxrpc_transport object, but these got removed without the |
| _bh annotation being retained on the next lock in. |
| |
| Fixes: 999b69f89241 ("rxrpc: Kill the client connection bundle concept") |
| Reported-by: syzbot+d82f3ac8d87e7ccbb2c9@syzkaller.appspotmail.com |
| Reported-by: syzbot+3f1fd6b8cbf8702d134e@syzkaller.appspotmail.com |
| Signed-off-by: David Howells <dhowells@redhat.com> |
| cc: Hillf Danton <hdanton@sina.com> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c |
| index bb920429580f..871dc0cc0149 100644 |
| --- a/net/rxrpc/call_object.c |
| +++ b/net/rxrpc/call_object.c |
| @@ -548,11 +548,11 @@ void rxrpc_put_call(struct rxrpc_call *call, enum rxrpc_call_trace op) |
| } |
| |
| /* |
| - * Final call destruction under RCU. |
| + * Final call destruction - but must be done in process context. |
| */ |
| -static void rxrpc_rcu_destroy_call(struct rcu_head *rcu) |
| +static void rxrpc_destroy_call(struct work_struct *work) |
| { |
| - struct rxrpc_call *call = container_of(rcu, struct rxrpc_call, rcu); |
| + struct rxrpc_call *call = container_of(work, struct rxrpc_call, processor); |
| struct rxrpc_net *rxnet = call->rxnet; |
| |
| rxrpc_put_connection(call->conn); |
| @@ -565,6 +565,22 @@ static void rxrpc_rcu_destroy_call(struct rcu_head *rcu) |
| } |
| |
| /* |
| + * Final call destruction under RCU. |
| + */ |
| +static void rxrpc_rcu_destroy_call(struct rcu_head *rcu) |
| +{ |
| + struct rxrpc_call *call = container_of(rcu, struct rxrpc_call, rcu); |
| + |
| + if (in_softirq()) { |
| + INIT_WORK(&call->processor, rxrpc_destroy_call); |
| + if (!rxrpc_queue_work(&call->processor)) |
| + BUG(); |
| + } else { |
| + rxrpc_destroy_call(&call->processor); |
| + } |
| +} |
| + |
| +/* |
| * clean up a call |
| */ |
| void rxrpc_cleanup_call(struct rxrpc_call *call) |
| -- |
| 2.7.4 |
| |