All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Jan Kara <jack@suse.cz>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	Linux API <linux-api@vger.kernel.org>,
	Michael Kerrisk <mtk.manpages@gmail.com>
Subject: Re: [PATCH v2 13/16] fanotify: report name info for FAN_DIR_MODIFY event
Date: Thu, 16 Apr 2020 14:16:40 +0200	[thread overview]
Message-ID: <CAKgNAkiVcjQfATKWwGNPDFucMEN4jJnQ5q6JHRzDihK1ZDnH8A@mail.gmail.com> (raw)
In-Reply-To: <20200217131455.31107-14-amir73il@gmail.com>

Hello Amir,

On Mon, 17 Feb 2020 at 15:10, Amir Goldstein <amir73il@gmail.com> wrote:
>
> Report event FAN_DIR_MODIFY with name in a variable length record similar
> to how fid's are reported.  With name info reporting implemented, setting
> FAN_DIR_MODIFY in mark mask is now allowed.

I see this was merged for 5.7. Would you be able to send a man-pages
patch that documents this new feature please.

Cheers,

Michael

>
> When events are reported with name, the reported fid identifies the
> directory and the name follows the fid. The info record type for this
> event info is FAN_EVENT_INFO_TYPE_DFID_NAME.
>
> For now, all reported events have at most one info record which is
> either FAN_EVENT_INFO_TYPE_FID or FAN_EVENT_INFO_TYPE_DFID_NAME (for
> FAN_DIR_MODIFY).  Later on, events "on child" will report both records.
>
> There are several ways that an application can use this information:
>
> 1. When watching a single directory, the name is always relative to
> the watched directory, so application need to fstatat(2) the name
> relative to the watched directory.
>
> 2. When watching a set of directories, the application could keep a map
> of dirfd for all watched directories and hash the map by fid obtained
> with name_to_handle_at(2).  When getting a name event, the fid in the
> event info could be used to lookup the base dirfd in the map and then
> call fstatat(2) with that dirfd.
>
> 3. When watching a filesystem (FAN_MARK_FILESYSTEM) or a large set of
> directories, the application could use open_by_handle_at(2) with the fid
> in event info to obtain dirfd for the directory where event happened and
> call fstatat(2) with this dirfd.
>
> The last option scales better for a large number of watched directories.
> The first two options may be available in the future also for non
> privileged fanotify watchers, because open_by_handle_at(2) requires
> the CAP_DAC_READ_SEARCH capability.
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  fs/notify/fanotify/fanotify.c      |   2 +-
>  fs/notify/fanotify/fanotify_user.c | 120 ++++++++++++++++++++++-------
>  include/linux/fanotify.h           |   3 +-
>  include/uapi/linux/fanotify.h      |   1 +
>  4 files changed, 98 insertions(+), 28 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index fc75dc53a218..b651c18d3a93 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -478,7 +478,7 @@ static int fanotify_handle_event(struct fsnotify_group *group,
>         BUILD_BUG_ON(FAN_OPEN_EXEC != FS_OPEN_EXEC);
>         BUILD_BUG_ON(FAN_OPEN_EXEC_PERM != FS_OPEN_EXEC_PERM);
>
> -       BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 19);
> +       BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 20);
>
>         mask = fanotify_group_event_mask(group, iter_info, mask, data,
>                                          data_type);
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 284f3548bb79..a1bafc21ebbb 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -51,20 +51,32 @@ struct kmem_cache *fanotify_name_event_cachep __read_mostly;
>  struct kmem_cache *fanotify_perm_event_cachep __read_mostly;
>
>  #define FANOTIFY_EVENT_ALIGN 4
> +#define FANOTIFY_INFO_HDR_LEN \
> +       (sizeof(struct fanotify_event_info_fid) + sizeof(struct file_handle))
>
> -static int fanotify_fid_info_len(struct fanotify_fid_hdr *fh)
> +static int fanotify_fid_info_len(int fh_len, int name_len)
>  {
> -       return roundup(sizeof(struct fanotify_event_info_fid) +
> -                      sizeof(struct file_handle) + fh->len,
> -                      FANOTIFY_EVENT_ALIGN);
> +       int info_len = fh_len;
> +
> +       if (name_len)
> +               info_len += name_len + 1;
> +
> +       return roundup(FANOTIFY_INFO_HDR_LEN + info_len, FANOTIFY_EVENT_ALIGN);
>  }
>
>  static int fanotify_event_info_len(struct fanotify_event *event)
>  {
> -       if (!fanotify_event_has_fid(event))
> -               return 0;
> +       int info_len = 0;
> +
> +       if (fanotify_event_has_fid(event))
> +               info_len += fanotify_fid_info_len(event->fh.len, 0);
> +
> +       if (fanotify_event_has_dfid_name(event)) {
> +               info_len += fanotify_fid_info_len(event->dfh.len,
> +                                       fanotify_event_name_len(event));
> +       }
>
> -       return fanotify_fid_info_len(&event->fh);
> +       return info_len;
>  }
>
>  /*
> @@ -210,23 +222,34 @@ static int process_access_response(struct fsnotify_group *group,
>         return -ENOENT;
>  }
>
> -static int copy_fid_to_user(__kernel_fsid_t *fsid, struct fanotify_fid_hdr *fh,
> -                           struct fanotify_fid *fid, char __user *buf)
> +static int copy_info_to_user(__kernel_fsid_t *fsid, struct fanotify_fid_hdr *fh,
> +                            struct fanotify_fid *fid, const struct qstr *name,
> +                            char __user *buf, size_t count)
>  {
>         struct fanotify_event_info_fid info = { };
>         struct file_handle handle = { };
> -       unsigned char bounce[FANOTIFY_INLINE_FH_LEN], *data;
> +       unsigned char bounce[max(FANOTIFY_INLINE_FH_LEN, DNAME_INLINE_LEN)];
> +       const unsigned char *data;
>         size_t fh_len = fh->len;
> -       size_t len = fanotify_fid_info_len(fh);
> +       size_t name_len = name ? name->len : 0;
> +       size_t info_len = fanotify_fid_info_len(fh_len, name_len);
> +       size_t len = info_len;
> +
> +       pr_debug("%s: fh_len=%lu name_len=%lu, info_len=%lu, count=%lu\n",
> +                __func__, fh_len, name_len, info_len, count);
>
> -       if (!len)
> +       if (!fh_len || (name && !name_len))
>                 return 0;
>
> -       if (WARN_ON_ONCE(len < sizeof(info) + sizeof(handle) + fh_len))
> +       if (WARN_ON_ONCE(len < sizeof(info) || len > count))
>                 return -EFAULT;
>
> -       /* Copy event info fid header followed by vaiable sized file handle */
> -       info.hdr.info_type = FAN_EVENT_INFO_TYPE_FID;
> +       /*
> +        * Copy event info fid header followed by vaiable sized file handle
> +        * and optionally followed by vaiable sized filename.
> +        */
> +       info.hdr.info_type = name_len ? FAN_EVENT_INFO_TYPE_DFID_NAME :
> +                                       FAN_EVENT_INFO_TYPE_FID;
>         info.hdr.len = len;
>         info.fsid = *fsid;
>         if (copy_to_user(buf, &info, sizeof(info)))
> @@ -234,6 +257,9 @@ static int copy_fid_to_user(__kernel_fsid_t *fsid, struct fanotify_fid_hdr *fh,
>
>         buf += sizeof(info);
>         len -= sizeof(info);
> +       if (WARN_ON_ONCE(len < sizeof(handle)))
> +               return -EFAULT;
> +
>         handle.handle_type = fh->type;
>         handle.handle_bytes = fh_len;
>         if (copy_to_user(buf, &handle, sizeof(handle)))
> @@ -241,9 +267,12 @@ static int copy_fid_to_user(__kernel_fsid_t *fsid, struct fanotify_fid_hdr *fh,
>
>         buf += sizeof(handle);
>         len -= sizeof(handle);
> +       if (WARN_ON_ONCE(len < fh_len))
> +               return -EFAULT;
> +
>         /*
> -        * For an inline fh, copy through stack to exclude the copy from
> -        * usercopy hardening protections.
> +        * For an inline fh and inline file name, copy through stack to exclude
> +        * the copy from usercopy hardening protections.
>          */
>         data = fanotify_fid_fh(fid, fh_len);
>         if (fh_len <= FANOTIFY_INLINE_FH_LEN) {
> @@ -253,14 +282,33 @@ static int copy_fid_to_user(__kernel_fsid_t *fsid, struct fanotify_fid_hdr *fh,
>         if (copy_to_user(buf, data, fh_len))
>                 return -EFAULT;
>
> -       /* Pad with 0's */
>         buf += fh_len;
>         len -= fh_len;
> +
> +       if (name_len) {
> +               /* Copy the filename with terminating null */
> +               name_len++;
> +               if (WARN_ON_ONCE(len < name_len))
> +                       return -EFAULT;
> +
> +               data = name->name;
> +               if (name_len <= DNAME_INLINE_LEN) {
> +                       memcpy(bounce, data, name_len);
> +                       data = bounce;
> +               }
> +               if (copy_to_user(buf, data, name_len))
> +                       return -EFAULT;
> +
> +               buf += name_len;
> +               len -= name_len;
> +       }
> +
> +       /* Pad with 0's */
>         WARN_ON_ONCE(len < 0 || len >= FANOTIFY_EVENT_ALIGN);
>         if (len > 0 && clear_user(buf, len))
>                 return -EFAULT;
>
> -       return 0;
> +       return info_len;
>  }
>
>  static ssize_t copy_event_to_user(struct fsnotify_group *group,
> @@ -282,12 +330,12 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>         metadata.mask = event->mask & FANOTIFY_OUTGOING_EVENTS;
>         metadata.pid = pid_vnr(event->pid);
>
> -       if (fanotify_event_has_path(event)) {
> +       if (FAN_GROUP_FLAG(group, FAN_REPORT_FID)) {
> +               metadata.event_len += fanotify_event_info_len(event);
> +       } else if (fanotify_event_has_path(event)) {
>                 fd = create_fd(group, event, &f);
>                 if (fd < 0)
>                         return fd;
> -       } else if (fanotify_event_has_fid(event)) {
> -               metadata.event_len += fanotify_event_info_len(event);
>         }
>         metadata.fd = fd;
>
> @@ -302,16 +350,36 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>         if (copy_to_user(buf, &metadata, FAN_EVENT_METADATA_LEN))
>                 goto out_close_fd;
>
> +       buf += FAN_EVENT_METADATA_LEN;
> +       count -= FAN_EVENT_METADATA_LEN;
> +
>         if (fanotify_is_perm_event(event->mask))
>                 FANOTIFY_PE(fsn_event)->fd = fd;
>
> -       if (fanotify_event_has_path(event)) {
> +       if (f)
>                 fd_install(fd, f);
> -       } else if (fanotify_event_has_fid(event)) {
> -               ret = copy_fid_to_user(&event->fsid, &event->fh, &event->fid,
> -                                      buf + FAN_EVENT_METADATA_LEN);
> +
> +       /* Event info records order is: dir fid + name, child fid */
> +       if (fanotify_event_has_dfid_name(event)) {
> +               struct fanotify_name_event *fne = FANOTIFY_NE(fsn_event);
> +
> +               ret = copy_info_to_user(&event->fsid, &event->dfh, &fne->dfid,
> +                                       &fne->name, buf, count);
>                 if (ret < 0)
>                         return ret;
> +
> +               buf += ret;
> +               count -= ret;
> +       }
> +
> +       if (fanotify_event_has_fid(event)) {
> +               ret = copy_info_to_user(&event->fsid, &event->fh, &event->fid,
> +                                       NULL, buf, count);
> +               if (ret < 0)
> +                       return ret;
> +
> +               buf += ret;
> +               count -= ret;
>         }
>
>         return metadata.event_len;
> diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
> index b79fa9bb7359..3049a6c06d9e 100644
> --- a/include/linux/fanotify.h
> +++ b/include/linux/fanotify.h
> @@ -47,7 +47,8 @@
>   * Directory entry modification events - reported only to directory
>   * where entry is modified and not to a watching parent.
>   */
> -#define FANOTIFY_DIRENT_EVENTS (FAN_MOVE | FAN_CREATE | FAN_DELETE)
> +#define FANOTIFY_DIRENT_EVENTS (FAN_MOVE | FAN_CREATE | FAN_DELETE | \
> +                                FAN_DIR_MODIFY)
>
>  /* Events that can only be reported with data type FSNOTIFY_EVENT_INODE */
>  #define FANOTIFY_INODE_EVENTS  (FANOTIFY_DIRENT_EVENTS | \
> diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
> index 615fa2c87179..2b56e194b858 100644
> --- a/include/uapi/linux/fanotify.h
> +++ b/include/uapi/linux/fanotify.h
> @@ -117,6 +117,7 @@ struct fanotify_event_metadata {
>  };
>
>  #define FAN_EVENT_INFO_TYPE_FID                1
> +#define FAN_EVENT_INFO_TYPE_DFID_NAME  2
>
>  /* Variable length info record following event metadata */
>  struct fanotify_event_info_header {
> --
> 2.17.1
>


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

  parent reply	other threads:[~2020-04-16 12:17 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-17 13:14 [PATCH v2 00/16] Fanotify event with name info Amir Goldstein
2020-02-17 13:14 ` [PATCH v2 01/16] fsnotify: tidy up FS_ and FAN_ constants Amir Goldstein
2020-02-17 13:14 ` [PATCH v2 02/16] fsnotify: factor helpers fsnotify_dentry() and fsnotify_file() Amir Goldstein
2020-02-25 13:46   ` Jan Kara
2020-02-25 14:27     ` Amir Goldstein
2020-02-26 13:59       ` Jan Kara
2020-02-17 13:14 ` [PATCH v2 03/16] fsnotify: funnel all dirent events through fsnotify_name() Amir Goldstein
2020-02-17 13:14 ` [PATCH v2 04/16] fsnotify: use helpers to access data by data_type Amir Goldstein
2020-02-17 13:14 ` [PATCH v2 05/16] fsnotify: simplify arguments passing to fsnotify_parent() Amir Goldstein
2020-02-19 10:50   ` kbuild test robot
2020-02-19 10:50     ` kbuild test robot
2020-02-19 11:11   ` Amir Goldstein
2020-02-17 13:14 ` [PATCH v2 06/16] fsnotify: pass dentry instead of inode for events possible on child Amir Goldstein
2020-02-17 13:14 ` [PATCH v2 07/16] fsnotify: replace inode pointer with tag Amir Goldstein
2020-02-26  8:20   ` Jan Kara
2020-02-26  9:34     ` Amir Goldstein
2020-02-26  8:52   ` Jan Kara
2020-02-17 13:14 ` [PATCH v2 08/16] fanotify: merge duplicate events on parent and child Amir Goldstein
2020-02-26  9:18   ` Jan Kara
2020-02-26 12:14     ` Amir Goldstein
2020-02-26 14:38       ` Jan Kara
2021-01-22 13:59         ` fanotify_merge improvements Amir Goldstein
2021-01-23 13:30           ` Amir Goldstein
2021-01-25 13:01             ` Jan Kara
2021-01-26 16:21               ` Amir Goldstein
2021-01-27 11:24                 ` Jan Kara
2021-01-27 12:57                   ` Amir Goldstein
2021-01-27 15:15                     ` Jan Kara
2021-01-27 18:03                       ` Amir Goldstein
2021-01-28 10:27                         ` Jan Kara
2021-01-28 18:50                           ` Amir Goldstein
2020-02-17 13:14 ` [PATCH v2 09/16] fanotify: fix merging marks masks with FAN_ONDIR Amir Goldstein
2020-02-17 13:14 ` [PATCH v2 10/16] fanotify: send FAN_DIR_MODIFY event flavor with dir inode and name Amir Goldstein
2020-02-17 13:14 ` [PATCH v2 11/16] fanotify: prepare to encode both parent and child fid's Amir Goldstein
2020-02-26 10:23   ` Jan Kara
2020-02-26 11:53     ` Amir Goldstein
2020-02-26 17:07       ` Jan Kara
2020-02-26 17:50         ` Amir Goldstein
2020-02-27  9:06           ` Amir Goldstein
2020-02-27 11:27             ` Jan Kara
2020-02-27 12:12               ` Amir Goldstein
2020-02-27 13:30                 ` Jan Kara
2020-02-27 14:06                   ` Amir Goldstein
2020-03-01 16:26                     ` Amir Goldstein
2020-03-05 15:49                       ` Jan Kara
2020-03-06 11:19                         ` Amir Goldstein
2020-03-08  7:29                           ` Amir Goldstein
2020-03-18 17:51                             ` Jan Kara
2020-03-18 18:50                               ` Amir Goldstein
2020-03-19  9:30                                 ` Jan Kara
2020-03-19 10:07                                   ` Amir Goldstein
2020-03-30 19:29                                 ` Amir Goldstein
2020-02-27 11:01           ` Jan Kara
2020-02-17 13:14 ` [PATCH v2 12/16] fanotify: record name info for FAN_DIR_MODIFY event Amir Goldstein
2020-02-17 13:14 ` [PATCH v2 13/16] fanotify: report " Amir Goldstein
2020-02-19  9:43   ` kbuild test robot
2020-02-19  9:43     ` kbuild test robot
2020-02-19 10:17   ` kbuild test robot
2020-02-19 10:17     ` kbuild test robot
2020-02-19 11:22   ` Amir Goldstein
2020-04-16 12:16   ` Michael Kerrisk (man-pages) [this message]
2020-04-20 15:53     ` Jan Kara
2020-04-20 18:45     ` Amir Goldstein
2020-04-20 18:47       ` Michael Kerrisk (man-pages)
2020-02-17 13:14 ` [PATCH v2 14/16] fanotify: report parent fid + name with FAN_REPORT_NAME Amir Goldstein
2020-02-17 13:14 ` [PATCH v2 15/16] fanotify: refine rules for when name is reported Amir Goldstein
2020-02-17 13:14 ` [BONUS][PATCH v2 16/16] fanotify: support limited functionality for unprivileged users Amir Goldstein
2020-02-20 22:10 ` [PATCH v2 00/16] Fanotify event with name info Matthew Bobrowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKgNAkiVcjQfATKWwGNPDFucMEN4jJnQ5q6JHRzDihK1ZDnH8A@mail.gmail.com \
    --to=mtk.manpages@gmail.com \
    --cc=amir73il@gmail.com \
    --cc=jack@suse.cz \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.