Re: [PATCH v3] fsnotify: Fix stale object mask after concurrent mark updates

On Wed, Aug 5, 2026 at 11:54 AM Jan Kara <[email protected]> wrote:
>
> On Sun 02-08-26 10:58:00, Youngjae Kwon wrote:
> > When a mark gets a new event bit, fanotify and inotify may avoid
> > recalculating the object mask if the cached aggregate already contains that
> > bit. This is racy with a recalculation triggered by a concurrent update to
> > another mark on the same connector.
> >
> > The concurrent scan can read the mark before the new bit is added, while
> > the updater reads the old aggregate before that scan publishes its result.
> > The updater then skips recalculation and the scan publishes a mask without
> > the bit, leaving the object mask stale after both updates complete.
> >
> > This can be reproduced with two fanotify groups watching the same inode:
> > one thread removes FAN_MODIFY from one existing mark while another thread
> > adds FAN_MODIFY to the other mark. After both fanotify_mark() calls return,
> > writes can fail to produce FAN_MODIFY for the group whose mark now contains
> > the bit. This was reproduced on an unmodified v6.12.95 kernel. The
> > equivalent inotify interleaving loses IN_MODIFY events.
> >
> > For normal fanotify additions, recalculate whenever the raw mark mask
> > changes. The normal mask is not cleared asynchronously, so an unchanged
> > addition cannot introduce missing interest. Always recalculate ignore-mask
> > updates because FS_MODIFY handling may clear the ignore mask without taking
> > mark->lock, making snapshot comparisons unreliable.
> >
> > Always recalculate after updating an existing inotify watch. Its replace
> > path temporarily sets mark->mask to zero, so a concurrent scan can observe
> > zero even when the old and final masks are equal. Assigning the replacement
> > mask directly would avoid the transient zero, but existing-watch updates
> > are infrequent, so unconditional recalculation is simpler.
> >
> > Link: https://lore.kernel.org/all/CACwKKmCZdiZDoFuYm6LZhQ=XvHPk0fNKH=X3LmoXMqakYqJaNw@mail.gmail.com/
> > Fixes: 63c882a05416 ("inotify: reimplement inotify using fsnotify")
> > Fixes: 912ee3946c5e ("fanotify: do not call fanotify_update_object_mask in fanotify_add_mark")
> > Cc: [email protected] # needs adjustments for <= 7.0
> > Suggested-by: Jan Kara <[email protected]>
> > Suggested-by: Amir Goldstein <[email protected]>
> > Signed-off-by: Youngjae Kwon <[email protected]>
>
> Thanks the patch looks good to me, I've added it to my tree. Amir, are you
> happy with the changes like this?

Sorry for the late reply.
Was on vacation.
Yes, this seems fine to me.

Thanks,
Amir.


>
>                                                                 Honza
>
> > ---
> > Changes in v3:
> > - Preserve the normal fanotify duplicate-add fast path by recalculating
> >   only when the raw normal mask actually changes.
> > - Recalculate fanotify ignore-mask updates unconditionally because the
> >   ignore mask may be cleared concurrently without mark->lock.
> > - Keep v2's unconditional recalculation for existing inotify watch updates.
> >
> > I will send backports for stable kernels through 7.0.y after this lands.
> >
> >  fs/notify/fanotify/fanotify_user.c | 12 +++++++-----
> >  fs/notify/inotify/inotify_user.c   | 15 +--------------
> >  2 files changed, 8 insertions(+), 19 deletions(-)
> >
> > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> > index 9ee373ff5..9c27db2b7 100644
> > --- a/fs/notify/fanotify/fanotify_user.c
> > +++ b/fs/notify/fanotify/fanotify_user.c
> > @@ -1321,16 +1321,18 @@ static bool fanotify_mark_update_flags(struct fsnotify_mark *fsn_mark,
> >  static bool fanotify_mark_add_to_mask(struct fsnotify_mark *fsn_mark,
> >                                     __u32 mask, unsigned int fan_flags)
> >  {
> > +     __u32 old_mask;
> >       bool recalc;
> >
> >       spin_lock(&fsn_mark->lock);
> > -     if (!(fan_flags & FANOTIFY_MARK_IGNORE_BITS))
> > +     if (!(fan_flags & FANOTIFY_MARK_IGNORE_BITS)) {
> > +             old_mask = fsn_mark->mask;
> >               fsn_mark->mask |= mask;
> > -     else
> > +             recalc = old_mask != fsn_mark->mask;
> > +     } else {
> >               fsn_mark->ignore_mask |= mask;
> > -
> > -     recalc = fsnotify_calc_mask(fsn_mark) &
> > -             ~fsnotify_conn_mask(fsn_mark->connector);
> > +             recalc = true;
> > +     }
> >
> >       recalc |= fanotify_mark_update_flags(fsn_mark, fan_flags);
> >       spin_unlock(&fsn_mark->lock);
> > diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
> > index ed37491c1..5f19c24ec 100644
> > --- a/fs/notify/inotify/inotify_user.c
> > +++ b/fs/notify/inotify/inotify_user.c
> > @@ -539,7 +539,6 @@ static int inotify_update_existing_watch(struct fsnotify_group *group,
> >  {
> >       struct fsnotify_mark *fsn_mark;
> >       struct inotify_inode_mark *i_mark;
> > -     __u32 old_mask, new_mask;
> >       int replace = !(arg & IN_MASK_ADD);
> >       int create = (arg & IN_MASK_CREATE);
> >       int ret;
> > @@ -555,27 +554,15 @@ static int inotify_update_existing_watch(struct fsnotify_group *group,
> >       i_mark = container_of(fsn_mark, struct inotify_inode_mark, fsn_mark);
> >
> >       spin_lock(&fsn_mark->lock);
> > -     old_mask = fsn_mark->mask;
> >       if (replace) {
> >               fsn_mark->mask = 0;
> >               fsn_mark->flags &= ~INOTIFY_MARK_FLAGS;
> >       }
> >       fsn_mark->mask |= inotify_arg_to_mask(inode, arg);
> >       fsn_mark->flags |= inotify_arg_to_flags(arg);
> > -     new_mask = fsn_mark->mask;
> >       spin_unlock(&fsn_mark->lock);
> >
> > -     if (old_mask != new_mask) {
> > -             /* more bits in old than in new? */
> > -             int dropped = (old_mask & ~new_mask);
> > -             /* more bits in this fsn_mark than the inode's mask? */
> > -             int do_inode = (new_mask & ~READ_ONCE(inode->i_fsnotify_mask));
> > -
> > -             /* update the inode with this new fsn_mark */
> > -             if (dropped || do_inode)
> > -                     fsnotify_recalc_mask(fsn_mark->connector);
> > -
> > -     }
> > +     fsnotify_recalc_mask(fsn_mark->connector);
> >
> >       /* return the wd */
> >       ret = i_mark->wd;
> > --
> > 2.50.1
> --
> Jan Kara <[email protected]>
> SUSE Labs, CR