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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/18/24 10:15, Shachar Sharon wrote:
> On Mon, Dec 16, 2024 at 11:14 PM Bernd Schubert <bschubert@xxxxxxx> 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@xxxxxxx>
>> ---
>>  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







[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux