mirrors/linux
1
0
Fork 1
mirror of https://github.com/torvalds/linux.git synced 2025-11-04 10:40:15 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

afs: Fix access after dec in put functions

Browse source 
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:
David Howells 2022-07-06 11:26:14 +01:00
parent c56f9ec8b2
commit 2757a4dc18
4 changed files with 26 additions and 21 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

2
fs/afs/cmservice.c
View file

@ -212,7 +212,7 @@ static void SRXAFSCB_CallBack(struct work_struct *work)
* to maintain cache coherency. * to maintain cache coherency.
*/ */
if (call->server) { if (call->server) {
trace_afs_server(call->server, trace_afs_server(call->server->debug_id,
refcount_read(&call->server->ref), refcount_read(&call->server->ref),
atomic_read(&call->server->active), atomic_read(&call->server->active),
afs_server_trace_callback); afs_server_trace_callback);

11
fs/afs/rxrpc.c
View file

@ -152,7 +152,7 @@ static struct afs_call *afs_alloc_call(struct afs_net *net,
call->iter = &call->def_iter; call->iter = &call->def_iter;
o = atomic_inc_return(&net->nr_outstanding_calls); 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)); __builtin_return_address(0));
return call; 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) void afs_put_call(struct afs_call *call)
{ {
struct afs_net *net = call->net; struct afs_net *net = call->net;
unsigned int debug_id = call->debug_id;
bool zero; bool zero;
int r, o; int r, o;
zero = __refcount_dec_and_test(&call->ref, &r); zero = __refcount_dec_and_test(&call->ref, &r);
o = atomic_read(&net->nr_outstanding_calls); 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)); __builtin_return_address(0));
if (zero) { if (zero) {
@ -186,7 +187,7 @@ void afs_put_call(struct afs_call *call)
afs_put_addrlist(call->alist); afs_put_addrlist(call->alist);
kfree(call->request); 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)); __builtin_return_address(0));
kfree(call); kfree(call);
@ -203,7 +204,7 @@ static struct afs_call *afs_get_call(struct afs_call *call,
__refcount_inc(&call->ref, &r); __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), atomic_read(&call->net->nr_outstanding_calls),
__builtin_return_address(0)); __builtin_return_address(0));
return call; return call;
@ -677,7 +678,7 @@ static void afs_wake_up_async_call(struct sock *sk, struct rxrpc_call *rxcall,
call->need_attention = true; call->need_attention = true;
if (__refcount_inc_not_zero(&call->ref, &r)) { 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), atomic_read(&call->net->nr_outstanding_calls),
__builtin_return_address(0)); __builtin_return_address(0));

22
fs/afs/server.c
View file

@ -243,7 +243,7 @@ static struct afs_server *afs_alloc_server(struct afs_cell *cell,
server->rtt = UINT_MAX; server->rtt = UINT_MAX;
afs_inc_servers_outstanding(net); 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); _leave(" = %p", server);
return 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, struct afs_server *afs_get_server(struct afs_server *server,
enum afs_server_trace reason) enum afs_server_trace reason)
{ {
unsigned int a;
int r; int r;
__refcount_inc(&server->ref, &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; return server;
} }
@ -372,7 +374,7 @@ static struct afs_server *afs_maybe_use_server(struct afs_server *server,
return NULL; return NULL;
a = atomic_inc_return(&server->active); 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; 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); __refcount_inc(&server->ref, &r);
a = atomic_inc_return(&server->active); 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; 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, void afs_put_server(struct afs_net *net, struct afs_server *server,
enum afs_server_trace reason) enum afs_server_trace reason)
{ {
unsigned int a, debug_id = server->debug_id;
bool zero; bool zero;
int r; int r;
if (!server) if (!server)
return; return;
a = atomic_inc_return(&server->active);
zero = __refcount_dec_and_test(&server->ref, &r); 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)) if (unlikely(zero))
__afs_put_server(net, server); __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); 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); atomic_read(&server->active), afs_server_trace_free);
afs_put_addrlist(rcu_access_pointer(server->addresses)); afs_put_addrlist(rcu_access_pointer(server->addresses));
kfree(server); 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); active = atomic_read(&server->active);
if (active == 0) { 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); active, afs_server_trace_gc);
next = rcu_dereference_protected( next = rcu_dereference_protected(
server->uuid_next, lockdep_is_held(&net->fs_lock.lock)); 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); _debug("manage %pU %u", &server->uuid, active);
if (purging) { 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); active, afs_server_trace_purging);
if (active != 0) if (active != 0)
pr_notice("Can't purge s=%08x\n", server->debug_id); 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(""); _enter("");
trace_afs_server(server, refcount_read(&server->ref), trace_afs_server(server->debug_id, refcount_read(&server->ref),
atomic_read(&server->active), atomic_read(&server->active),
afs_server_trace_update); afs_server_trace_update);

12
include/trace/events/afs.h
View file

@ -727,10 +727,10 @@ TRACE_EVENT(afs_cb_call,
); );
TRACE_EVENT(afs_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), 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( TP_STRUCT__entry(
__field(unsigned int, call ) __field(unsigned int, call )
@ -741,7 +741,7 @@ TRACE_EVENT(afs_call,
), ),
TP_fast_assign( TP_fast_assign(
__entry->call = call->debug_id; __entry->call = call_debug_id;
__entry->op = op; __entry->op = op;
__entry->ref = ref; __entry->ref = ref;
__entry->outstanding = outstanding; __entry->outstanding = outstanding;
@ -1433,10 +1433,10 @@ TRACE_EVENT(afs_cb_miss,
); );
TRACE_EVENT(afs_server, 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), enum afs_server_trace reason),
TP_ARGS(server, ref, active, reason), TP_ARGS(server_debug_id, ref, active, reason),
TP_STRUCT__entry( TP_STRUCT__entry(
__field(unsigned int, server ) __field(unsigned int, server )
@ -1446,7 +1446,7 @@ TRACE_EVENT(afs_server,
), ),
TP_fast_assign( TP_fast_assign(
__entry->server = server->debug_id; __entry->server = server_debug_id;
__entry->ref = ref; __entry->ref = ref;
__entry->active = active; __entry->active = active;
__entry->reason = reason; __entry->reason = reason;