Re: [RFC][PATCH 2/2] fanotify: report mntid info record with FAN_UNMOUNT events

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

 



On Fri, Apr 14, 2023 at 9:29 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> Report mntid in an info record of type FAN_EVENT_INFO_TYPE_MNTID
> with FAN_UNMOUNT event in addition to the fid info record.
>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
>  fs/notify/fanotify/fanotify.c      | 37 +++++++++++++++++++------
>  fs/notify/fanotify/fanotify.h      | 20 +++++++++++++-
>  fs/notify/fanotify/fanotify_user.c | 44 +++++++++++++++++++++++++++---
>  include/uapi/linux/fanotify.h      | 10 +++++++
>  4 files changed, 97 insertions(+), 14 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 384d2b2e55e7..c204259be6cc 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -17,6 +17,7 @@
>  #include <linux/stringhash.h>
>
>  #include "fanotify.h"
> +#include "../../mount.h" /* for mnt_id */
>
>  static bool fanotify_path_equal(const struct path *p1, const struct path *p2)
>  {
> @@ -41,6 +42,11 @@ static unsigned int fanotify_hash_fsid(__kernel_fsid_t *fsid)
>                 hash_32(fsid->val[1], FANOTIFY_EVENT_HASH_BITS);
>  }
>
> +static unsigned int fanotify_hash_mntid(int mntid)
> +{
> +       return hash_32(mntid, FANOTIFY_EVENT_HASH_BITS);
> +}

This hash is not needed of course as I later decided to never merge
FAN_UNMOUNT events.

> +
>  static bool fanotify_fh_equal(struct fanotify_fh *fh1,
>                               struct fanotify_fh *fh2)
>  {
> @@ -133,6 +139,12 @@ static bool fanotify_error_event_equal(struct fanotify_error_event *fee1,
>         return true;
>  }
>
> +/*
> + * FAN_RENAME and FAN_UNMOUNT are reported with special info record types,
> + * so we cannot merge them with other events.
> + */
> +#define FANOTIFY_NO_MERGE_EVENTS (FAN_RENAME | FAN_UNMOUNT)
> +
>  static bool fanotify_should_merge(struct fanotify_event *old,
>                                   struct fanotify_event *new)
>  {
> @@ -153,11 +165,8 @@ static bool fanotify_should_merge(struct fanotify_event *old,
>         if ((old->mask & FS_ISDIR) != (new->mask & FS_ISDIR))
>                 return false;
>
> -       /*
> -        * FAN_RENAME event is reported with special info record types,
> -        * so we cannot merge it with other events.
> -        */
> -       if ((old->mask & FAN_RENAME) != (new->mask & FAN_RENAME))
> +       if ((old->mask & FANOTIFY_NO_MERGE_EVENTS) ||
> +           (new->mask & FANOTIFY_NO_MERGE_EVENTS))
>                 return false;

BTW there is a minor change of logic here, from not merging FAN_RENAME
events with anything but other FAN_RENAME events, to not merging
FAN_RENAME events at all.

But considering that FAN_RENAME events always have name info, the
only possible merge of FAN_RENAME events is in a situation of
loop { mv A B; mv B A;}
and I don't think that this corner case justifies the merge of rename events.


>
>         switch (old->type) {
> @@ -593,9 +602,11 @@ static struct fanotify_event *fanotify_alloc_perm_event(const struct path *path,
>
>  static struct fanotify_event *fanotify_alloc_fid_event(struct inode *id,
>                                                        __kernel_fsid_t *fsid,
> +                                                      struct mount *mnt,
>                                                        unsigned int *hash,
>                                                        gfp_t gfp)
>  {
> +       unsigned int fh_len = fanotify_encode_fh_len(id);
>         struct fanotify_fid_event *ffe;
>
>         ffe = kmem_cache_alloc(fanotify_fid_event_cachep, gfp);
> @@ -605,8 +616,14 @@ static struct fanotify_event *fanotify_alloc_fid_event(struct inode *id,
>         ffe->fae.type = FANOTIFY_EVENT_TYPE_FID;
>         ffe->fsid = *fsid;
>         *hash ^= fanotify_hash_fsid(fsid);
> -       fanotify_encode_fh(&ffe->object_fh, id, fanotify_encode_fh_len(id),
> -                          hash, gfp);
> +       fanotify_encode_fh(&ffe->object_fh, id, fh_len, hash, gfp);
> +       /* Record fid event with fsid, mntid and empty fh */
> +       if (mnt && !WARN_ON_ONCE(fh_len)) {
> +               ffe->mnt_id = mnt->mnt_id;
> +               ffe->object_fh.flags = FANOTIFY_FH_FLAG_MNT_ID;
> +               if (hash)
> +                       *hash ^= fanotify_hash_mntid(mnt->mnt_id);
> +       }
>
>         return &ffe->fae;
>  }
> @@ -737,6 +754,7 @@ static struct fanotify_event *fanotify_alloc_event(
>                                               fid_mode);
>         struct inode *dirid = fanotify_dfid_inode(mask, data, data_type, dir);
>         const struct path *path = fsnotify_data_path(data, data_type);
> +       struct mount *mnt = NULL;
>         struct mem_cgroup *old_memcg;
>         struct dentry *moved = NULL;
>         struct inode *child = NULL;
> @@ -746,7 +764,8 @@ static struct fanotify_event *fanotify_alloc_event(
>         struct pid *pid;
>
>         if (mask & FAN_UNMOUNT && !WARN_ON_ONCE(!path || !fid_mode)) {
> -               /* Record fid event with fsid and empty fh */
> +               /* Record fid event with fsid, mntid and empty fh */
> +               mnt = real_mount(path->mnt);
>                 id = NULL;
>         } else if ((fid_mode & FAN_REPORT_DIR_FID) && dirid) {
>                 /*
> @@ -834,7 +853,7 @@ static struct fanotify_event *fanotify_alloc_event(
>                 event = fanotify_alloc_name_event(dirid, fsid, file_name, child,
>                                                   moved, &hash, gfp);
>         } else if (fid_mode) {
> -               event = fanotify_alloc_fid_event(id, fsid, &hash, gfp);
> +               event = fanotify_alloc_fid_event(id, fsid, mnt, &hash, gfp);
>         } else {
>                 event = fanotify_alloc_path_event(path, &hash, gfp);
>         }
> diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
> index f98dcf5b7a19..3d8391a77031 100644
> --- a/fs/notify/fanotify/fanotify.h
> +++ b/fs/notify/fanotify/fanotify.h
> @@ -33,6 +33,7 @@ struct fanotify_fh {
>         u8 type;
>         u8 len;
>  #define FANOTIFY_FH_FLAG_EXT_BUF 1
> +#define FANOTIFY_FH_FLAG_MNT_ID  2
>         u8 flags;
>         u8 pad;
>         unsigned char buf[];
> @@ -279,7 +280,10 @@ static inline void fanotify_init_event(struct fanotify_event *event,
>  struct {                                                               \
>         struct fanotify_fh (name);                                      \
>         /* Space for object_fh.buf[] - access with fanotify_fh_buf() */ \
> -       unsigned char _inline_fh_buf[(size)];                           \
> +       union {                                                         \
> +               unsigned char _inline_fh_buf[(size)];                   \
> +               int mnt_id;     /* For FAN_UNMOUNT */                   \
> +       };                                                              \
>  }
>
>  struct fanotify_fid_event {
> @@ -335,6 +339,20 @@ static inline __kernel_fsid_t *fanotify_event_fsid(struct fanotify_event *event)
>                 return NULL;
>  }
>
> +static inline int fanotify_event_mntid(struct fanotify_event *event)
> +{
> +       struct fanotify_fh *fh = NULL;
> +
> +       if (event->mask & FAN_UNMOUNT &&
> +           event->type == FANOTIFY_EVENT_TYPE_FID)
> +               fh = &FANOTIFY_FE(event)->object_fh;
> +
> +       if (fh && !fh->len && fh->flags == FANOTIFY_FH_FLAG_MNT_ID)
> +               return FANOTIFY_FE(event)->mnt_id;
> +
> +       return 0;
> +}
> +
>  static inline struct fanotify_fh *fanotify_event_object_fh(
>                                                 struct fanotify_event *event)
>  {
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 0b3de6218c56..db3b79b8e901 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -120,7 +120,9 @@ struct kmem_cache *fanotify_perm_event_cachep __read_mostly;
>  #define FANOTIFY_EVENT_ALIGN 4
>  #define FANOTIFY_FID_INFO_HDR_LEN \
>         (sizeof(struct fanotify_event_info_fid) + sizeof(struct file_handle))
> -#define FANOTIFY_PIDFD_INFO_HDR_LEN \
> +#define FANOTIFY_MNTID_INFO_LEN \
> +       sizeof(struct fanotify_event_info_mntid)
> +#define FANOTIFY_PIDFD_INFO_LEN \
>         sizeof(struct fanotify_event_info_pidfd)
>  #define FANOTIFY_ERROR_INFO_LEN \
>         (sizeof(struct fanotify_event_info_error))
> @@ -178,8 +180,11 @@ static size_t fanotify_event_len(unsigned int info_mode,
>                 dot_len = 1;
>         }
>
> +       if (fanotify_event_mntid(event))
> +               event_len += FANOTIFY_MNTID_INFO_LEN;
> +
>         if (info_mode & FAN_REPORT_PIDFD)
> -               event_len += FANOTIFY_PIDFD_INFO_HDR_LEN;
> +               event_len += FANOTIFY_PIDFD_INFO_LEN;
>
>         if (fanotify_event_has_object_fh(event)) {
>                 fh_len = fanotify_event_object_fh_len(event);
> @@ -515,7 +520,7 @@ static int copy_pidfd_info_to_user(int pidfd,
>                                    size_t count)
>  {
>         struct fanotify_event_info_pidfd info = { };
> -       size_t info_len = FANOTIFY_PIDFD_INFO_HDR_LEN;
> +       size_t info_len = FANOTIFY_PIDFD_INFO_LEN;
>
>         if (WARN_ON_ONCE(info_len > count))
>                 return -EFAULT;
> @@ -530,6 +535,26 @@ static int copy_pidfd_info_to_user(int pidfd,
>         return info_len;
>  }
>
> +static int copy_mntid_info_to_user(int mntid,
> +                                  char __user *buf,
> +                                  size_t count)
> +{
> +       struct fanotify_event_info_mntid info = { };
> +       size_t info_len = FANOTIFY_MNTID_INFO_LEN;
> +
> +       if (WARN_ON_ONCE(info_len > count))
> +               return -EFAULT;
> +
> +       info.hdr.info_type = FAN_EVENT_INFO_TYPE_MNTID;
> +       info.hdr.len = info_len;
> +       info.mnt_id = mntid;
> +
> +       if (copy_to_user(buf, &info, info_len))
> +               return -EFAULT;
> +
> +       return info_len;
> +}
> +
>  static int copy_info_records_to_user(struct fanotify_event *event,
>                                      struct fanotify_info *info,
>                                      unsigned int info_mode, int pidfd,
> @@ -538,6 +563,7 @@ static int copy_info_records_to_user(struct fanotify_event *event,
>         int ret, total_bytes = 0, info_type = 0;
>         unsigned int fid_mode = info_mode & FANOTIFY_FID_BITS;
>         unsigned int pidfd_mode = info_mode & FAN_REPORT_PIDFD;
> +       int mntid = fanotify_event_mntid(event);
>
>         /*
>          * Event info records order is as follows:
> @@ -632,6 +658,16 @@ static int copy_info_records_to_user(struct fanotify_event *event,
>                 total_bytes += ret;
>         }
>
> +       if (mntid) {
> +               ret = copy_mntid_info_to_user(mntid, buf, count);
> +               if (ret < 0)
> +                       return ret;
> +
> +               buf += ret;
> +               count -= ret;
> +               total_bytes += ret;
> +       }
> +
>         if (pidfd_mode) {
>                 ret = copy_pidfd_info_to_user(pidfd, buf, count);
>                 if (ret < 0)
> @@ -1770,7 +1806,7 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
>          * inotify sends unsoliciled IN_UNMOUNT per marked inode on sb shutdown.
>          * FAN_UNMOUNT event is about unmount of a mount, not about sb shutdown,
>          * so allow setting it only in mount mark mask.
> -        * FAN_UNMOUNT requires FAN_REPORT_FID to report fsid with empty fh.
> +        * FAN_UNMOUNT requires FAN_REPORT_FID to report fsid and mntid.
>          */
>         if (mask & FAN_UNMOUNT &&
>             (!(fid_mode & FAN_REPORT_FID) || mark_type != FAN_MARK_MOUNT))
> diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
> index 70f2d43e8ba4..886efbd877ba 100644
> --- a/include/uapi/linux/fanotify.h
> +++ b/include/uapi/linux/fanotify.h
> @@ -144,6 +144,7 @@ struct fanotify_event_metadata {
>  #define FAN_EVENT_INFO_TYPE_DFID       3
>  #define FAN_EVENT_INFO_TYPE_PIDFD      4
>  #define FAN_EVENT_INFO_TYPE_ERROR      5
> +#define FAN_EVENT_INFO_TYPE_MNTID      6
>
>  /* Special info types for FAN_RENAME */
>  #define FAN_EVENT_INFO_TYPE_OLD_DFID_NAME      10
> @@ -184,6 +185,15 @@ struct fanotify_fhandle {
>         __u64 fh_ino;
>  };
>
> +/*
> + * This structure is used for info records of type FAN_EVENT_INFO_TYPE_MNTID.
> + */
> +struct fanotify_event_info_mntid {
> +       struct fanotify_event_info_header hdr;
> +       /* matches mount_id from name_to_handle_at(2) */
> +       __s32 mnt_id;
> +};
> +
>  /*
>   * This structure is used for info records of type FAN_EVENT_INFO_TYPE_PIDFD.
>   * It holds a pidfd for the pid that was responsible for generating an event.
> --
> 2.34.1
>




[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