[PATCH 0/2] fuse: Increase FUSE_NAME_MAX limit

[PATCH 0/2] fuse: Increase FUSE_NAME_MAX limit

[Bernd Schubert]

First patch switches fuse_notify_inval_entry and fuse_notify_delete
to allocate name buffers to the actual file name size and not
FUSE_NAME_MAX anymore. 
Second patch increases the FUSE_NAME_MAX limit.

Signed-off-by: Bernd Schubert <bschubert@ddn.com>
---
Bernd Schubert (2):
      fuse: Allocate only namelen buf memory in fuse_notify_
      fuse: Increase FUSE_NAME_MAX to PATH_MAX

 fs/fuse/dev.c    | 26 ++++++++++++++------------
 fs/fuse/fuse_i.h |  2 +-
 2 files changed, 15 insertions(+), 13 deletions(-)
---
base-commit: f92f4749861b06fed908d336b4dee1326003291b
change-id: 20241212-fuse_name_max-limit-6-13-18582bd39dd4

Best regards,
-- 
Bernd Schubert <bschubert@ddn.com>

[PATCH 1/2] fuse: Allocate only namelen buf memory in fuse_notify_

[Bernd Schubert]

fuse_notify_inval_entry and fuse_notify_delete were using fixed allocations
of FUSE_NAME_MAX to hold the file name. Often that large buffers are not
needed as file names might be smaller, so this uses the actual file name
size to do the allocation.

