From: J. Bruce Fields <[email protected]>
commit bf7bd3e98be5c74813bee6ad496139fb0a011b3b upstream.
This fixes a regression from 68a3396178e6688ad7367202cdf0af8ed03c8727
"nfsd4: shut down more of delegation earlier".
After that commit, nfs4_set_delegation() failures result in
nfs4_put_delegation being called, but nfs4_put_delegation doesn't free
the nfs4_file that has already been set by alloc_init_deleg().
This can result in an oops on later unmounting the exported filesystem.
Note also delaying the fi_had_conflict check we're able to return a
better error (hence give 4.1 clients a better idea why the delegation
failed; though note CONFLICT isn't an exact match here, as that's
supposed to indicate a current conflict, but all we know here is that
there was one recently).
Reported-by: Toralf F?rster <[email protected]>
Tested-by: Toralf F?rster <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
[tuomasjjrasanen: backported to 3.10
Conflicts fs/nfsd/nfs4state.c:
Delegation type flags have been removed from upstream code. In 3.10-series,
they still exists and therefore the commit caused few conflicts in function
signatures.
]
Signed-off-by: Tuomas R?s?nen <[email protected]>
---
fs/nfsd/nfs4state.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index bdff771..836307a 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -367,7 +367,6 @@ static struct nfs4_delegation *
alloc_init_deleg(struct nfs4_client *clp, struct nfs4_ol_stateid *stp, struct svc_fh *current_fh, u32 type)
{
struct nfs4_delegation *dp;
- struct nfs4_file *fp = stp->st_file;
dprintk("NFSD alloc_init_deleg\n");
/*
@@ -377,8 +376,6 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_ol_stateid *stp, struct sv
*/
if (type != NFS4_OPEN_DELEGATE_READ)
return NULL;
- if (fp->fi_had_conflict)
- return NULL;
if (num_delegations > max_delegations)
return NULL;
dp = delegstateid(nfs4_alloc_stid(clp, deleg_slab));
@@ -395,8 +392,7 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_ol_stateid *stp, struct sv
INIT_LIST_HEAD(&dp->dl_perfile);
INIT_LIST_HEAD(&dp->dl_perclnt);
INIT_LIST_HEAD(&dp->dl_recall_lru);
- get_nfs4_file(fp);
- dp->dl_file = fp;
+ dp->dl_file = NULL;
dp->dl_type = type;
fh_copy_shallow(&dp->dl_fh, ¤t_fh->fh_handle);
dp->dl_time = 0;
@@ -2965,22 +2961,35 @@ static int nfs4_setlease(struct nfs4_delegation *dp, int flag)
return 0;
}
-static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
+static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag, struct nfs4_file *fp)
{
- struct nfs4_file *fp = dp->dl_file;
+ int status;
- if (!fp->fi_lease)
- return nfs4_setlease(dp, flag);
+ if (fp->fi_had_conflict)
+ return -EAGAIN;
+ get_nfs4_file(fp);
+ dp->dl_file = fp;
+ if (!fp->fi_lease) {
+ status = nfs4_setlease(dp, flag);
+ if (status)
+ goto out_free;
+ return 0;
+ }
spin_lock(&recall_lock);
if (fp->fi_had_conflict) {
spin_unlock(&recall_lock);
- return -EAGAIN;
+ status = -EAGAIN;
+ goto out_free;
}
atomic_inc(&fp->fi_delegees);
list_add(&dp->dl_perfile, &fp->fi_delegations);
spin_unlock(&recall_lock);
list_add(&dp->dl_perclnt, &dp->dl_stid.sc_client->cl_delegations);
return 0;
+out_free:
+ put_nfs4_file(fp);
+ dp->dl_file = fp;
+ return status;
}
static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
@@ -3046,7 +3055,7 @@ nfs4_open_delegation(struct net *net, struct svc_fh *fh,
dp = alloc_init_deleg(oo->oo_owner.so_client, stp, fh, flag);
if (dp == NULL)
goto out_no_deleg;
- status = nfs4_set_delegation(dp, flag);
+ status = nfs4_set_delegation(dp, flag, stp->st_file);
if (status)
goto out_free;
--
2.1.3
Acked-by: J. Bruce Fields <[email protected]>
On Tue, Nov 25, 2014 at 11:05:21AM +0200, =?UTF-8?q?Tuomas=20R=C3=A4s=C3=A4nen?= wrote:
> From: J. Bruce Fields <[email protected]>
>
> commit bf7bd3e98be5c74813bee6ad496139fb0a011b3b upstream.
>
> This fixes a regression from 68a3396178e6688ad7367202cdf0af8ed03c8727
> "nfsd4: shut down more of delegation earlier".
>
> After that commit, nfs4_set_delegation() failures result in
> nfs4_put_delegation being called, but nfs4_put_delegation doesn't free
> the nfs4_file that has already been set by alloc_init_deleg().
>
> This can result in an oops on later unmounting the exported filesystem.
>
> Note also delaying the fi_had_conflict check we're able to return a
> better error (hence give 4.1 clients a better idea why the delegation
> failed; though note CONFLICT isn't an exact match here, as that's
> supposed to indicate a current conflict, but all we know here is that
> there was one recently).
>
> Reported-by: Toralf F?rster <[email protected]>
> Tested-by: Toralf F?rster <[email protected]>
> Signed-off-by: J. Bruce Fields <[email protected]>
> [tuomasjjrasanen: backported to 3.10
> Conflicts fs/nfsd/nfs4state.c:
> Delegation type flags have been removed from upstream code. In 3.10-series,
> they still exists and therefore the commit caused few conflicts in function
> signatures.
> ]
> Signed-off-by: Tuomas R?s?nen <[email protected]>
> ---
> fs/nfsd/nfs4state.c | 31 ++++++++++++++++++++-----------
> 1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index bdff771..836307a 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -367,7 +367,6 @@ static struct nfs4_delegation *
> alloc_init_deleg(struct nfs4_client *clp, struct nfs4_ol_stateid *stp, struct svc_fh *current_fh, u32 type)
> {
> struct nfs4_delegation *dp;
> - struct nfs4_file *fp = stp->st_file;
>
> dprintk("NFSD alloc_init_deleg\n");
> /*
> @@ -377,8 +376,6 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_ol_stateid *stp, struct sv
> */
> if (type != NFS4_OPEN_DELEGATE_READ)
> return NULL;
> - if (fp->fi_had_conflict)
> - return NULL;
> if (num_delegations > max_delegations)
> return NULL;
> dp = delegstateid(nfs4_alloc_stid(clp, deleg_slab));
> @@ -395,8 +392,7 @@ alloc_init_deleg(struct nfs4_client *clp, struct nfs4_ol_stateid *stp, struct sv
> INIT_LIST_HEAD(&dp->dl_perfile);
> INIT_LIST_HEAD(&dp->dl_perclnt);
> INIT_LIST_HEAD(&dp->dl_recall_lru);
> - get_nfs4_file(fp);
> - dp->dl_file = fp;
> + dp->dl_file = NULL;
> dp->dl_type = type;
> fh_copy_shallow(&dp->dl_fh, ¤t_fh->fh_handle);
> dp->dl_time = 0;
> @@ -2965,22 +2961,35 @@ static int nfs4_setlease(struct nfs4_delegation *dp, int flag)
> return 0;
> }
>
> -static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag)
> +static int nfs4_set_delegation(struct nfs4_delegation *dp, int flag, struct nfs4_file *fp)
> {
> - struct nfs4_file *fp = dp->dl_file;
> + int status;
>
> - if (!fp->fi_lease)
> - return nfs4_setlease(dp, flag);
> + if (fp->fi_had_conflict)
> + return -EAGAIN;
> + get_nfs4_file(fp);
> + dp->dl_file = fp;
> + if (!fp->fi_lease) {
> + status = nfs4_setlease(dp, flag);
> + if (status)
> + goto out_free;
> + return 0;
> + }
> spin_lock(&recall_lock);
> if (fp->fi_had_conflict) {
> spin_unlock(&recall_lock);
> - return -EAGAIN;
> + status = -EAGAIN;
> + goto out_free;
> }
> atomic_inc(&fp->fi_delegees);
> list_add(&dp->dl_perfile, &fp->fi_delegations);
> spin_unlock(&recall_lock);
> list_add(&dp->dl_perclnt, &dp->dl_stid.sc_client->cl_delegations);
> return 0;
> +out_free:
> + put_nfs4_file(fp);
> + dp->dl_file = fp;
> + return status;
> }
>
> static void nfsd4_open_deleg_none_ext(struct nfsd4_open *open, int status)
> @@ -3046,7 +3055,7 @@ nfs4_open_delegation(struct net *net, struct svc_fh *fh,
> dp = alloc_init_deleg(oo->oo_owner.so_client, stp, fh, flag);
> if (dp == NULL)
> goto out_no_deleg;
> - status = nfs4_set_delegation(dp, flag);
> + status = nfs4_set_delegation(dp, flag, stp->st_file);
> if (status)
> goto out_free;
>
> --
> 2.1.3
>