From: Amir Goldstein <amir73il@gmail.com>
To: Jan Kara <jack@suse.cz>
Cc: Christian Brauner <brauner@kernel.org>, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 2/2] fanotify: allow "weak" fsid when watching a single filesystem
Date: Thu, 30 Nov 2023 14:42:29 +0200 [thread overview]
Message-ID: <CAOQ4uxhrPofk==ASam-uV0=JKSff=YEJm_VUuWw_PpAAi9YYFw@mail.gmail.com> (raw)
In-Reply-To: <CAOQ4uxgfyAcD_pZ8egQuEoiNstgrD8E0YPDps5Am=St9jY6rXw@mail.gmail.com>
On Tue, Nov 21, 2023 at 3:33 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Mon, Nov 20, 2023 at 9:42 AM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > On Sat, Nov 18, 2023 at 8:30 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > So far, fanotify returns -ENODEV or -EXDEV when trying to set a mark
> > > on a filesystem with a "weak" fsid, namely, zero fsid (e.g. fuse), or
> > > non-uniform fsid (e.g. btrfs non-root subvol).
> > >
> > > When group is watching inodes all from the same filesystem (or subvol),
> > > allow adding inode marks with "weak" fsid, because there is no ambiguity
> > > regarding which filesystem reports the event.
> > >
> > > The first mark added to a group determines if this group is single or
> > > multi filesystem, depending on the fsid at the path of the added mark.
> > >
> > > If the first mark added has a "strong" fsid, marks with "weak" fsid
> > > cannot be added and vice versa.
> > >
> > > If the first mark added has a "weak" fsid, following marks must have
> > > the same "weak" fsid and the same sb as the first mark.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---
> > > fs/notify/fanotify/fanotify.c | 15 +----
> > > fs/notify/fanotify/fanotify.h | 6 ++
> > > fs/notify/fanotify/fanotify_user.c | 97 ++++++++++++++++++++++++------
> > > include/linux/fsnotify_backend.h | 1 +
> > > 4 files changed, 90 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> > > index aff1ab3c32aa..1e4def21811e 100644
> > > --- a/fs/notify/fanotify/fanotify.c
> > > +++ b/fs/notify/fanotify/fanotify.c
> > > @@ -29,12 +29,6 @@ static unsigned int fanotify_hash_path(const struct path *path)
> > > hash_ptr(path->mnt, FANOTIFY_EVENT_HASH_BITS);
> > > }
> > >
> > > -static inline bool fanotify_fsid_equal(__kernel_fsid_t *fsid1,
> > > - __kernel_fsid_t *fsid2)
> > > -{
> > > - return fsid1->val[0] == fsid2->val[0] && fsid1->val[1] == fsid2->val[1];
> > > -}
> > > -
> > > static unsigned int fanotify_hash_fsid(__kernel_fsid_t *fsid)
> > > {
> > > return hash_32(fsid->val[0], FANOTIFY_EVENT_HASH_BITS) ^
> > > @@ -851,7 +845,8 @@ static __kernel_fsid_t fanotify_get_fsid(struct fsnotify_iter_info *iter_info)
> > > if (!(mark->flags & FSNOTIFY_MARK_FLAG_HAS_FSID))
> > > continue;
> > > fsid = FANOTIFY_MARK(mark)->fsid;
> > > - if (WARN_ON_ONCE(!fsid.val[0] && !fsid.val[1]))
> > > + if (!(mark->flags & FSNOTIFY_MARK_FLAG_WEAK_FSID) &&
> > > + WARN_ON_ONCE(!fsid.val[0] && !fsid.val[1]))
> > > continue;
> > > return fsid;
> > > }
> > > @@ -933,12 +928,8 @@ static int fanotify_handle_event(struct fsnotify_group *group, u32 mask,
> > > return 0;
> > > }
> > >
> > > - if (FAN_GROUP_FLAG(group, FANOTIFY_FID_BITS)) {
> > > + if (FAN_GROUP_FLAG(group, FANOTIFY_FID_BITS))
> > > fsid = fanotify_get_fsid(iter_info);
> > > - /* Racing with mark destruction or creation? */
> > > - if (!fsid.val[0] && !fsid.val[1])
> > > - return 0;
> > > - }
> > >
> > > event = fanotify_alloc_event(group, mask, data, data_type, dir,
> > > file_name, &fsid, match_mask);
> > > diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
> > > index 2847fa564298..9c97ae638ac5 100644
> > > --- a/fs/notify/fanotify/fanotify.h
> > > +++ b/fs/notify/fanotify/fanotify.h
> > > @@ -504,6 +504,12 @@ static inline __kernel_fsid_t *fanotify_mark_fsid(struct fsnotify_mark *mark)
> > > return &FANOTIFY_MARK(mark)->fsid;
> > > }
> > >
> > > +static inline bool fanotify_fsid_equal(__kernel_fsid_t *fsid1,
> > > + __kernel_fsid_t *fsid2)
> > > +{
> > > + return fsid1->val[0] == fsid2->val[0] && fsid1->val[1] == fsid2->val[1];
> > > +}
> > > +
> > > static inline unsigned int fanotify_mark_user_flags(struct fsnotify_mark *mark)
> > > {
> > > unsigned int mflags = 0;
> > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> > > index e3d836d4d156..1cc68ea56c17 100644
> > > --- a/fs/notify/fanotify/fanotify_user.c
> > > +++ b/fs/notify/fanotify/fanotify_user.c
> > > @@ -23,7 +23,7 @@
> > >
> > > #include <asm/ioctls.h>
> > >
> > > -#include "../../mount.h"
> > > +#include "../fsnotify.h"
> > > #include "../fdinfo.h"
> > > #include "fanotify.h"
> > >
> > > @@ -1192,11 +1192,68 @@ static bool fanotify_mark_add_to_mask(struct fsnotify_mark *fsn_mark,
> > > return recalc;
> > > }
> > >
> > > +struct fan_fsid {
> > > + struct super_block *sb;
> > > + __kernel_fsid_t id;
> > > + bool weak;
> > > +};
> > > +
> > > +static int fanotify_check_mark_fsid(struct fsnotify_group *group,
> > > + struct fsnotify_mark *mark,
> > > + struct fan_fsid *fsid)
> > > +{
> > > + struct fsnotify_mark_connector *conn;
> > > + struct fsnotify_mark *old;
> > > + struct super_block *old_sb = NULL;
> > > +
> > > + *fanotify_mark_fsid(mark) = fsid->id;
> > > + mark->flags |= FSNOTIFY_MARK_FLAG_HAS_FSID;
> > > + if (fsid->weak)
> > > + mark->flags |= FSNOTIFY_MARK_FLAG_WEAK_FSID;
> > > +
> > > + /* First mark added will determine if group is single or multi fsid */
> > > + if (list_empty(&group->marks_list))
> > > + return 0;
> > > +
> > > + /* Find sb of an existing mark */
> > > + list_for_each_entry(old, &group->marks_list, g_list) {
> > > + conn = READ_ONCE(old->connector);
> > > + if (!conn)
> > > + continue;
> > > + old_sb = fsnotify_connector_sb(conn);
> > > + if (old_sb)
> > > + break;
> > > + }
> > > +
> > > + /* Only detached marks left? */
> > > + if (!old_sb)
> > > + return 0;
> > > +
> > > + /* Do not allow mixing of marks with weak and strong fsid */
> > > + if ((mark->flags ^ old->flags) & FSNOTIFY_MARK_FLAG_WEAK_FSID)
> > > + return -EXDEV;
> > > +
> > > + /* Allow mixing of marks with strong fsid from different fs */
> > > + if (!fsid->weak)
> > > + return 0;
> > > +
> > > + /* Do not allow mixing marks with weak fsid from different fs */
> > > + if (old_sb != fsid->sb)
> > > + return -EXDEV;
> > > +
> > > + /* Do not allow mixing marks from different btrfs sub-volumes */
> > > + if (!fanotify_fsid_equal(fanotify_mark_fsid(old),
> > > + fanotify_mark_fsid(mark)))
> > > + return -EXDEV;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static struct fsnotify_mark *fanotify_add_new_mark(struct fsnotify_group *group,
> > > fsnotify_connp_t *connp,
> > > unsigned int obj_type,
> > > unsigned int fan_flags,
> > > - __kernel_fsid_t *fsid)
> > > + struct fan_fsid *fsid)
> > > {
> > > struct ucounts *ucounts = group->fanotify_data.ucounts;
> > > struct fanotify_mark *fan_mark;
> > > @@ -1225,8 +1282,9 @@ static struct fsnotify_mark *fanotify_add_new_mark(struct fsnotify_group *group,
> > >
> > > /* Cache fsid of filesystem containing the marked object */
> > > if (fsid) {
> > > - fan_mark->fsid = *fsid;
> > > - mark->flags |= FSNOTIFY_MARK_FLAG_HAS_FSID;
> > > + ret = fanotify_check_mark_fsid(group, mark, fsid);
> > > + if (ret)
> >
> > OOPS, missed fsnotify_put_mark(mark);
> > better add a goto target out_put_mark as this is the second case now.
>
> FYI, I pushed the fix to:
> https://github.com/amir73il/linux/commits/fanotify_fsid
>
> Let me know if you want me to post v2 for this.
>
Hi Jan,
Ping.
Reminder, I have LTP tests for testing fs with "weak" fsid [2].
After your LTP patches are merged, I will rebase them.
The way that I tested fs with "weak" fsid is that LTP tests
the ntfs-3g FUSE filesystem with .all_filesystems = 1.
With this work, the tests started to run on FUSE and skip
the test cases of sb/mount marks.
Thanks,
Amir.
[2] https://github.com/amir73il/tlp/commits/fanotify_fsid
next prev parent reply other threads:[~2023-11-30 12:42 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-18 18:30 [PATCH 0/2] Support fanotify FAN_REPORT_FID on all filesystems Amir Goldstein
2023-11-18 18:30 ` [PATCH 1/2] fanotify: store fsid in mark instead of in connector Amir Goldstein
2023-11-30 14:25 ` Jan Kara
2023-11-30 15:29 ` Amir Goldstein
2023-11-30 15:50 ` Jan Kara
2023-11-18 18:30 ` [PATCH 2/2] fanotify: allow "weak" fsid when watching a single filesystem Amir Goldstein
2023-11-20 7:42 ` Amir Goldstein
2023-11-20 15:48 ` Christian Brauner
2023-11-20 16:20 ` Amir Goldstein
2023-11-21 13:33 ` Amir Goldstein
2023-11-30 12:42 ` Amir Goldstein [this message]
2023-11-30 15:47 ` Jan Kara
2023-11-30 16:28 ` Amir Goldstein
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='CAOQ4uxhrPofk==ASam-uV0=JKSff=YEJm_VUuWw_PpAAi9YYFw@mail.gmail.com' \
--to=amir73il@gmail.com \
--cc=brauner@kernel.org \
--cc=jack@suse.cz \
--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.