2019-03-23 13:46:51

by Pei-Feng liu

[permalink] [raw]
Subject: [PATCH] Using a larger memory to cache filename when decoding FH

The maximum length of filename string in 'fs/exportfs/expfs.c' is limited to be
'NAME_MAX'. But I have a special filesystem driver with longer filename. When
exporting it's namespace with NFS, 'exportfs_decode_fh()' can't holds the
filename if its length is larger than 'NAME_MAX'. And 'dentry' in VFS layer
doesn't contrain the length of filename within 'NAME_MAX'. I guess the correct
method is firstly probe the length of filename with 'vfs_statfs()'. With this
value to malloc a larger enough memory to cache the filename when decoding a FH.

Pei-Feng liu (1):
fs/exportfs: Decoding FH with a larger memory to cache the filename.

fs/exportfs/expfs.c | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)

--
1.8.3.1



2019-03-23 13:46:56

by Pei-Feng liu

[permalink] [raw]
Subject: [PATCH] fs/exportfs: Decoding FH with a larger memory to cache the filename.

It uses 'NAME_MAX+1' memory to cache a filename string when decoding a FH
to a connected 'dentry' object. As a matter of fact, the correct method
is using 'f_namelen' that returned by 'vfs_statfs()'.

More details: I have a proprietary filesystem driver, which looks like
XFS or EXT4 and is going to be exported its namespace with a special
NFS/CIFS server. The filename's maxlen (in bytes) of it is larger than
'NAME_MAX'. And 'dentry' object doesn't contraint that the name string
within 'NAME_MAX'. So, this piece of code in 'exportfs_decode_fh' in
'fs/exportfs/expfs.c' doesn't work when the exported namespace is from
my proprietary filesystem.

This patch is filed to get this resolved. The max length of a filesystem
is probed firstly with 'vfs_statfs()', then malloc a large enough memory
to cache the filename string when decoding a 'FH'.

Signed-off-by: Pei-Feng liu <[email protected]>
---
fs/exportfs/expfs.c | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/fs/exportfs/expfs.c b/fs/exportfs/expfs.c
index c69927be..8738868 100644
--- a/fs/exportfs/expfs.c
+++ b/fs/exportfs/expfs.c
@@ -16,6 +16,8 @@
#include <linux/namei.h>
#include <linux/sched.h>
#include <linux/cred.h>
+#include <linux/slab.h>
+#include <linux/statfs.h>

#define dprintk(fmt, args...) do{}while(0)

@@ -422,7 +424,7 @@ struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
{
const struct export_operations *nop = mnt->mnt_sb->s_export_op;
struct dentry *result, *alias;
- char nbuf[NAME_MAX+1];
+ int nbuf_max = NAME_MAX+1;
int err;

/*
@@ -445,6 +447,15 @@ struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
if (!acceptable)
return result;

+ if (NULL!=mnt->mnt_sb->s_op && NULL!=mnt->mnt_sb->s_op->statfs) {
+ struct kstatfs stfs;
+
+ if ((0 == mnt->mnt_sb->s_op->statfs(mnt->mnt_root, &stfs)) && \
+ (stfs.f_namelen > 0) && (stfs.f_namelen < INT_MAX)) {
+ nbuf_max = (uint32_t)stfs.f_namelen + 1;
+ }
+ }
+
if (d_is_dir(result)) {
/*
* This request is for a directory.
@@ -455,7 +466,14 @@ struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
* filesystem root.
*/
if (result->d_flags & DCACHE_DISCONNECTED) {
+ char *nbuf = (char *)kzalloc(nbuf_max, GFP_KERNEL);
+ if (NULL == nbuf) {
+ err = -ENOMEM;
+ goto err_result;
+ }
+
err = reconnect_path(mnt, result, nbuf);
+ kfree(nbuf);
if (err)
goto err_result;
}
@@ -471,6 +489,7 @@ struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
* It's not a directory. Life is a little more complicated.
*/
struct dentry *target_dir, *nresult;
+ char *nbuf = NULL;

/*
* See if either the dentry we just got from the filesystem
@@ -501,6 +520,12 @@ struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
if (IS_ERR(target_dir))
goto err_result;

+ nbuf = (char *)kzalloc(nbuf_max, GFP_KERNEL);
+ if (NULL == nbuf) {
+ err = -ENOMEM;
+ goto err_result;
+ }
+
/*
* And as usual we need to make sure the parent directory is
* connected to the filesystem root. The VFS really doesn't
@@ -509,6 +534,7 @@ struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
err = reconnect_path(mnt, target_dir, nbuf);
if (err) {
dput(target_dir);
+ kfree(nbuf);
goto err_result;
}

@@ -532,6 +558,8 @@ struct dentry *exportfs_decode_fh(struct vfsmount *mnt, struct fid *fid,
}
}

+ kfree(nbuf);
+
/*
* At this point we are done with the parent, but it's pinned
* by the child dentry anyway.
--
1.8.3.1


2019-03-25 23:21:14

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH] Using a larger memory to cache filename when decoding FH

On Sat, Mar 23, 2019 at 09:46:33PM +0800, Pei-Feng liu wrote:
> The maximum length of filename string in 'fs/exportfs/expfs.c' is limited to be
> 'NAME_MAX'. But I have a special filesystem driver with longer filename.

That's interesting. I'm sort of surprised that other parts of the
kernel or libc don't rely on the NAME_MAX assumption.

Anyway, we probably can't consider this for upstream unless the
filesystem driver that needs it is also upstream.

--b.

> When
> exporting it's namespace with NFS, 'exportfs_decode_fh()' can't holds the
> filename if its length is larger than 'NAME_MAX'. And 'dentry' in VFS layer
> doesn't contrain the length of filename within 'NAME_MAX'. I guess the correct
> method is firstly probe the length of filename with 'vfs_statfs()'. With this
> value to malloc a larger enough memory to cache the filename when decoding a FH.
>
> Pei-Feng liu (1):
> fs/exportfs: Decoding FH with a larger memory to cache the filename.
>
> fs/exportfs/expfs.c | 30 +++++++++++++++++++++++++++++-
> 1 file changed, 29 insertions(+), 1 deletion(-)
>
> --
> 1.8.3.1