[RFC][PATCH] nfsd: add +1 to reference counting scheme for struct nfsd4_session

[David Windsor]

In furtherance of the KSPP effort to add overflow protection to kernel
reference counters, a new type (refcount_t) and API have been created.
Part of the refcount_t API is refcount_inc(), which will not increment a
refcount_t variable if its value is 0 (as this would indicate a possible
use-after-free condition). 

In auditing the kernel for refcounting corner cases, we've come across the
case of struct nfsd4_session.  

>From fs/nfsd/state.h:

/*
 * Representation of a v4.1+ session. These are refcounted in a similar 
 * fashion to the nfs4_client. References are only taken when the server
 * is actively working on the object (primarily during the processing of
 * compounds).
 */
struct nfsd4_session {
    atomic_t se_ref;
    ...
};


>From fs/nfsd/nfs4state.c:

static void init_session(..., struct nfsd4_session *new, ...)
{
    ...
    atomic_set(&new->se_ref, 0);
    ...
}
 
Since nfsd4_session objects are initialized with refcount = 0, subsequent
increments will fail using the new refcount_t API.

Being largely unfamiliar with this subsystem's garbage collection
mechanism, I'm unsure how to best fix this.  Attached is a patch that
performs a logical +1 on struct nfsd4_session's reference counting
scheme.

If this is the correct route to take, I will resubmit this patch with
updated comments for how struct nfsd4_session is refcounted (see the above
comment from fs/nsfd/state.h).  This is in preparation for the previously
mentioned refcount_t API series.

Thanks,
David Windsor

Signed-off-by: David Windsor <dwind...@gmail.com>
---
 fs/nfsd/nfs4state.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a0dee8a..b0f3010 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -196,7 +196,7 @@ static void nfsd4_put_session_locked(struct nfsd4_session 
*ses)
 
        lockdep_assert_held(&nn->client_lock);
 
-       if (atomic_dec_and_test(&ses->se_ref) && is_session_dead(ses))
+       if (!atomic_add_unless(&ses->se_ref, -1, 1) && is_session_des(ses))
                free_session(ses);
        put_client_renew_locked(clp);
 }
@@ -1645,7 +1645,7 @@ static void init_session(struct svc_rqst *rqstp, struct 
nfsd4_session *new, stru
        new->se_flags = cses->flags;
        new->se_cb_prog = cses->callback_prog;
        new->se_cb_sec = cses->cb_sec;
-       atomic_set(&new->se_ref, 0);
+       atomic_set(&new->se_ref, 1);
        idx = hash_sessionid(&new->se_sessionid);
        list_add(&new->se_hash, &nn->sessionid_hashtbl[idx]);
        spin_lock(&clp->cl_lock);
@@ -1792,7 +1792,7 @@ free_client(struct nfs4_client *clp)
                ses = list_entry(clp->cl_sessions.next, struct nfsd4_session,
                                se_perclnt);
                list_del(&ses->se_perclnt);
-               WARN_ON_ONCE(atomic_read(&ses->se_ref));
+               WARN_ON_ONCE((atomic_read(&ses->se_ref) > 1));
                free_session(ses);
        }
        rpc_destroy_wait_queue(&clp->cl_cb_waitq);
-- 
2.7.4