On Thu, Nov 11, 2010 at 8:22 AM, Benny Halevy <bhalevy@xxxxxxxxxxx> wrote: > On 2010-11-11 16:10, andros@xxxxxxxxxx wrote: >> From: Andy Adamson <andros@xxxxxxxxxx> >> >> Guarantee that the nfs_client exists when referenced to by callback processing >> by not procssing callbacks on an nfs_client in the process of being freed. >> >> Signed-off-by: Andy Adamson <andros@xxxxxxxxxx> >> --- >> fs/nfs/client.c | 8 ++++++-- >> 1 files changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/fs/nfs/client.c b/fs/nfs/client.c >> index dbf43e7..86657ee 100644 >> --- a/fs/nfs/client.c >> +++ b/fs/nfs/client.c >> @@ -392,7 +392,9 @@ struct nfs_client *nfs_find_client(const struct sockaddr *addr, u32 nfsversion) >> if (!nfs_sockaddr_match_ipaddr(addr, clap)) >> continue; >> >> - atomic_inc(&clp->cl_count); >> + /* Don't return an nfs_client that is being freed */ >> + if (!atomic_inc_not_zero(&clp->cl_count)) >> + continue; >> spin_unlock(&nfs_client_lock); >> return clp; >> } >> @@ -425,7 +427,9 @@ struct nfs_client *nfs_find_client_next(struct nfs_client *clp) >> if (!nfs_sockaddr_match_ipaddr(sap, clap)) >> continue; >> >> - atomic_inc(&clp->cl_count); >> + /* Don't return an nfs_client that is being freed */ >> + if (!atomic_inc_not_zero(&clp->cl_count)) >> + continue; >> spin_unlock(&nfs_client_lock); >> return clp; >> } > > Hmm, nfs_put_client deletes the client when cl_count reaches zero > so how can cl_count be zero while clp is listed? For some reason, I missed the lock part of atomic_dec_and_lock in nfs_put_client which removes the nfs_client from the list under the lock. We don't need this patch. What's weird about the back channel server processing is that the RPC layer pg_authenticate (nfs_callback_authenticate) call in svc_process_common finds an nfs_client struct based solely the callback client address and so may find the wrong nfs_client struct (nfsv4.0 instead of v4.1, or wrong session). So the nfs_client has to be put at the end of pg_authenticate and another nfs_find_client call is needed in the dispatcher routines after decoding. This means the callback server could start processing a callback and have the nfs_client struct freed between the pg_authenticate call and the dispatcher operation call, or it could have found the wrong nfs_client in the first place. If the nfs_client is not found in pg_authenticate, the request is simply dropped (SVC_DROP). But if an nfs_client is not found in the dispatcher routines NFS4ERR_BADSESSION is returned for v4.1 requests and NFS4ERR_BADHANDLE for v4.0 requests. I guess there's not much we can do about this. -->Andy > Benny > > In put_nfs_client > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
