On 2010-11-12 10:13, William A. (Andy) Adamson wrote: > 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. Seems bad enough to fix :) Benny > > 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
