diff mbox series

[v3,2/2] fuse: Increase FUSE_NAME_MAX to PATH_MAX

Message ID 20241216-fuse_name_max-limit-6-13-v3-2-b4b04966ecea@ddn.com (mailing list archive)
State New
Headers show
Series fuse: Increase FUSE_NAME_MAX limit | expand

Commit Message

Bernd Schubert Dec. 16, 2024, 9:14 p.m. UTC
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 %xx escape characters.
The limit is increased to (PATH_MAX - 1), as we need
3 x 1024 and that is close to PATH_MAX (4kB) already.
-1 is used as the terminating null is not included in the
length calculation.

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

The connection is initialized with FUSE_NAME_LOW_MAX, which
is set to the previous value of FUSE_NAME_MAX of 1024. With
FUSE_MIN_READ_BUFFER of 8192 that is enough for two file names
+ fuse headers.
When FUSE_INIT reply sets max_pages to a value > 1 we know
that fuse daemon supports request buffers of at least 2 pages
(+ header) and can therefore hold 2 x PATH_MAX file names - operations
like rename or link that need two file names are no issue then.

Signed-off-by: Bernd Schubert <bschubert@ddn.com>
---
 fs/fuse/dev.c    |  4 ++--
 fs/fuse/dir.c    |  2 +-
 fs/fuse/fuse_i.h | 11 +++++++++--
 fs/fuse/inode.c  |  8 ++++++++
 4 files changed, 20 insertions(+), 5 deletions(-)

Comments

