Re: [PATCH] fanotify: report names longer than NAME_MAX

[Amir Goldstein]

On Sat, Sep 26, 2026 at 4:08 AM Matthias Goergens
<matthias.goergens@xxxxxxxxx> wrote:
>
> fanotify keeps the length of a reported name in a u8, so it drops any
> name longer than NAME_MAX and warns:
>
>   WARNING: fs/notify/fanotify/fanotify.h:217 at fanotify_handle_event+0x2e82/0x39f0
>
> Such names are legitimate on some filesystems.  vfat mounted with utf8
> takes up to 255 UTF-16 characters, which are up to 765 bytes of UTF-8,
> and FUSE accepts names up to PATH_MAX - 1.  readdir() and inotify report
> them in full.  fanotify sends the event without the name, and for
> FAN_RENAME it trips a second warning in copy_fid_info_to_user(), after
> which read() fails with EFAULT and the event is lost.
>
> When syzbot hit this, Jan suggested accommodating names up to PATH_MAX
> [1], the limit readdir() already applies to a name
> (verify_dirent_name()).  Store the name lengths as u16, which keeps
> struct fanotify_info at 8 bytes, and drop without a warning only names
> of PATH_MAX bytes or more, which no path can refer to.  An event with
> such a name can exceed a page, so allocate name events with kvmalloc(),
> as Jan also suggested.  Names up to NAME_MAX are reported exactly as
> before, and the record format is unchanged: an event with a long name is
> just longer.
>
> Link: https://lore.kernel.org/all/20241112115059.ecvomkalee5m4i4i@quack3/ [1]
> Fixes: 2d9374f09513 ("fanotify: use macros to get the offset to fanotify_info buffer")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Matthias Goergens <matthias.goergens@xxxxxxxxx>
> ---
>
> Tested in a VM with KASAN, lockdep and kmemleak: on vfat (utf8), names
> of 258 to 765 bytes now arrive with their name, including FAN_RENAME,
> for root and for an unprivileged watcher; FUSE names up to 4095 bytes
> likewise, also a FAN_RENAME between two of them.  With kvmalloc()'s
> kmalloc() attempt forced to fail, the events arrive intact from vmalloc.
> No warnings.  The LTP fanotify and inotify tests give the same results
> as before (fanotify20 skipped in both).
>
> 5.15.y fails the same way (the same warnings and EFAULT on 5.15.221),
> and the patch applies there and fixes it.
>
> Honza, on the msdos report [2] you pointed at the filesystem, and msdos
> now rejects such names (9d7ed813ee5f).  vfat cannot do the same without
> making files that other systems wrote unreachable, and FUSE allows them
> by design.
>
> [2] https://lore.kernel.org/all/3kzvlb3wpzbg6k6ttzp6rh2cfmz3hmmmlisgxjn5u4xmaaqibm@qkre7flkhejl/
>
>  fs/notify/fanotify/fanotify.c |  4 ++--
>  fs/notify/fanotify/fanotify.h | 20 +++++++++++++-------
>  2 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index a208a7ec1692..c8289aa8a70a 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -656,7 +656,7 @@ static struct fanotify_event *fanotify_alloc_name_event(struct inode *dir,
>                 size += FANOTIFY_FH_HDR_LEN + dir2_fh_len;
>         if (child_fh_len)
>                 size += FANOTIFY_FH_HDR_LEN + child_fh_len;
> -       fne = kmalloc(size, gfp);
> +       fne = kvmalloc(size, gfp);
>         if (!fne)
>                 return NULL;
>
> @@ -1050,7 +1050,7 @@ static void fanotify_free_fid_event(struct fanotify_event *event)
>
>  static void fanotify_free_name_event(struct fanotify_event *event)
>  {
> -       kfree(FANOTIFY_NE(event));
> +       kvfree(FANOTIFY_NE(event));
>  }
>
>  static void fanotify_free_error_event(struct fsnotify_group *group,
> diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
> index 3710543dbf82..4aad60d2aaf5 100644
> --- a/fs/notify/fanotify/fanotify.h
> +++ b/fs/notify/fanotify/fanotify.h
> @@ -43,9 +43,16 @@ struct fanotify_info {
>         u8 dir_fh_totlen;
>         u8 dir2_fh_totlen;
>         u8 file_fh_totlen;
> -       u8 name_len;
> -       u8 name2_len;
> -       u8 pad[3];
> +       u8 pad;
> +       /*
> +        * Deliberately not limited to NAME_MAX: some filesystems return longer
> +        * names (vfat up to 255 UTF-16 characters, as Windows allows, which is
> +        * up to 765 bytes of UTF-8; FUSE up to PATH_MAX - 1), and readdir() and
> +        * inotify report them in full. Only names of PATH_MAX bytes or more,
> +        * which no path can refer to, are dropped.
> +        */
> +       u16 name_len;
> +       u16 name2_len;
>         unsigned char buf[];
>         /*
>          * (struct fanotify_fh) dir_fh starts at buf[0]
> @@ -168,7 +175,7 @@ static inline char *fanotify_info_name2(struct fanotify_info *info)
>  static inline void fanotify_info_init(struct fanotify_info *info)
>  {
>         BUILD_BUG_ON(FANOTIFY_FH_HDR_LEN + MAX_HANDLE_SZ > U8_MAX);
> -       BUILD_BUG_ON(NAME_MAX > U8_MAX);
> +       BUILD_BUG_ON(PATH_MAX > U16_MAX);
>
>         info->dir_fh_totlen = 0;
>         info->dir2_fh_totlen = 0;
> @@ -214,8 +221,7 @@ static inline void fanotify_info_set_file_fh(struct fanotify_info *info,
>  static inline void fanotify_info_copy_name(struct fanotify_info *info,
>                                            const struct qstr *name)
>  {
> -       if (WARN_ON_ONCE(name->len > NAME_MAX) ||
> -           WARN_ON_ONCE(info->name2_len > 0))
> +       if (name->len >= PATH_MAX || WARN_ON_ONCE(info->name2_len > 0))
>                 return;
>
>         info->name_len = name->len;
> @@ -225,7 +231,7 @@ static inline void fanotify_info_copy_name(struct fanotify_info *info,
>  static inline void fanotify_info_copy_name2(struct fanotify_info *info,
>                                             const struct qstr *name)
>  {
> -       if (WARN_ON_ONCE(name->len > NAME_MAX))
> +       if (name->len >= PATH_MAX)
>                 return;
>
>         info->name2_len = name->len;
>
> base-commit: 165768bb70265b5c38cf0b73fafd75be235f8b14
> --
> 2.55.0
>

Hi Matthias,

The fix for name_len < PATH_MAX looks good, but since you dealt with the
the unexpected case of name_len > PATH_MAX more gracefully on event record
time (empty name) I would rather that we treat it gracefully only on read() time
and just report the event with no name instead of potentially breaking
the read()
of other events in the queue.

Something like this:

--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -579,11 +579,17 @@ static int copy_fid_info_to_user(__kernel_fsid_t
*fsid, struct fanotify_fh *fh,
                        return -EFAULT;
                break;
        case FAN_EVENT_INFO_TYPE_DFID_NAME:
-       case FAN_EVENT_INFO_TYPE_OLD_DFID_NAME:
-       case FAN_EVENT_INFO_TYPE_NEW_DFID_NAME:
                if (WARN_ON_ONCE(!name || !name_len))
                        return -EFAULT;
                break;
+       case FAN_EVENT_INFO_TYPE_OLD_DFID_NAME:
+       case FAN_EVENT_INFO_TYPE_NEW_DFID_NAME:
+               /*
+                * FAN_RENAME uses these types so userspace can tell old from
+                * new parent. The name may be absent if it could not be
+                * recorded (e.g. >= PATH_MAX); emit fid-only like DFID.
+                */
+               break;
        default:
                return -EFAULT;
        }

Thanks,
Amir.