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
>