Shachar Sharon Dec. 18, 2024, 9:15 a.m. UTC | #1
On Mon, Dec 16, 2024 at 11:14 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 %xx escape characters.
> The limit is increased to (PATH_MAX - 1), as we need
> 3 x 1024 and that is close to PATH_MAX (4kB) already.
> -1 is used as the terminating null is not included in the
> length calculation.
>
> 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
>
> The connection is initialized with FUSE_NAME_LOW_MAX, which
> is set to the previous value of FUSE_NAME_MAX of 1024. With
> FUSE_MIN_READ_BUFFER of 8192 that is enough for two file names
> + fuse headers.
> When FUSE_INIT reply sets max_pages to a value > 1 we know
> that fuse daemon supports request buffers of at least 2 pages
> (+ header) and can therefore hold 2 x PATH_MAX file names - operations
> like rename or link that need two file names are no issue then.
>
> Signed-off-by: Bernd Schubert <bschubert@ddn.com>
> ---
>  fs/fuse/dev.c    |  4 ++--
>  fs/fuse/dir.c    |  2 +-
>  fs/fuse/fuse_i.h | 11 +++++++++--
>  fs/fuse/inode.c  |  8 ++++++++
>  4 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index c979ce93685f8338301a094ac513c607f44ba572..3b4bdff84e534be8b1ce4a970e841b6a362ef176 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -1538,7 +1538,7 @@ static int fuse_notify_inval_entry(struct fuse_conn *fc, unsigned int size,
>                 goto err;
>
>         err = -ENAMETOOLONG;
> -       if (outarg.namelen > FUSE_NAME_MAX)
> +       if (outarg.namelen > fc->name_max)
>                 goto err;
>
>         err = -EINVAL;
> @@ -1587,7 +1587,7 @@ static int fuse_notify_delete(struct fuse_conn *fc, unsigned int size,
>                 goto err;
>
>         err = -ENAMETOOLONG;
> -       if (outarg.namelen > FUSE_NAME_MAX)
> +       if (outarg.namelen > fc->name_max)
>                 goto err;
>
>         err = -EINVAL;
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 494ac372ace07ab4ea06c13a404ecc1d2ccb4f23..42db112e052f0c26d1ba9973b033b1c7cd822359 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -371,7 +371,7 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
>
>         *inode = NULL;
>         err = -ENAMETOOLONG;
> -       if (name->len > FUSE_NAME_MAX)
> +       if (name->len > fm->fc->name_max)
>                 goto out;
>
>
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 74744c6f286003251564d1235f4d2ca8654d661b..5ce19bc6871291eeaa4c4af4ea935d4de80e8a00 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -38,8 +38,12 @@
>  /** Bias for fi->writectr, meaning new writepages must not be sent */
>  #define FUSE_NOWRITE INT_MIN
>
> -/** It could be as large as PATH_MAX, but would that have any uses? */
> -#define FUSE_NAME_MAX 1024
> +/** Maximum length of a filename, not including terminating null */
> +
> +/* maximum, small enough for FUSE_MIN_READ_BUFFER*/
> +#define FUSE_NAME_LOW_MAX 1024
> +/* maximum, but needs a request buffer > FUSE_MIN_READ_BUFFER */
> +#define FUSE_NAME_MAX (PATH_MAX - 1)
>
>  /** Number of dentries for each connection in the control filesystem */
>  #define FUSE_CTL_NUM_DENTRIES 5
> @@ -893,6 +897,9 @@ struct fuse_conn {
>         /** Version counter for evict inode */
>         atomic64_t evict_ctr;
>
> +       /* maximum file name length */
> +       u32 name_max;
> +
>         /** Called on final put */
>         void (*release)(struct fuse_conn *);
>
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 3ce4f4e81d09e867c3a7db7b1dbb819f88ed34ef..4d61dacedf6a1684eb5dc39a6f56ded0ca4c1fe4 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -978,6 +978,7 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
>         fc->user_ns = get_user_ns(user_ns);
>         fc->max_pages = FUSE_DEFAULT_MAX_PAGES_PER_REQ;
>         fc->max_pages_limit = fuse_max_pages_limit;
> +       fc->name_max = FUSE_NAME_LOW_MAX;
>
>         if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
>                 fuse_backing_files_init(fc);
> @@ -1335,6 +1336,13 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
>                                 fc->max_pages =
>                                         min_t(unsigned int, fc->max_pages_limit,
>                                         max_t(unsigned int, arg->max_pages, 1));
> +
> +                               /*
> +                                * PATH_MAX file names might need two pages for
> +                                * ops like rename
> +                                */
> +                               if (fc->max_pages > 1)
> +                                       fc->name_max = FUSE_NAME_MAX;

For the case of FUSE_REANME (and FUSE_RENAME2, FUSE_SYMLINK) with
large file-names (4095) you would need 3 pages (PAGE_SIZE=4096):
fuse_in_header (40) + fuse_rename_in (8) + names (2 * 4095).


>                         }
>                         if (IS_ENABLED(CONFIG_FUSE_DAX)) {
>                                 if (flags & FUSE_MAP_ALIGNMENT &&
>
> --
> 2.43.0
>
Bernd Schubert Dec. 18, 2024, 9:25 a.m. UTC | #2
On 12/18/24 10:15, Shachar Sharon wrote:
> On Mon, Dec 16, 2024 at 11:14 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 %xx escape characters.
>> The limit is increased to (PATH_MAX - 1), as we need
>> 3 x 1024 and that is close to PATH_MAX (4kB) already.
>> -1 is used as the terminating null is not included in the
>> length calculation.
>>
>> 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
>>
>> The connection is initialized with FUSE_NAME_LOW_MAX, which
>> is set to the previous value of FUSE_NAME_MAX of 1024. With
>> FUSE_MIN_READ_BUFFER of 8192 that is enough for two file names
>> + fuse headers.
>> When FUSE_INIT reply sets max_pages to a value > 1 we know
>> that fuse daemon supports request buffers of at least 2 pages
>> (+ header) and can therefore hold 2 x PATH_MAX file names - operations
>> like rename or link that need two file names are no issue then.
>>
>> Signed-off-by: Bernd Schubert <bschubert@ddn.com>
>> ---
>>  fs/fuse/dev.c    |  4 ++--
>>  fs/fuse/dir.c    |  2 +-
>>  fs/fuse/fuse_i.h | 11 +++++++++--
>>  fs/fuse/inode.c  |  8 ++++++++
>>  4 files changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
>> index c979ce93685f8338301a094ac513c607f44ba572..3b4bdff84e534be8b1ce4a970e841b6a362ef176 100644
>> --- a/fs/fuse/dev.c
>> +++ b/fs/fuse/dev.c
>> @@ -1538,7 +1538,7 @@ static int fuse_notify_inval_entry(struct fuse_conn *fc, unsigned int size,
>>                 goto err;
>>
>>         err = -ENAMETOOLONG;
>> -       if (outarg.namelen > FUSE_NAME_MAX)
>> +       if (outarg.namelen > fc->name_max)
>>                 goto err;
>>
>>         err = -EINVAL;
>> @@ -1587,7 +1587,7 @@ static int fuse_notify_delete(struct fuse_conn *fc, unsigned int size,
>>                 goto err;
>>
>>         err = -ENAMETOOLONG;
>> -       if (outarg.namelen > FUSE_NAME_MAX)
>> +       if (outarg.namelen > fc->name_max)
>>                 goto err;
>>
>>         err = -EINVAL;
>> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
>> index 494ac372ace07ab4ea06c13a404ecc1d2ccb4f23..42db112e052f0c26d1ba9973b033b1c7cd822359 100644
>> --- a/fs/fuse/dir.c
>> +++ b/fs/fuse/dir.c
>> @@ -371,7 +371,7 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
>>
>>         *inode = NULL;
>>         err = -ENAMETOOLONG;
>> -       if (name->len > FUSE_NAME_MAX)
>> +       if (name->len > fm->fc->name_max)
>>                 goto out;
>>
>>
>> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
>> index 74744c6f286003251564d1235f4d2ca8654d661b..5ce19bc6871291eeaa4c4af4ea935d4de80e8a00 100644
>> --- a/fs/fuse/fuse_i.h
>> +++ b/fs/fuse/fuse_i.h
>> @@ -38,8 +38,12 @@
>>  /** Bias for fi->writectr, meaning new writepages must not be sent */
>>  #define FUSE_NOWRITE INT_MIN
>>
>> -/** It could be as large as PATH_MAX, but would that have any uses? */
>> -#define FUSE_NAME_MAX 1024
>> +/** Maximum length of a filename, not including terminating null */
>> +
>> +/* maximum, small enough for FUSE_MIN_READ_BUFFER*/
>> +#define FUSE_NAME_LOW_MAX 1024
>> +/* maximum, but needs a request buffer > FUSE_MIN_READ_BUFFER */
>> +#define FUSE_NAME_MAX (PATH_MAX - 1)
>>
>>  /** Number of dentries for each connection in the control filesystem */
>>  #define FUSE_CTL_NUM_DENTRIES 5
>> @@ -893,6 +897,9 @@ struct fuse_conn {
>>         /** Version counter for evict inode */
>>         atomic64_t evict_ctr;
>>
>> +       /* maximum file name length */
>> +       u32 name_max;
>> +
>>         /** Called on final put */
>>         void (*release)(struct fuse_conn *);
>>
>> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
>> index 3ce4f4e81d09e867c3a7db7b1dbb819f88ed34ef..4d61dacedf6a1684eb5dc39a6f56ded0ca4c1fe4 100644
>> --- a/fs/fuse/inode.c
>> +++ b/fs/fuse/inode.c
>> @@ -978,6 +978,7 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
>>         fc->user_ns = get_user_ns(user_ns);
>>         fc->max_pages = FUSE_DEFAULT_MAX_PAGES_PER_REQ;
>>         fc->max_pages_limit = fuse_max_pages_limit;
>> +       fc->name_max = FUSE_NAME_LOW_MAX;
>>
>>         if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
>>                 fuse_backing_files_init(fc);
>> @@ -1335,6 +1336,13 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
>>                                 fc->max_pages =
>>                                         min_t(unsigned int, fc->max_pages_limit,
>>                                         max_t(unsigned int, arg->max_pages, 1));
>> +
>> +                               /*
>> +                                * PATH_MAX file names might need two pages for
>> +                                * ops like rename
>> +                                */
>> +                               if (fc->max_pages > 1)
>> +                                       fc->name_max = FUSE_NAME_MAX;
> 
> For the case of FUSE_REANME (and FUSE_RENAME2, FUSE_SYMLINK) with
> large file-names (4095) you would need 3 pages (PAGE_SIZE=4096):
> fuse_in_header (40) + fuse_rename_in (8) + names (2 * 4095).

Sure, but if you look into functions like fuse_perform_write() and
fuse_direct_io(), fc->max_pages does not account the header. 
Maybe not documented as that, but effectively used as "payload"
pages, which is also what the file/dir names are.


Thanks,
Bernd
diff mbox series

Patch

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index c979ce93685f8338301a094ac513c607f44ba572..3b4bdff84e534be8b1ce4a970e841b6a362ef176 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -1538,7 +1538,7 @@  static int fuse_notify_inval_entry(struct fuse_conn *fc, unsigned int size,
 		goto err;
 
 	err = -ENAMETOOLONG;
-	if (outarg.namelen > FUSE_NAME_MAX)
+	if (outarg.namelen > fc->name_max)
 		goto err;
 
 	err = -EINVAL;
@@ -1587,7 +1587,7 @@  static int fuse_notify_delete(struct fuse_conn *fc, unsigned int size,
 		goto err;
 
 	err = -ENAMETOOLONG;
-	if (outarg.namelen > FUSE_NAME_MAX)
+	if (outarg.namelen > fc->name_max)
 		goto err;
 
 	err = -EINVAL;
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 494ac372ace07ab4ea06c13a404ecc1d2ccb4f23..42db112e052f0c26d1ba9973b033b1c7cd822359 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -371,7 +371,7 @@  int fuse_lookup_name(struct super_block *sb, u64 nodeid, const struct qstr *name
 
 	*inode = NULL;
 	err = -ENAMETOOLONG;
-	if (name->len > FUSE_NAME_MAX)
+	if (name->len > fm->fc->name_max)
 		goto out;
 
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 74744c6f286003251564d1235f4d2ca8654d661b..5ce19bc6871291eeaa4c4af4ea935d4de80e8a00 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -38,8 +38,12 @@ 
 /** Bias for fi->writectr, meaning new writepages must not be sent */
 #define FUSE_NOWRITE INT_MIN
 
-/** It could be as large as PATH_MAX, but would that have any uses? */
-#define FUSE_NAME_MAX 1024
+/** Maximum length of a filename, not including terminating null */
+
+/* maximum, small enough for FUSE_MIN_READ_BUFFER*/
+#define FUSE_NAME_LOW_MAX 1024
+/* maximum, but needs a request buffer > FUSE_MIN_READ_BUFFER */
+#define FUSE_NAME_MAX (PATH_MAX - 1)
 
 /** Number of dentries for each connection in the control filesystem */
 #define FUSE_CTL_NUM_DENTRIES 5
@@ -893,6 +897,9 @@  struct fuse_conn {
 	/** Version counter for evict inode */
 	atomic64_t evict_ctr;
 
+	/* maximum file name length */
+	u32 name_max;
+
 	/** Called on final put */
 	void (*release)(struct fuse_conn *);
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 3ce4f4e81d09e867c3a7db7b1dbb819f88ed34ef..4d61dacedf6a1684eb5dc39a6f56ded0ca4c1fe4 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -978,6 +978,7 @@  void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
 	fc->user_ns = get_user_ns(user_ns);
 	fc->max_pages = FUSE_DEFAULT_MAX_PAGES_PER_REQ;
 	fc->max_pages_limit = fuse_max_pages_limit;
+	fc->name_max = FUSE_NAME_LOW_MAX;
 
 	if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
 		fuse_backing_files_init(fc);
@@ -1335,6 +1336,13 @@  static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 				fc->max_pages =
 					min_t(unsigned int, fc->max_pages_limit,
 					max_t(unsigned int, arg->max_pages, 1));
+
+				/*
+				 * PATH_MAX file names might need two pages for
+				 * ops like rename
+				 */
+				if (fc->max_pages > 1)
+					fc->name_max = FUSE_NAME_MAX;
 			}
 			if (IS_ENABLED(CONFIG_FUSE_DAX)) {
 				if (flags & FUSE_MAP_ALIGNMENT &&