On 8/19/22 10:37 AM, Olga Kornievskaia wrote:
On Fri, Aug 19, 2022 at 11:42 AM <dai.ngo@xxxxxxxxxx> wrote:
On 8/19/22 7:22 AM, Olga Kornievskaia wrote:
On Thu, Aug 18, 2022 at 10:52 PM <dai.ngo@xxxxxxxxxx> wrote:
On 8/18/22 6:13 AM, Olga Kornievskaia wrote:
On Thu, Aug 18, 2022 at 1:52 AM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
On Thu, Aug 18, 2022 at 08:19:54AM +0300, Amir Goldstein wrote:
NFS spec does not guarantee the safety of the server.
It's like saying that the Law makes Crime impossible.
The law needs to be enforced, so if server gets a request
to COPY from/to an fhandle that resolves as a non-regular file
(from a rogue or buggy NFS client) the server should return an
error and not continue to alloc_file_pseudo().
FWIW, my preference would be to have alloc_file_pseudo() reject
directory inodes if it ever gets such.
I'm still not sure that my (and yours, apparently) interpretation
of what Olga said is correct, though.
Would it be appropriate to do the following then:
diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
index e88f6b18445e..112134b6438d 100644
--- a/fs/nfs/nfs4file.c
+++ b/fs/nfs/nfs4file.c
@@ -340,6 +340,11 @@ static struct file *__nfs42_ssc_open(struct
vfsmount *ss_mnt,
goto out;
}
+ if (S_ISDIR(fattr->mode)) {
+ res = ERR_PTR(-EBADF);
+ goto out;
+ }
+
Can we also enhance nfsd4_do_async_copy to check for
-EBADF and returns nfserr_wrong_type? perhaps adding
an error mapping function to handle other errors also.
On the server side, if the open fails that's already translated into
the appropriate error -- err_off_load_denied.
Currently the server returns nfserr_offload_denied if the open
fails for any reasons. I'm wondering whether the server should
return more accurate error code such as if the source file handle
is a wrong type then the server should return nfserr_wrong_type,
instead of nfserr_offload_denied, to match the spec:
Both SAVED_FH and CURRENT_FH must be regular files. If either
SAVED_FH or CURRENT_FH is not a regular file, the operation MUST fail
and return NFS4ERR_WRONG_TYPE.
Ok sure. That's a relevant but a separate patch.
Thank you Olga!
-Dai
-Dai
-Dai
res = ERR_PTR(-ENOMEM);
len = strlen(SSC_READ_NAME_BODY) + 16;
read_name = kzalloc(len, GFP_KERNEL);
@@ -357,6 +362,7 @@ static struct file *__nfs42_ssc_open(struct
vfsmount *ss_mnt,
r_ino->i_fop);
if (IS_ERR(filep)) {
res = ERR_CAST(filep);
+ iput(r_ino);
goto out_free_name;
}
