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: 341f741f04be ("afs: Refcount the afs_call struct")
Fixes: 452181936931 ("afs: Trace afs_server usage")
Fixes: 977e5f8ed0ab ("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/

diff --git a/fs/afs/cmservice.c b/fs/afs/cmservice.c
index cedd627e1fae032e009276b38d755f4ea61fe55a..0a090d614e76280beaae7664fcb6f4a223192164 100644 (file)
--- a/fs/afs/cmservice.c
+++ b/fs/afs/cmservice.c
@@ -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);

diff --git a/fs/afs/rxrpc.c b/fs/afs/rxrpc.c
index d9acc43cb6f0001de5c2931c8fdba37f29c3d534..d5c4785c862d994835a455d9cfc94a55ce38518f 100644 (file)
--- a/fs/afs/rxrpc.c
+++ b/fs/afs/rxrpc.c
@@ -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));
 

diff --git a/fs/afs/server.c b/fs/afs/server.c
index ffed828622b603fb73752ed0600e46aafd76db24..4981baf97835cb870a6abfb854b7d9a9122e4eba 100644 (file)
--- a/fs/afs/server.c
+++ b/fs/afs/server.c
@@ -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);
 

diff --git a/include/trace/events/afs.h b/include/trace/events/afs.h
index aa60f42a9763a0fc35e9c3b6eb2a5bde2bb2a70d..e9d412d19dbbbcf2bd0e080fd7e0d60f529681fb 100644 (file)
--- a/include/trace/events/afs.h
+++ b/include/trace/events/afs.h
@@ -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;