Re: [BUG] fsnotify: concurrent mark updates can publish a stale aggregate mask and lose events

Hello!

On Thu 30-07-26 00:39:49, 권영재 / 학생 / 전기·정보공학부 wrote:
> This did not come from a real application. It started as an LLM-assisted
> source review, but I only reported it after reproducing the lost
> FAN_MODIFY/IN_MODIFY events on an unmodified v6.12.95 kernel.

OK, thanks for confirmation!

> The fix you describe makes sense to me. Happy to test any patch
> against my reproducer.

Since you wrote: "If either direction looks reasonable, I can prepare a
cleaned-up patch and a regression test for review." my understanding was
you want to write the fix yourself to get the full credit for you work
(which is fair). If not, just let me know I'll write the fix myself.

								Honza

> 2026년 7월 29일 (수) 오후 9:40, Jan Kara <[email protected]>님이 작성:
> >
> > Hi!
> >
> > On Tue 28-07-26 14:32:33, 권영재 / 학생 / 전기·정보공학부 wrote:
> > > Concurrent updates to marks from different groups on the same inode can
> > > leave the object's cached aggregate mask missing an event bit that is
> > > still present in a live mark. An operation started after both mark-update
> > > syscalls have returned can then be filtered by fsnotify_object_watched(),
> > > so the matching fanotify or inotify notification is not delivered.
> > >
> > > I forced this source interleaving with diagnostic scheduling barriers and
> > > reproduced persistent FAN_MODIFY and IN_MODIFY loss in 3/3 independent
> > > boots for each interface. A control update that caused a fresh aggregate
> > > scan restored delivery. I also observed the same loss-and-repair outcome
> > > without kernel instrumentation on an unmodified v6.12.95 kernel image in
> > > 3/3 independent boots, with 9 strict hits in 30,000 attempts. I have not
> > > identified a memory-safety issue or a security-boundary crossing; I am
> > > reporting this as a notification correctness bug.
> >
> > Thanks for the report. To clarify is this a result of a code review
> > (personal or by LLM) or some real application was hitting the problem?
> >
> > Also for the next time I'd like to ask you to try to keep your report a bit
> > more condensed (either by instructing LLM or by selecting the substantial
> > content from LLM output). E.g. in this case the above two paragraphs
> > explain substance of the problem and reproducibility, the scheme in "The
> > race" section shows how the problem happens and the rest is mostly just
> > uninteresting noise so this report could have been less than two screens
> > worth of text whereas now it is over 7 screens in my terminal.
> >
> > Now to the problem:
> >
> > > The race
> > > ========
> > >
> > > Let P and E be marks from different groups on one inode. P initially
> > > supplies event bit X to the cached aggregate, while E does not contain X.
> > >
> > >   P update/removal                 E update/add
> > >   ----------------                ---------------------------------
> > >   remove X from P
> > >   start connector recalculation
> > >   scan E without X
> > >                                   add X to E
> > >                                   observe old aggregate containing X
> > >                                   decide recalculation is redundant
> > >   publish the older scan without X
> > >   both syscalls return
> > >
> > > The add-side decisions are not serialized with connector->lock:
> > >
> > >   fanotify_mark_add_to_mask():
> > >
> > >     recalc = fsnotify_calc_mask(fsn_mark) &
> > >              ~fsnotify_conn_mask(fsn_mark->connector);
> > >
> > >   inotify_update_existing_watch():
> > >
> > >     dropped = old_mask & ~new_mask;
> > >     do_inode = new_mask & ~READ_ONCE(inode->i_fsnotify_mask);
> > >     if (dropped || do_inode)
> > >             fsnotify_recalc_mask(...);
> > >
> > > __fsnotify_recalc_mask() scans under connector->lock and later publishes
> > > the sampled union with WRITE_ONCE(). A different mark can be mutated after
> > > it has been sampled. Its add-side redundancy check can still observe the
> > > old aggregate, which the in-flight scan then overwrites.
> > >
> > > After this ordering, E contains X but the aggregate does not. The state
> > > persists until another operation causes a full recalculation.
> >
> > Yeah, right. So for simplicity I'd just solve this by always calling
> > fsnotify_recalc_mask() whenever mark->mask is changed. We *could* keep the
> > current behavior by providing new mark->mask to fsnotify_recalc_mask() when
> > adding events and then doing the check under conn->lock or even improve on
> > it and completely avoid scanning the whole conn->list and just add the new
> > mask to the current connector mask. But at this point I think it is a
> > premature optimization - we don't expect many notification groups to be
> > watching the same inode and frequently adding marks to it. So let's keep
> > things simple for now.
> >
> >                                                                 Honza
> > --
> > Jan Kara <[email protected]>
> > SUSE Labs, CR
> >
> >
-- 
Jan Kara <[email protected]>
SUSE Labs, CR