mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 02:30:34 +02:00 
			
		
		
		
	afs: Fix access after dec in put functions
Reference-putting functions should not access the object being put after decrementing the refcount unless they reduce the refcount to zero. Fix a couple of instances of this in afs by copying the information to be logged by tracepoint to local variables before doing the decrement. [Fixed a bit in afs_put_server() that I'd missed but Marc caught] Fixes:341f741f04("afs: Refcount the afs_call struct") Fixes:4521819369("afs: Trace afs_server usage") Fixes:977e5f8ed0("afs: Split the usage count on struct afs_server") Signed-off-by: David Howells <dhowells@redhat.com> cc: Marc Dionne <marc.dionne@auristor.com> cc: linux-afs@lists.infradead.org Link: https://lore.kernel.org/r/165911278430.3745403.16526310736054780645.stgit@warthog.procyon.org.uk/ # v1
This commit is contained in:
		
							parent
							
								
									c56f9ec8b2
								
							
						
					
					
						commit
						2757a4dc18
					
				
					 4 changed files with 26 additions and 21 deletions
				
			
		| 
						 | 
				
			
			@ -212,7 +212,7 @@ static void SRXAFSCB_CallBack(struct work_struct *work)
 | 
			
		|||
	 * to maintain cache coherency.
 | 
			
		||||
	 */
 | 
			
		||||
	if (call->server) {
 | 
			
		||||
		trace_afs_server(call->server,
 | 
			
		||||
		trace_afs_server(call->server->debug_id,
 | 
			
		||||
				 refcount_read(&call->server->ref),
 | 
			
		||||
				 atomic_read(&call->server->active),
 | 
			
		||||
				 afs_server_trace_callback);
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -152,7 +152,7 @@ static struct afs_call *afs_alloc_call(struct afs_net *net,
 | 
			
		|||
	call->iter = &call->def_iter;
 | 
			
		||||
 | 
			
		||||
	o = atomic_inc_return(&net->nr_outstanding_calls);
 | 
			
		||||
	trace_afs_call(call, afs_call_trace_alloc, 1, o,
 | 
			
		||||
	trace_afs_call(call->debug_id, afs_call_trace_alloc, 1, o,
 | 
			
		||||
		       __builtin_return_address(0));
 | 
			
		||||
	return call;
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			@ -163,12 +163,13 @@ static struct afs_call *afs_alloc_call(struct afs_net *net,
 | 
			
		|||
void afs_put_call(struct afs_call *call)
 | 
			
		||||
{
 | 
			
		||||
	struct afs_net *net = call->net;
 | 
			
		||||
	unsigned int debug_id = call->debug_id;
 | 
			
		||||
	bool zero;
 | 
			
		||||
	int r, o;
 | 
			
		||||
 | 
			
		||||
	zero = __refcount_dec_and_test(&call->ref, &r);
 | 
			
		||||
	o = atomic_read(&net->nr_outstanding_calls);
 | 
			
		||||
	trace_afs_call(call, afs_call_trace_put, r - 1, o,
 | 
			
		||||
	trace_afs_call(debug_id, afs_call_trace_put, r - 1, o,
 | 
			
		||||
		       __builtin_return_address(0));
 | 
			
		||||
 | 
			
		||||
	if (zero) {
 | 
			
		||||
| 
						 | 
				
			
			@ -186,7 +187,7 @@ void afs_put_call(struct afs_call *call)
 | 
			
		|||
		afs_put_addrlist(call->alist);
 | 
			
		||||
		kfree(call->request);
 | 
			
		||||
 | 
			
		||||
		trace_afs_call(call, afs_call_trace_free, 0, o,
 | 
			
		||||
		trace_afs_call(call->debug_id, afs_call_trace_free, 0, o,
 | 
			
		||||
			       __builtin_return_address(0));
 | 
			
		||||
		kfree(call);
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -203,7 +204,7 @@ static struct afs_call *afs_get_call(struct afs_call *call,
 | 
			
		|||
 | 
			
		||||
	__refcount_inc(&call->ref, &r);
 | 
			
		||||
 | 
			
		||||
	trace_afs_call(call, why, r + 1,
 | 
			
		||||
	trace_afs_call(call->debug_id, why, r + 1,
 | 
			
		||||
		       atomic_read(&call->net->nr_outstanding_calls),
 | 
			
		||||
		       __builtin_return_address(0));
 | 
			
		||||
	return call;
 | 
			
		||||
| 
						 | 
				
			
			@ -677,7 +678,7 @@ static void afs_wake_up_async_call(struct sock *sk, struct rxrpc_call *rxcall,
 | 
			
		|||
	call->need_attention = true;
 | 
			
		||||
 | 
			
		||||
	if (__refcount_inc_not_zero(&call->ref, &r)) {
 | 
			
		||||
		trace_afs_call(call, afs_call_trace_wake, r + 1,
 | 
			
		||||
		trace_afs_call(call->debug_id, afs_call_trace_wake, r + 1,
 | 
			
		||||
			       atomic_read(&call->net->nr_outstanding_calls),
 | 
			
		||||
			       __builtin_return_address(0));
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -243,7 +243,7 @@ static struct afs_server *afs_alloc_server(struct afs_cell *cell,
 | 
			
		|||
	server->rtt = UINT_MAX;
 | 
			
		||||
 | 
			
		||||
	afs_inc_servers_outstanding(net);
 | 
			
		||||
	trace_afs_server(server, 1, 1, afs_server_trace_alloc);
 | 
			
		||||
	trace_afs_server(server->debug_id, 1, 1, afs_server_trace_alloc);
 | 
			
		||||
	_leave(" = %p", server);
 | 
			
		||||
	return server;
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -352,10 +352,12 @@ void afs_servers_timer(struct timer_list *timer)
 | 
			
		|||
struct afs_server *afs_get_server(struct afs_server *server,
 | 
			
		||||
				  enum afs_server_trace reason)
 | 
			
		||||
{
 | 
			
		||||
	unsigned int a;
 | 
			
		||||
	int r;
 | 
			
		||||
 | 
			
		||||
	__refcount_inc(&server->ref, &r);
 | 
			
		||||
	trace_afs_server(server, r + 1, atomic_read(&server->active), reason);
 | 
			
		||||
	a = atomic_read(&server->active);
 | 
			
		||||
	trace_afs_server(server->debug_id, r + 1, a, reason);
 | 
			
		||||
	return server;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -372,7 +374,7 @@ static struct afs_server *afs_maybe_use_server(struct afs_server *server,
 | 
			
		|||
		return NULL;
 | 
			
		||||
 | 
			
		||||
	a = atomic_inc_return(&server->active);
 | 
			
		||||
	trace_afs_server(server, r + 1, a, reason);
 | 
			
		||||
	trace_afs_server(server->debug_id, r + 1, a, reason);
 | 
			
		||||
	return server;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -387,7 +389,7 @@ struct afs_server *afs_use_server(struct afs_server *server, enum afs_server_tra
 | 
			
		|||
	__refcount_inc(&server->ref, &r);
 | 
			
		||||
	a = atomic_inc_return(&server->active);
 | 
			
		||||
 | 
			
		||||
	trace_afs_server(server, r + 1, a, reason);
 | 
			
		||||
	trace_afs_server(server->debug_id, r + 1, a, reason);
 | 
			
		||||
	return server;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -397,14 +399,16 @@ struct afs_server *afs_use_server(struct afs_server *server, enum afs_server_tra
 | 
			
		|||
void afs_put_server(struct afs_net *net, struct afs_server *server,
 | 
			
		||||
		    enum afs_server_trace reason)
 | 
			
		||||
{
 | 
			
		||||
	unsigned int a, debug_id = server->debug_id;
 | 
			
		||||
	bool zero;
 | 
			
		||||
	int r;
 | 
			
		||||
 | 
			
		||||
	if (!server)
 | 
			
		||||
		return;
 | 
			
		||||
 | 
			
		||||
	a = atomic_inc_return(&server->active);
 | 
			
		||||
	zero = __refcount_dec_and_test(&server->ref, &r);
 | 
			
		||||
	trace_afs_server(server, r - 1, atomic_read(&server->active), reason);
 | 
			
		||||
	trace_afs_server(debug_id, r - 1, a, reason);
 | 
			
		||||
	if (unlikely(zero))
 | 
			
		||||
		__afs_put_server(net, server);
 | 
			
		||||
}
 | 
			
		||||
| 
						 | 
				
			
			@ -441,7 +445,7 @@ static void afs_server_rcu(struct rcu_head *rcu)
 | 
			
		|||
{
 | 
			
		||||
	struct afs_server *server = container_of(rcu, struct afs_server, rcu);
 | 
			
		||||
 | 
			
		||||
	trace_afs_server(server, refcount_read(&server->ref),
 | 
			
		||||
	trace_afs_server(server->debug_id, refcount_read(&server->ref),
 | 
			
		||||
			 atomic_read(&server->active), afs_server_trace_free);
 | 
			
		||||
	afs_put_addrlist(rcu_access_pointer(server->addresses));
 | 
			
		||||
	kfree(server);
 | 
			
		||||
| 
						 | 
				
			
			@ -492,7 +496,7 @@ static void afs_gc_servers(struct afs_net *net, struct afs_server *gc_list)
 | 
			
		|||
 | 
			
		||||
		active = atomic_read(&server->active);
 | 
			
		||||
		if (active == 0) {
 | 
			
		||||
			trace_afs_server(server, refcount_read(&server->ref),
 | 
			
		||||
			trace_afs_server(server->debug_id, refcount_read(&server->ref),
 | 
			
		||||
					 active, afs_server_trace_gc);
 | 
			
		||||
			next = rcu_dereference_protected(
 | 
			
		||||
				server->uuid_next, lockdep_is_held(&net->fs_lock.lock));
 | 
			
		||||
| 
						 | 
				
			
			@ -558,7 +562,7 @@ void afs_manage_servers(struct work_struct *work)
 | 
			
		|||
		_debug("manage %pU %u", &server->uuid, active);
 | 
			
		||||
 | 
			
		||||
		if (purging) {
 | 
			
		||||
			trace_afs_server(server, refcount_read(&server->ref),
 | 
			
		||||
			trace_afs_server(server->debug_id, refcount_read(&server->ref),
 | 
			
		||||
					 active, afs_server_trace_purging);
 | 
			
		||||
			if (active != 0)
 | 
			
		||||
				pr_notice("Can't purge s=%08x\n", server->debug_id);
 | 
			
		||||
| 
						 | 
				
			
			@ -638,7 +642,7 @@ static noinline bool afs_update_server_record(struct afs_operation *op,
 | 
			
		|||
 | 
			
		||||
	_enter("");
 | 
			
		||||
 | 
			
		||||
	trace_afs_server(server, refcount_read(&server->ref),
 | 
			
		||||
	trace_afs_server(server->debug_id, refcount_read(&server->ref),
 | 
			
		||||
			 atomic_read(&server->active),
 | 
			
		||||
			 afs_server_trace_update);
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -727,10 +727,10 @@ TRACE_EVENT(afs_cb_call,
 | 
			
		|||
	    );
 | 
			
		||||
 | 
			
		||||
TRACE_EVENT(afs_call,
 | 
			
		||||
	    TP_PROTO(struct afs_call *call, enum afs_call_trace op,
 | 
			
		||||
	    TP_PROTO(unsigned int call_debug_id, enum afs_call_trace op,
 | 
			
		||||
		     int ref, int outstanding, const void *where),
 | 
			
		||||
 | 
			
		||||
	    TP_ARGS(call, op, ref, outstanding, where),
 | 
			
		||||
	    TP_ARGS(call_debug_id, op, ref, outstanding, where),
 | 
			
		||||
 | 
			
		||||
	    TP_STRUCT__entry(
 | 
			
		||||
		    __field(unsigned int,		call		)
 | 
			
		||||
| 
						 | 
				
			
			@ -741,7 +741,7 @@ TRACE_EVENT(afs_call,
 | 
			
		|||
			     ),
 | 
			
		||||
 | 
			
		||||
	    TP_fast_assign(
 | 
			
		||||
		    __entry->call = call->debug_id;
 | 
			
		||||
		    __entry->call = call_debug_id;
 | 
			
		||||
		    __entry->op = op;
 | 
			
		||||
		    __entry->ref = ref;
 | 
			
		||||
		    __entry->outstanding = outstanding;
 | 
			
		||||
| 
						 | 
				
			
			@ -1433,10 +1433,10 @@ TRACE_EVENT(afs_cb_miss,
 | 
			
		|||
	    );
 | 
			
		||||
 | 
			
		||||
TRACE_EVENT(afs_server,
 | 
			
		||||
	    TP_PROTO(struct afs_server *server, int ref, int active,
 | 
			
		||||
	    TP_PROTO(unsigned int server_debug_id, int ref, int active,
 | 
			
		||||
		     enum afs_server_trace reason),
 | 
			
		||||
 | 
			
		||||
	    TP_ARGS(server, ref, active, reason),
 | 
			
		||||
	    TP_ARGS(server_debug_id, ref, active, reason),
 | 
			
		||||
 | 
			
		||||
	    TP_STRUCT__entry(
 | 
			
		||||
		    __field(unsigned int,		server		)
 | 
			
		||||
| 
						 | 
				
			
			@ -1446,7 +1446,7 @@ TRACE_EVENT(afs_server,
 | 
			
		|||
			     ),
 | 
			
		||||
 | 
			
		||||
	    TP_fast_assign(
 | 
			
		||||
		    __entry->server = server->debug_id;
 | 
			
		||||
		    __entry->server = server_debug_id;
 | 
			
		||||
		    __entry->ref = ref;
 | 
			
		||||
		    __entry->active = active;
 | 
			
		||||
		    __entry->reason = reason;
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in a new issue