[PATCH] Using a larger memory to cache filename when decoding FH

[PATCH] Using a larger memory to cache filename when decoding FH

[Pei-Feng liu]

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

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

[Pei-Feng liu]

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 <liupeifeng@horiscale.com>
---
 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

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

[J. Bruce Fields]

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