Commit 2757a4dc authored by David Howells's avatar David Howells
Browse files

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: 341f741f ("afs: Refcount the afs_call struct")
Fixes: 45218193 ("afs: Trace afs_server usage")
Fixes: 977e5f8e ("afs: Split the usage count on struct afs_server")
Signed-off-by: default avatarDavid 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
parent c56f9ec8

fs/afs/cmservice.c

+1 −1
Original line number Diff line number Diff line
@@ -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);

fs/afs/rxrpc.c

+6 −5
Original line number Diff line number Diff line
@@ -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));

fs/afs/server.c

+13 −9
Original line number Diff line number Diff line
@@ -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);

include/trace/events/afs.h

+6 −6
Original line number Diff line number Diff line
@@ -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;