Re: [PATCH] NFSD: do not return nfserr_symlink for the LINK operation

[Yang Hongyang <yanghy@cn.fujitsu.com>]

Benny Halevy wrote:
> On Mar. 19, 2009, 8:59 +0200, Yang Hongyang <yanghy@cn.fujitsu.com> wrote:
>> Benny Halevy wrote:
>>> As reported by Ni Wenjuan <niwj@cn.fujitsu.com> on 2009-03-05:
>>>> the result of newpynfs test case of  LINK4a .  if you link with target directoty
>>>> is a symbole file,it should get NFS4ERR_NOTDIR ,instead got NFS4ERR_SYMLINK.
>>> THE LINK operation doesn't list NFS4ERR_SYMLINK as a valid error in the spec.
>>> Although NFS4ERR_SYMLINK seems like a reasonable error and even though
>>> it was added for LINK in NFSv4.1 we should still return NFSERR_NOTDIR for
>>> nfsv[234].
>> HI,Benny:
>> There are other places that have the same problem,i'm preparing a all-in-one
>> patch.^!^
> 
> Cool.
> Bruce, please ignore this patch then.
> 
> Thanks!

There are four placees that returned inappropriate err nfserr_symlink accroding to 
newpynfs test #LINK4a#LOOKP2a#OPCF3a#SATT12a.nfserr_symlink do not listed
in these operations's err list in the spec.
For LINK and LOOKUPP operation,nfserr_notdir should be returned.
For OPEN_CONFIRM and SETATTR operation,nfserr_inval should be returned.

Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>

---
 fs/nfsd/nfs4proc.c  |    6 +++++-
 fs/nfsd/nfs4state.c |    6 +++++-
 fs/nfsd/vfs.c       |    6 ++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 9fa60a3..d25e7cf 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -493,8 +493,12 @@ nfsd4_lookupp(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		return nfserr_noent;
 	}
 	fh_put(&tmp_fh);
-	return nfsd_lookup(rqstp, &cstate->current_fh,
+	ret = nfsd_lookup(rqstp, &cstate->current_fh,
 			   "..", 2, &cstate->current_fh);
+	/* nfserr_symlink returned is inappropriate for LOOKUPP*/
+	if (ret == nfserr_symlink)
+		ret = nfserr_notdir;
+	return ret;
 }
 
 static __be32
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b6f60f4..fe274fc 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2234,8 +2234,12 @@ nfsd4_open_confirm(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 			cstate->current_fh.fh_dentry->d_name.name);
 
 	status = fh_verify(rqstp, &cstate->current_fh, S_IFREG, 0);
-	if (status)
+	if (status) {
+		/* nfserr_symlink returned is inappropriate for OPEN_CONFIRM*/
+		if (status == nfserr_symlink)
+			status = nfserr_inval;
 		return status;
+	}
 
 	nfs4_lock_state();
 
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 6e50aaa..69eba75 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -397,6 +397,9 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap,
 		if (EX_ISSYNC(fhp->fh_export))
 			write_inode_now(inode, 1);
 out:
+	/* nfserr_symlink returned is inappropriate for SETATTR*/
+	if (err == nfserr_symlink)
+		err = nfserr_inval;
 	return err;
 
 out_nfserr:
@@ -1637,6 +1640,9 @@ out_dput:
 out_unlock:
 	fh_unlock(ffhp);
 out:
+	/* nfserr_symlink returned is inappropriate for LINK*/
+	if (err == nfserr_symlink)
+		err = nfserr_notdir;
 	return err;
 
 out_nfserr:
-- 
1.6.0.3


> 
> Benny
> 
>>> Signed-off-by: Benny Halevy <bhalevy@panasas.com>
>>> ---
>>>  fs/nfsd/vfs.c |    3 +++
>>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
>>> index 6e50aaa..9165b1f 100644
>>> --- a/fs/nfsd/vfs.c
>>> +++ b/fs/nfsd/vfs.c
>>> @@ -1637,6 +1637,9 @@ out_dput:
>>>  out_unlock:
>>>  	fh_unlock(ffhp);
>>>  out:
>>> +	/* nfserr_symlink returned from fh_verify is inappropriate for LINK */
>>> +	if (err == nfserr_symlink)
>>> +		err = nfserr_notdir;
>>>  	return err;
>>>  
>>>  out_nfserr:
>>>
>>>
>>
> 
> 


-- 
Regards
Yang Hongyang