Signed-off-by: Bernd Schubert <bschubert@ddn.com>
---
 fs/fuse/dev.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 27ccae63495d14ea339aa6c8da63d0ac44fc8885..c979ce93685f8338301a094ac513c607f44ba572 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -1525,14 +1525,10 @@ static int fuse_notify_inval_entry(struct fuse_conn *fc, unsigned int size,
 				   struct fuse_copy_state *cs)
 {
 	struct fuse_notify_inval_entry_out outarg;
-	int err = -ENOMEM;
-	char *buf;
+	int err;
+	char *buf = NULL;
 	struct qstr name;
 
-	buf = kzalloc(FUSE_NAME_MAX + 1, GFP_KERNEL);
-	if (!buf)
-		goto err;
-
 	err = -EINVAL;
 	if (size < sizeof(outarg))
 		goto err;
@@ -1549,6 +1545,11 @@ static int fuse_notify_inval_entry(struct fuse_conn *fc, unsigned int size,
 	if (size != sizeof(outarg) + outarg.namelen + 1)
 		goto err;
 
+	err = -ENOMEM;
+	buf = kzalloc(outarg.namelen + 1, GFP_KERNEL);
+	if (!buf)
+		goto err;
+
 	name.name = buf;
 	name.len = outarg.namelen;
 	err = fuse_copy_one(cs, buf, outarg.namelen + 1);
@@ -1573,14 +1574,10 @@ static int fuse_notify_delete(struct fuse_conn *fc, unsigned int size,
 			      struct fuse_copy_state *cs)
 {
 	struct fuse_notify_delete_out outarg;
-	int err = -ENOMEM;
-	char *buf;
+	int err;
+	char *buf = NULL;
 	struct qstr name;
 
-	buf = kzalloc(FUSE_NAME_MAX + 1, GFP_KERNEL);
-	if (!buf)
-		goto err;
-
 	err = -EINVAL;
 	if (size < sizeof(outarg))
 		goto err;
@@ -1597,6 +1594,11 @@ static int fuse_notify_delete(struct fuse_conn *fc, unsigned int size,
 	if (size != sizeof(outarg) + outarg.namelen + 1)
 		goto err;
 
+	err = -ENOMEM;
+	buf = kzalloc(outarg.namelen + 1, GFP_KERNEL);
+	if (!buf)
+		goto err;
+
 	name.name = buf;
 	name.len = outarg.namelen;
 	err = fuse_copy_one(cs, buf, outarg.namelen + 1);

-- 
2.43.0

Re: [PATCH 1/2] fuse: Allocate only namelen buf memory in fuse_notify_

[Jingbo Xu]

On 12/13/24 5:50 AM, Bernd Schubert wrote:
> fuse_notify_inval_entry and fuse_notify_delete were using fixed allocations
> of FUSE_NAME_MAX to hold the file name. Often that large buffers are not
> needed as file names might be smaller, so this uses the actual file name
> size to do the allocation.
> 
> Signed-off-by: Bernd Schubert <bschubert@ddn.com>

Reviewed-by: Jingbo Xu <jefflexu@linux.alibaba.com>


> ---
>  fs/fuse/dev.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 27ccae63495d14ea339aa6c8da63d0ac44fc8885..c979ce93685f8338301a094ac513c607f44ba572 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -1525,14 +1525,10 @@ static int fuse_notify_inval_entry(struct fuse_conn *fc, unsigned int size,
>  				   struct fuse_copy_state *cs)
>  {
>  	struct fuse_notify_inval_entry_out outarg;
> -	int err = -ENOMEM;
> -	char *buf;
> +	int err;
> +	char *buf = NULL;
>  	struct qstr name;
>  
> -	buf = kzalloc(FUSE_NAME_MAX + 1, GFP_KERNEL);
> -	if (!buf)
> -		goto err;
> -
>  	err = -EINVAL;
>  	if (size < sizeof(outarg))
>  		goto err;
> @@ -1549,6 +1545,11 @@ static int fuse_notify_inval_entry(struct fuse_conn *fc, unsigned int size,
>  	if (size != sizeof(outarg) + outarg.namelen + 1)
>  		goto err;
>  
> +	err = -ENOMEM;
> +	buf = kzalloc(outarg.namelen + 1, GFP_KERNEL);
> +	if (!buf)
> +		goto err;
> +
>  	name.name = buf;
>  	name.len = outarg.namelen;
>  	err = fuse_copy_one(cs, buf, outarg.namelen + 1);
> @@ -1573,14 +1574,10 @@ static int fuse_notify_delete(struct fuse_conn *fc, unsigned int size,
>  			      struct fuse_copy_state *cs)
>  {
>  	struct fuse_notify_delete_out outarg;
> -	int err = -ENOMEM;
> -	char *buf;
> +	int err;
> +	char *buf = NULL;
>  	struct qstr name;
>  
> -	buf = kzalloc(FUSE_NAME_MAX + 1, GFP_KERNEL);
> -	if (!buf)
> -		goto err;
> -
>  	err = -EINVAL;
>  	if (size < sizeof(outarg))
>  		goto err;
> @@ -1597,6 +1594,11 @@ static int fuse_notify_delete(struct fuse_conn *fc, unsigned int size,
>  	if (size != sizeof(outarg) + outarg.namelen + 1)
>  		goto err;
>  
> +	err = -ENOMEM;
> +	buf = kzalloc(outarg.namelen + 1, GFP_KERNEL);
> +	if (!buf)
> +		goto err;
> +
>  	name.name = buf;
>  	name.len = outarg.namelen;
>  	err = fuse_copy_one(cs, buf, outarg.namelen + 1);
> 

-- 
Thanks,
Jingbo

[PATCH 2/2] fuse: Increase FUSE_NAME_MAX to PATH_MAX

[Bernd Schubert]

Our file system has a translation capability for S3-to-posix.
The current value of 1kiB is enough to cover S3 keys, but
does not allow encoding of escape characters. The limit is
increased by factor 3 to allow worst case encoding with %xx.

Testing large file names was hard with libfuse/example file systems,
so I created a new memfs that does not have a 255 file name length
limitation.
https://github.com/libfuse/libfuse/pull/1077

Signed-off-by: Bernd Schubert <bschubert@ddn.com>
---
 fs/fuse/fuse_i.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 74744c6f286003251564d1235f4d2ca8654d661b..91b4cdd60fd4fe4ca5c3b8f2c9e5999c69d40690 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -39,7 +39,7 @@
 #define FUSE_NOWRITE INT_MIN
 
 /** It could be as large as PATH_MAX, but would that have any uses? */
-#define FUSE_NAME_MAX 1024
+#define FUSE_NAME_MAX (3 * 1024)
 
 /** Number of dentries for each connection in the control filesystem */
 #define FUSE_CTL_NUM_DENTRIES 5

-- 
2.43.0

Re: [PATCH 2/2] fuse: Increase FUSE_NAME_MAX to PATH_MAX

[Jingbo Xu]

On 12/13/24 5:50 AM, Bernd Schubert wrote:
> Our file system has a translation capability for S3-to-posix.
> The current value of 1kiB is enough to cover S3 keys, but
> does not allow encoding of escape characters. The limit is
> increased by factor 3 to allow worst case encoding with %xx.
> 
> Testing large file names was hard with libfuse/example file systems,
> so I created a new memfs that does not have a 255 file name length
> limitation.
> https://github.com/libfuse/libfuse/pull/1077
> 
> Signed-off-by: Bernd Schubert <bschubert@ddn.com>
> ---
>  fs/fuse/fuse_i.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 74744c6f286003251564d1235f4d2ca8654d661b..91b4cdd60fd4fe4ca5c3b8f2c9e5999c69d40690 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -39,7 +39,7 @@
>  #define FUSE_NOWRITE INT_MIN
>  
>  /** It could be as large as PATH_MAX, but would that have any uses? */
> -#define FUSE_NAME_MAX 1024
> +#define FUSE_NAME_MAX (3 * 1024)


So why not increase it directly to PATH_MAX, as indicated by the comment?

-- 
Thanks,
Jingbo

Re: [PATCH 2/2] fuse: Increase FUSE_NAME_MAX to PATH_MAX

[Shachar Sharon]

The <linux/limits.h> defines NAME_MAX as 255 (_not_ including nul) and
PATH_MAX as 4096 (including nul). It would be nice to keep this
convention also on the FUSE side; that is, define FUSE_NAME_MAX as
1023, or in your case (3 * 1024 - 1). I think this is the also
intention of the code in fuse_notify_inval_entry:

  err = -ENAMETOOLONG
  if (outarg.namelen > FUSE_NAME_MAX)
          goto err;

Otherwise, we should fix this check as well (outarg.namelen >=
FUSE_NAME_MAX). That said, keep in mind that using dir-entry names
larger then NAME_MAX would also cause ENAMETOOLONG by glibc’s
readdir[1]

- Shachar.

[1] https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/readdir_r.c#L52-L59


On Thu, Dec 12, 2024 at 11:51 PM Bernd Schubert <bschubert@ddn.com> wrote:
>
> Our file system has a translation capability for S3-to-posix.
> The current value of 1kiB is enough to cover S3 keys, but
> does not allow encoding of escape characters. The limit is
> increased by factor 3 to allow worst case encoding with %xx.
>
> Testing large file names was hard with libfuse/example file systems,
> so I created a new memfs that does not have a 255 file name length
> limitation.
> https://github.com/libfuse/libfuse/pull/1077
>
> Signed-off-by: Bernd Schubert <bschubert@ddn.com>
> ---
>  fs/fuse/fuse_i.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 74744c6f286003251564d1235f4d2ca8654d661b..91b4cdd60fd4fe4ca5c3b8f2c9e5999c69d40690 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -39,7 +39,7 @@
>  #define FUSE_NOWRITE INT_MIN
>
>  /** It could be as large as PATH_MAX, but would that have any uses? */
> -#define FUSE_NAME_MAX 1024
> +#define FUSE_NAME_MAX (3 * 1024)
>
>  /** Number of dentries for each connection in the control filesystem */
>  #define FUSE_CTL_NUM_DENTRIES 5
>
> --
> 2.43.0
>
>

Re: [PATCH 2/2] fuse: Increase FUSE_NAME_MAX to PATH_MAX

[Bernd Schubert]

[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]

On 12/13/24 10:17, Shachar Sharon wrote:
> [You don't often get email from synarete@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> The <linux/limits.h> defines NAME_MAX as 255 (_not_ including nul) and
> PATH_MAX as 4096 (including nul). It would be nice to keep this
> convention also on the FUSE side; that is, define FUSE_NAME_MAX as
> 1023, or in your case (3 * 1024 - 1). I think this is the also
> intention of the code in fuse_notify_inval_entry:
> 
>   err = -ENAMETOOLONG
>   if (outarg.namelen > FUSE_NAME_MAX)
>           goto err;

Thanks for the review, I can change it to (3 * 1024 - 1) or 
(PATH_MAX - 1).

> 
> Otherwise, we should fix this check as well (outarg.namelen >=
> FUSE_NAME_MAX). That said, keep in mind that using dir-entry names
> larger then NAME_MAX would also cause ENAMETOOLONG by glibc’s
> readdir[1]
> 
> - Shachar.
> 
> [1] https://github.com/bminor/glibc/blob/master/sysdeps/unix/sysv/linux/readdir_r.c#L52-L59

Yes, I had seen that glibc uses NAME_MAX, but I had also tested the
patch using the new memfs [1] and the attached script. You can try
it out yourself - listing long file names actually works (on a
Debian 12 system).



Thanks,
Bernd

[1] https://github.com/libfuse/libfuse/blob/master/example/memfs_ll.cc

[-- Attachment #2: long-file-name.sh --]
[-- Type: application/x-shellscript, Size: 424 bytes --]