On Sun 25-11-18 15:43:44, Amir Goldstein wrote:
> With group flag FAN_REPORT_FID, we do not store event->path.
> Instead, we will store the file handle and fsid in event->info.fid.
>
> Because event->info and event->path share the same union space, set
> bit 0 of event->info.flags, so we know how to free and compare the
> event objects.
>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> ---
> fs/notify/fanotify/fanotify.c | 35 ++++++++++++++++++++++++------
> fs/notify/fanotify/fanotify.h | 27 +++++++++++++++++++++++
> fs/notify/fanotify/fanotify_user.c | 3 +++
> 3 files changed, 58 insertions(+), 7 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 59d093923c97..8192d4b1db21 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -25,11 +25,16 @@ static bool should_merge(struct fsnotify_event *old_fsn,
> old = FANOTIFY_E(old_fsn);
> new = FANOTIFY_E(new_fsn);
>
> - if (old_fsn->inode == new_fsn->inode && old->pid == new->pid &&
> - old->path.mnt == new->path.mnt &&
> - old->path.dentry == new->path.dentry)
> - return true;
> - return false;
> + if (old_fsn->inode != new_fsn->inode || old->pid != new->pid)
> + return false;
> +
> + if (FANOTIFY_HAS_FID(new)) {
> + return FANOTIFY_HAS_FID(old) &&
> + fanotify_fid_equal(old->info.fid, new->info.fid);
> + } else {
> + return old->path.mnt == new->path.mnt &&
> + old->path.dentry == new->path.dentry;
> + }
> }
>
> /* and the list better be locked by something too! */
> @@ -178,7 +183,10 @@ init: __maybe_unused
> event->pid = get_pid(task_pid(current));
> else
> event->pid = get_pid(task_tgid(current));
> - if (path && !FAN_GROUP_FLAG(group, FAN_REPORT_FID)) {
> + if (FAN_GROUP_FLAG(group, FAN_REPORT_FID)) {
> + /* TODO: allocate buffer and encode file handle */
> + fanotify_set_fid(event, NULL);
> + } else if (path) {
> event->path = *path;
> path_get(&event->path);
> } else {
> @@ -277,8 +285,21 @@ static void fanotify_free_event(struct fsnotify_event *fsn_event)
> {
> struct fanotify_event *event;
>
> + /*
> + * event->path and event->info are a union. Make sure that
> + * event->info.flags overlaps with path.dentry pointer, so it is safe
> + * to use bit 0 to classify the event info type (FANOTIFY_HAS_FID).
> + */
> + BUILD_BUG_ON(offsetof(struct fanotify_event, path.dentry) !=
> + offsetof(struct fanotify_event, info.flags));
> + BUILD_BUG_ON(!__builtin_types_compatible_p(typeof(event->info.flags),
> + unsigned long));
> +
> event = FANOTIFY_E(fsn_event);
> - path_put(&event->path);
> + if (FANOTIFY_HAS_FID(event))
> + kfree(event->info.fid);
> + else
> + path_put(&event->path);
> put_pid(event->pid);
> if (fanotify_is_perm_event(fsn_event->mask)) {
> kmem_cache_free(fanotify_perm_event_cachep,
> diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
> index 2e4fca30afda..a79dcbd41702 100644
> --- a/fs/notify/fanotify/fanotify.h
> +++ b/fs/notify/fanotify/fanotify.h
> @@ -13,8 +13,20 @@ extern struct kmem_cache *fanotify_perm_event_cachep;
>
> struct fanotify_info {
> struct fanotify_event_fid *fid;
> + unsigned long flags;
> };
>
> +static inline bool fanotify_fid_equal(const struct fanotify_event_fid *fid1,
> + const struct fanotify_event_fid *fid2)
> +{
> + if (fid1 == fid2)
> + return true;
> +
> + return fid1 && fid2 && fid1->handle_bytes == fid2->handle_bytes &&
> + !memcmp((void *)fid1, (void *)fid2,
> + FANOTIFY_FID_LEN(fid1->handle_bytes));
> +}
> +
> /*
> * Structure for normal fanotify events. It gets allocated in
> * fanotify_handle_event() and freed when the information is retrieved by
> @@ -38,6 +50,21 @@ struct fanotify_event {
> struct pid *pid;
> };
>
> +/*
> + * Bit 0 of info.flags is set when event has fid information.
> + * event->info shares the same union address with event->path, so this helps
> + * us tell if event has fid or path.
> + */
> +#define __FANOTIFY_FID 1UL
> +#define FANOTIFY_HAS_FID(event) ((event)->info.flags & __FANOTIFY_FID)
Why isn't this an inline function? And then the name wouldn't have to be
all caps... Also
> +static inline void fanotify_set_fid(struct fanotify_event *event,
> + struct fanotify_event_fid *fid)
> +{
> + event->info.fid = fid;
> + event->info.flags = fid ? __FANOTIFY_FID : 0;
> +}
> +
> /*
> * Structure for permission fanotify events. It gets allocated and freed in
> * fanotify_handle_event() since we wait there for user response. When the
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 93e1aa2a389f..47e8bf3bcd28 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -81,6 +81,9 @@ static int create_fd(struct fsnotify_group *group,
>
> pr_debug("%s: group=%p event=%p\n", __func__, group, event);
>
> + if (WARN_ON_ONCE(FANOTIFY_HAS_FID(event)))
> + return -EBADF;
> +
> client_fd = get_unused_fd_flags(group->fanotify_data.f_flags);
> if (client_fd < 0)
> return client_fd;
> --
> 2.17.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
