[BUG] fanotify: destroy/add race leaves a mark on a detached connector

[BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Daehyeon Ko]

Hello,

I found an unprivileged race in fsnotify_destroy_marks() that can leave an
attached fanotify mark on a detached connector. On unmodified Linux v7.2 and
v6.12.105 source builds, it naturally reaches the fsnotify_conn_mask()
warning from fanotify_mark(). On unmodified builds, the only observed
system-wide failure was a v7.2 panic when panic_on_warn=1; no memory
corruption or privilege escalation was observed.

Tested and source-inspected versions:

  - Linux v7.2, commit
    8d3ae59288f1e7d58d76558a6ee96d533bc5019f: reproduced.
  - Linux v6.12.105, commit
    14c37ff05f22da2fa7076d10f6a07c7ede330c83: reproduced.
  - Torvalds master at
    73e3f0710014fe6d4ed98cfc02292f6121db7558: the relevant destroy
    iterator and fdinfo consumer remain present. Commit e422777fdd47
    changed mark-mask updates and can mask the immediate warning, but it
    did not change this iterator or the detached-connector state.

The unconditional connector detachment involved in the race appears to
originate in commit 6b3f05d24d35 ("fsnotify: Detach mark from object list
when last reference is dropped"), released in v4.12-rc1. My unprivileged
runtime results are limited to the two versions listed above. The limited
unprivileged fanotify functionality used by this reproducer was introduced
by 7cea2a3c505e ("fanotify: support limited functionality for unprivileged
users"), released in v5.13-rc1; I am not asserting ordinary-user
reachability before that change.

Root cause:

fsnotify_destroy_marks() holds a reference to the current mark, drops
conn->lock, destroys that mark, and then continues the hlist walk. While the
lock is dropped, an equal-priority mark can be inserted immediately before
the current entry. The walk resumes from the current entry's next pointer,
so it never visits the new predecessor, but it still detaches the connector
from the object after the walk.

The skipped mark remains ALIVE|ATTACHED and remains on both its group list
and the connector list. At the same time, the object no longer points to the
connector, conn->obj is NULL, and conn->type is DETACHED.

Representative v6.12.105 output is:

  WARNING: CPU: 1 PID: 163 at fs/notify/mark.c:128
  fsnotify_conn_mask+0x113/0x150
  CPU: 1 UID: 65534 PID: 163 Comm: fanotify_destro
  ...
  do_fanotify_mark
  __x64_sys_fanotify_mark

The source reproducer ran on ext4 as UID/GID 65534 with CapEff=0 and
NoNewPrivs=1. It uses ordinary FAN_REPORT_FID groups and races final unlink
against fd-based mark insertion. Both builds used a KASAN-enabled research
configuration, but neither positive was a KASAN report.

With panic_on_warn=0 and panic_on_oops=0, the unmodified v7.2 run emitted the
warning and then completed 10,000 iterations and powered off cleanly. A
separate fresh v7.2 run with panic_on_warn=1 reached the same warning and
panicked through check_panic_on_warn(). The v6.12.105 run emitted seven
warnings and reached its 10,000-iteration progress line; it later timed out
during post-loop teardown, for which I am not assigning a kernel cause.

A timing-only diagnostic kernel also demonstrated that fdinfo can combine
the old INODE type with the detached NULL object and reach
fanotify_show_fdinfo() -> igrab(NULL). I have not reproduced that Oops on an
unmodified kernel, so it is not part of the impact claim. The skipped mark's
group-list reference keeps the connector allocated; I have no evidence of a
connector UAF, arbitrary read or write, information disclosure, or LPE.

I do not have a submission-ready patch. During fix development I rejected a
survivor-preservation prototype because it introduced a mask race, a
drain/restart prototype because additions could starve cleanup, and a bounded
retry prototype because review found an unresolved lock-order/deadlock risk
when generic callers hold external locks needed by cleanup. I am reporting
the root cause rather than sending an unsafe fix.

A tested source reproducer, full serial logs, configs, and diagnostic details
are available privately to the maintainers on request. AI-assisted tooling
was used during discovery and analysis; I reviewed the source and the literal
runtime evidence above and am reporting publicly without the reproducer as
required by Documentation/process/security-bugs.rst.

Assisted-by: LLM

Thanks,
Daehyeon Ko

Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Amir Goldstein]

On Thu, Aug 27, 2026 at 6:50 AM Daehyeon Ko <4ncienth@gmail.com> wrote:
>
> Hello,
>
> I found an unprivileged race in fsnotify_destroy_marks() that can leave an
> attached fanotify mark on a detached connector. On unmodified Linux v7.2 and
> v6.12.105 source builds, it naturally reaches the fsnotify_conn_mask()
> warning from fanotify_mark(). On unmodified builds, the only observed
> system-wide failure was a v7.2 panic when panic_on_warn=1; no memory
> corruption or privilege escalation was observed.
>
> Tested and source-inspected versions:
>
>   - Linux v7.2, commit
>     8d3ae59288f1e7d58d76558a6ee96d533bc5019f: reproduced.
>   - Linux v6.12.105, commit
>     14c37ff05f22da2fa7076d10f6a07c7ede330c83: reproduced.
>   - Torvalds master at
>     73e3f0710014fe6d4ed98cfc02292f6121db7558: the relevant destroy
>     iterator and fdinfo consumer remain present. Commit e422777fdd47
>     changed mark-mask updates and can mask the immediate warning, but it
>     did not change this iterator or the detached-connector state.
>
> The unconditional connector detachment involved in the race appears to
> originate in commit 6b3f05d24d35 ("fsnotify: Detach mark from object list
> when last reference is dropped"), released in v4.12-rc1. My unprivileged
> runtime results are limited to the two versions listed above. The limited
> unprivileged fanotify functionality used by this reproducer was introduced
> by 7cea2a3c505e ("fanotify: support limited functionality for unprivileged
> users"), released in v5.13-rc1; I am not asserting ordinary-user
> reachability before that change.
>
> Root cause:
>
> fsnotify_destroy_marks() holds a reference to the current mark, drops
> conn->lock, destroys that mark, and then continues the hlist walk. While the
> lock is dropped, an equal-priority mark can be inserted immediately before
> the current entry. The walk resumes from the current entry's next pointer,
> so it never visits the new predecessor, but it still detaches the connector
> from the object after the walk.
>
> The skipped mark remains ALIVE|ATTACHED and remains on both its group list
> and the connector list. At the same time, the object no longer points to the
> connector, conn->obj is NULL, and conn->type is DETACHED.
>
> Representative v6.12.105 output is:
>
>   WARNING: CPU: 1 PID: 163 at fs/notify/mark.c:128
>   fsnotify_conn_mask+0x113/0x150
>   CPU: 1 UID: 65534 PID: 163 Comm: fanotify_destro
>   ...
>   do_fanotify_mark
>   __x64_sys_fanotify_mark
>
> The source reproducer ran on ext4 as UID/GID 65534 with CapEff=0 and
> NoNewPrivs=1. It uses ordinary FAN_REPORT_FID groups and races final unlink
> against fd-based mark insertion. Both builds used a KASAN-enabled research
> configuration, but neither positive was a KASAN report.
>
> With panic_on_warn=0 and panic_on_oops=0, the unmodified v7.2 run emitted the
> warning and then completed 10,000 iterations and powered off cleanly. A
> separate fresh v7.2 run with panic_on_warn=1 reached the same warning and
> panicked through check_panic_on_warn(). The v6.12.105 run emitted seven
> warnings and reached its 10,000-iteration progress line; it later timed out
> during post-loop teardown, for which I am not assigning a kernel cause.
>
> A timing-only diagnostic kernel also demonstrated that fdinfo can combine
> the old INODE type with the detached NULL object and reach
> fanotify_show_fdinfo() -> igrab(NULL). I have not reproduced that Oops on an
> unmodified kernel, so it is not part of the impact claim. The skipped mark's
> group-list reference keeps the connector allocated; I have no evidence of a
> connector UAF, arbitrary read or write, information disclosure, or LPE.
>
> I do not have a submission-ready patch. During fix development I rejected a
> survivor-preservation prototype because it introduced a mask race, a
> drain/restart prototype because additions could starve cleanup, and a bounded
> retry prototype because review found an unresolved lock-order/deadlock risk
> when generic callers hold external locks needed by cleanup. I am reporting
> the root cause rather than sending an unsafe fix.
>
> A tested source reproducer, full serial logs, configs, and diagnostic details
> are available privately to the maintainers on request. AI-assisted tooling
> was used during discovery and analysis; I reviewed the source and the literal
> runtime evidence above and am reporting publicly without the reproducer as
> required by Documentation/process/security-bugs.rst.
>

Hi Daehyeon,

Thanks for the detailed report.

Without getting into discussions on solutions to the race, do we even care
that a detached connector has a leftover attached mark in this tiny corner case?

I mean the assertion was added because we did not anticipate it,
but now that we understand that it can happen, the assertion could be removed
or relaxed if there are no other major consequences.

Regarding the reproduced igrab(NULL), this could be fixed by
using a helper like this in show fdino code:

static inline struct inode *fsnotify_conn_get_inode_safe(struct
fsnotify_mark_connector *conn)
{
        struct inode *inode = conn->obj;

        return inode ? igrab(inode) : NULL;
}

WDYT?

Thanks,
Amir.

Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Jan Kara]

Hello!

On Thu 27-08-26 13:50:04, Daehyeon Ko wrote:
> I found an unprivileged race in fsnotify_destroy_marks() that can leave an
> attached fanotify mark on a detached connector. On unmodified Linux v7.2 and
> v6.12.105 source builds, it naturally reaches the fsnotify_conn_mask()
> warning from fanotify_mark(). On unmodified builds, the only observed
> system-wide failure was a v7.2 panic when panic_on_warn=1; no memory
> corruption or privilege escalation was observed.
> 
> Tested and source-inspected versions:
> 
>   - Linux v7.2, commit
>     8d3ae59288f1e7d58d76558a6ee96d533bc5019f: reproduced.
>   - Linux v6.12.105, commit
>     14c37ff05f22da2fa7076d10f6a07c7ede330c83: reproduced.
>   - Torvalds master at
>     73e3f0710014fe6d4ed98cfc02292f6121db7558: the relevant destroy
>     iterator and fdinfo consumer remain present. Commit e422777fdd47
>     changed mark-mask updates and can mask the immediate warning, but it
>     did not change this iterator or the detached-connector state.
> 
> The unconditional connector detachment involved in the race appears to
> originate in commit 6b3f05d24d35 ("fsnotify: Detach mark from object list
> when last reference is dropped"), released in v4.12-rc1. My unprivileged
> runtime results are limited to the two versions listed above. The limited
> unprivileged fanotify functionality used by this reproducer was introduced
> by 7cea2a3c505e ("fanotify: support limited functionality for unprivileged
> users"), released in v5.13-rc1; I am not asserting ordinary-user
> reachability before that change.
> 
> Root cause:
> 
> fsnotify_destroy_marks() holds a reference to the current mark, drops
> conn->lock, destroys that mark, and then continues the hlist walk. While the
> lock is dropped, an equal-priority mark can be inserted immediately before
> the current entry. The walk resumes from the current entry's next pointer,
> so it never visits the new predecessor, but it still detaches the connector
> from the object after the walk.
> 
> The skipped mark remains ALIVE|ATTACHED and remains on both its group list
> and the connector list. At the same time, the object no longer points to the
> connector, conn->obj is NULL, and conn->type is DETACHED.
> 
> Representative v6.12.105 output is:
> 
>   WARNING: CPU: 1 PID: 163 at fs/notify/mark.c:128
>   fsnotify_conn_mask+0x113/0x150
>   CPU: 1 UID: 65534 PID: 163 Comm: fanotify_destro
>   ...
>   do_fanotify_mark
>   __x64_sys_fanotify_mark
> 
> The source reproducer ran on ext4 as UID/GID 65534 with CapEff=0 and
> NoNewPrivs=1. It uses ordinary FAN_REPORT_FID groups and races final unlink
> against fd-based mark insertion.

Thanks for the detailed report. Before discussing possible solutions I'd
like to better understand how the race happens because so far that's
unclear to me. You mention you are racing final unlink with fd-based mark
insertion - so I presume we are speaking about inode marks here. Fair.

Now for "final unlink" to reach fsnotify_destroy_marks() it has to go
through:
  d_delete() -> dentry_unlink_inode() -> fsnotify_inoderemove()

But to go through this path, d_delete() has to validate that
dentry->d_lockref.count == 1 which should not be possible if the inode is
still open (and thus the file->f_path holds the dentry reference). What am
I missing here? Which two calls are exactly racing?

FWIW my current suspicion is that calling fanotify_inode_delete() from
fsnotify_inoderemove() is actually too early and we should always leave it
to __destroy_inode() time which would also avoid any races with mark
addition. But the fact that the above race can happen at all makes me
nervous we can be missing some other cornercases...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Daehyeon Ko]

Hi Honza,

You are right that an fd-held dentry cannot satisfy the count == 1 check. I
was imprecise in the report: the fd and the final unlink refer to two
different hard-link dentries for the same inode.

The setup is:

  1. create A/x and link it as B/y;
  2. open B/y, add the old-group inode mark through that fd, then unlink
     B/y, leaving i_nlink == 1 while the fd retains the B/y dentry;
  3. race these two calls:

       unlink("A/x")

       fanotify_mark(racer_group, FAN_MARK_ADD, FAN_MODIFY,
                     victim_fd, NULL)

     where victim_fd still refers to the already unlinked B/y dentry.

So the A/x dentry checked by d_delete() is not held by the fd and can have
d_lockref.count == 1. Its unlink drops the inode's last link and reaches
dentry_unlink_inode() -> fsnotify_inoderemove(). At the same time,
fanotify_find_path() gets the same inode through the fd-held B/y dentry and
adds the racer mark.

Your suspicion matches this topology: __fsnotify_inode_delete() from
fsnotify_inoderemove() is running while the inode is still live and reachable
through the other, fd-held dentry. Deferring mark cleanup until
__destroy_inode() would avoid this particular add-versus-cleanup race. I have
not audited the broader consequences of that change yet.

I can send the exact reproducer privately if that would be useful.

Thanks,
Daehyeon

Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Jan Kara]

Hi!

On Fri 28-08-26 13:24:19, Daehyeon Ko wrote:
> You are right that an fd-held dentry cannot satisfy the count == 1 check. I
> was imprecise in the report: the fd and the final unlink refer to two
> different hard-link dentries for the same inode.
> 
> The setup is:
> 
>   1. create A/x and link it as B/y;
>   2. open B/y, add the old-group inode mark through that fd, then unlink
>      B/y, leaving i_nlink == 1 while the fd retains the B/y dentry;
>   3. race these two calls:
> 
>        unlink("A/x")
> 
>        fanotify_mark(racer_group, FAN_MARK_ADD, FAN_MODIFY,
>                      victim_fd, NULL)
> 
>      where victim_fd still refers to the already unlinked B/y dentry.
> 
> So the A/x dentry checked by d_delete() is not held by the fd and can have
> d_lockref.count == 1. Its unlink drops the inode's last link and reaches
> dentry_unlink_inode() -> fsnotify_inoderemove(). At the same time,
> fanotify_find_path() gets the same inode through the fd-held B/y dentry and
> adds the racer mark.

Thanks for explanation of the reproducer. Now it makes sense. The
conclusion from this is that fsnotify_inoderemove() can indeed be called
for still fully alive open inodes which should be (think of permission /
pre-content events) still generating events. That could be actually
breaking some assumptions of userspace although I don't think it's a
serious issue in practice because users of permission / pre-content watches
tend to use filesystem / mount watches.

Also I think we could generate FS_DELETE_SELF twice for the inode - once it
will we generated as a result of d_delete("A/x"), and finish_dput() ->
dentry_kill() -> dentry_unlink_inode() will generate it for the second time
on the last close of the open through "B/y". That should be mostly harmless
but still it's an oddity to keep in mind and preferably avoid if we can.

Now there's the question about the best fix for the problem. I've realized
we cannot just remove the call to fsnotify_inode_delete() from the place
where fsnotify_inoderemove() is called (and rely on the call from
__destroy_inode()) because the connector holds inode reference and so
__destroy_inode() would never get called. Hrm, this is a hairy issue and so
far I don't see a good place to reliably trigger detaching of connector
from the inode. One reliable place would be iput() but that's really ugly
and would slow down every iput() user. The cleanest solution I can
currently see is to stop holding inode reference from a connector as I've
proposed in the past. Then we can reliably perform the cleanup in
__destroy_inode(). But that will be a rather non-trivial undertaking. Amir,
do you see some other option?

								Honza 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Amir Goldstein]

On Fri, Aug 28, 2026 at 11:05 AM Jan Kara <jack@suse.cz> wrote:
>
> Hi!
>
> On Fri 28-08-26 13:24:19, Daehyeon Ko wrote:
> > You are right that an fd-held dentry cannot satisfy the count == 1 check. I
> > was imprecise in the report: the fd and the final unlink refer to two
> > different hard-link dentries for the same inode.
> >
> > The setup is:
> >
> >   1. create A/x and link it as B/y;
> >   2. open B/y, add the old-group inode mark through that fd, then unlink
> >      B/y, leaving i_nlink == 1 while the fd retains the B/y dentry;
> >   3. race these two calls:
> >
> >        unlink("A/x")
> >
> >        fanotify_mark(racer_group, FAN_MARK_ADD, FAN_MODIFY,
> >                      victim_fd, NULL)
> >
> >      where victim_fd still refers to the already unlinked B/y dentry.
> >
> > So the A/x dentry checked by d_delete() is not held by the fd and can have
> > d_lockref.count == 1. Its unlink drops the inode's last link and reaches
> > dentry_unlink_inode() -> fsnotify_inoderemove(). At the same time,
> > fanotify_find_path() gets the same inode through the fd-held B/y dentry and
> > adds the racer mark.
>
> Thanks for explanation of the reproducer. Now it makes sense. The
> conclusion from this is that fsnotify_inoderemove() can indeed be called
> for still fully alive open inodes which should be (think of permission /
> pre-content events) still generating events. That could be actually
> breaking some assumptions of userspace although I don't think it's a
> serious issue in practice because users of permission / pre-content watches
> tend to use filesystem / mount watches.
>
> Also I think we could generate FS_DELETE_SELF twice for the inode - once it
> will we generated as a result of d_delete("A/x"), and finish_dput() ->
> dentry_kill() -> dentry_unlink_inode() will generate it for the second time
> on the last close of the open through "B/y". That should be mostly harmless
> but still it's an oddity to keep in mind and preferably avoid if we can.
>

I don't know. This sounds like a fundamental behavior change.
In current upstream FAN_DELETE_SELF is a dying breath of an
inode and no more events on this inode are expected IIUC.

Applications could be relying on this.
In fact I have an application that almost relies on this, but IIRC
it does one last open_by_handle_at() to make sure before calling
the inode dead. Other applications could be less strict.

Changing behavior so events could happen after FAN_DELETE_SELF
sounds risky.

> Now there's the question about the best fix for the problem. I've realized
> we cannot just remove the call to fsnotify_inode_delete() from the place
> where fsnotify_inoderemove() is called (and rely on the call from
> __destroy_inode()) because the connector holds inode reference and so
> __destroy_inode() would never get called. Hrm, this is a hairy issue and so
> far I don't see a good place to reliably trigger detaching of connector
> from the inode. One reliable place would be iput() but that's really ugly
> and would slow down every iput() user. The cleanest solution I can
> currently see is to stop holding inode reference from a connector as I've
> proposed in the past. Then we can reliably perform the cleanup in
> __destroy_inode(). But that will be a rather non-trivial undertaking. Amir,
> do you see some other option?
>

What about the option I proposed to live with the orphan marks
Is it so bad? I mean it's confusing and removing the assertion could
hide real bugs/races, but maybe it's worth not complicating the code
until we move forward to re-attachable marks.

One way to think about it is that if we do the re-attachable marks,
we would need to store ino/st_dev and maybe fid in the mark and
then show_fdinfo wont need to get the inode reference at all.

So in a way, allowing an orphan mark on the connector is a step in
the direction of allowing re-attachable marks on the connector.

Maybe...

Thanks,
Amir.

Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Jan Kara]

On Fri 28-08-26 12:18:52, Amir Goldstein wrote:
> On Fri, Aug 28, 2026 at 11:05 AM Jan Kara <jack@suse.cz> wrote:
> > On Fri 28-08-26 13:24:19, Daehyeon Ko wrote:
> > > You are right that an fd-held dentry cannot satisfy the count == 1 check. I
> > > was imprecise in the report: the fd and the final unlink refer to two
> > > different hard-link dentries for the same inode.
> > >
> > > The setup is:
> > >
> > >   1. create A/x and link it as B/y;
> > >   2. open B/y, add the old-group inode mark through that fd, then unlink
> > >      B/y, leaving i_nlink == 1 while the fd retains the B/y dentry;
> > >   3. race these two calls:
> > >
> > >        unlink("A/x")
> > >
> > >        fanotify_mark(racer_group, FAN_MARK_ADD, FAN_MODIFY,
> > >                      victim_fd, NULL)
> > >
> > >      where victim_fd still refers to the already unlinked B/y dentry.
> > >
> > > So the A/x dentry checked by d_delete() is not held by the fd and can have
> > > d_lockref.count == 1. Its unlink drops the inode's last link and reaches
> > > dentry_unlink_inode() -> fsnotify_inoderemove(). At the same time,
> > > fanotify_find_path() gets the same inode through the fd-held B/y dentry and
> > > adds the racer mark.
> >
> > Thanks for explanation of the reproducer. Now it makes sense. The
> > conclusion from this is that fsnotify_inoderemove() can indeed be called
> > for still fully alive open inodes which should be (think of permission /
> > pre-content events) still generating events. That could be actually
> > breaking some assumptions of userspace although I don't think it's a
> > serious issue in practice because users of permission / pre-content watches
> > tend to use filesystem / mount watches.
> >
> > Also I think we could generate FS_DELETE_SELF twice for the inode - once it
> > will we generated as a result of d_delete("A/x"), and finish_dput() ->
> > dentry_kill() -> dentry_unlink_inode() will generate it for the second time
> > on the last close of the open through "B/y". That should be mostly harmless
> > but still it's an oddity to keep in mind and preferably avoid if we can.
> 
> I don't know. This sounds like a fundamental behavior change.
> In current upstream FAN_DELETE_SELF is a dying breath of an
> inode and no more events on this inode are expected IIUC.
> 
> Applications could be relying on this.
> In fact I have an application that almost relies on this, but IIRC
> it does one last open_by_handle_at() to make sure before calling
> the inode dead. Other applications could be less strict.
> 
> Changing behavior so events could happen after FAN_DELETE_SELF
> sounds risky.

I was probably imprecise with my wording. What I meant is that *currently*
when you have a hardlinked inode, you open and unlink through B/y and then
the unlink though A/x races with close of the B/y file, then you could get
two FAN_DELETE_SELF events if the race works out right. The only change of
behavior I was planning was avoiding this race if possible :).
 
> > Now there's the question about the best fix for the problem. I've realized
> > we cannot just remove the call to fsnotify_inode_delete() from the place
> > where fsnotify_inoderemove() is called (and rely on the call from
> > __destroy_inode()) because the connector holds inode reference and so
> > __destroy_inode() would never get called. Hrm, this is a hairy issue and so
> > far I don't see a good place to reliably trigger detaching of connector
> > from the inode. One reliable place would be iput() but that's really ugly
> > and would slow down every iput() user. The cleanest solution I can
> > currently see is to stop holding inode reference from a connector as I've
> > proposed in the past. Then we can reliably perform the cleanup in
> > __destroy_inode(). But that will be a rather non-trivial undertaking. Amir,
> > do you see some other option?
> 
> What about the option I proposed to live with the orphan marks
> Is it so bad? I mean it's confusing and removing the assertion could
> hide real bugs/races, but maybe it's worth not complicating the code
> until we move forward to re-attachable marks.

I'm not so much worried about the case when placing of the mark races with
unlink. There I agree we can somehow paper over the problem for now. What
I'm more worried about is the case where someone places a precontent /
permission mark on the inode and a malicious user does hardlink + open +
unlink trick and after unlinking the original link to the inode, he has an
open fd to the inode stripped of any notification marks which I'd say
violates the promise of precontent / permission marks.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Amir Goldstein]

On Fri, Aug 28, 2026 at 7:36 PM Jan Kara <jack@suse.cz> wrote:
>
> On Fri 28-08-26 12:18:52, Amir Goldstein wrote:
> > On Fri, Aug 28, 2026 at 11:05 AM Jan Kara <jack@suse.cz> wrote:
> > > On Fri 28-08-26 13:24:19, Daehyeon Ko wrote:
> > > > You are right that an fd-held dentry cannot satisfy the count == 1 check. I
> > > > was imprecise in the report: the fd and the final unlink refer to two
> > > > different hard-link dentries for the same inode.
> > > >
> > > > The setup is:
> > > >
> > > >   1. create A/x and link it as B/y;
> > > >   2. open B/y, add the old-group inode mark through that fd, then unlink
> > > >      B/y, leaving i_nlink == 1 while the fd retains the B/y dentry;
> > > >   3. race these two calls:
> > > >
> > > >        unlink("A/x")
> > > >
> > > >        fanotify_mark(racer_group, FAN_MARK_ADD, FAN_MODIFY,
> > > >                      victim_fd, NULL)
> > > >
> > > >      where victim_fd still refers to the already unlinked B/y dentry.
> > > >
> > > > So the A/x dentry checked by d_delete() is not held by the fd and can have
> > > > d_lockref.count == 1. Its unlink drops the inode's last link and reaches
> > > > dentry_unlink_inode() -> fsnotify_inoderemove(). At the same time,
> > > > fanotify_find_path() gets the same inode through the fd-held B/y dentry and
> > > > adds the racer mark.
> > >
> > > Thanks for explanation of the reproducer. Now it makes sense. The
> > > conclusion from this is that fsnotify_inoderemove() can indeed be called
> > > for still fully alive open inodes which should be (think of permission /
> > > pre-content events) still generating events. That could be actually
> > > breaking some assumptions of userspace although I don't think it's a
> > > serious issue in practice because users of permission / pre-content watches
> > > tend to use filesystem / mount watches.
> > >
> > > Also I think we could generate FS_DELETE_SELF twice for the inode - once it
> > > will we generated as a result of d_delete("A/x"), and finish_dput() ->
> > > dentry_kill() -> dentry_unlink_inode() will generate it for the second time
> > > on the last close of the open through "B/y". That should be mostly harmless
> > > but still it's an oddity to keep in mind and preferably avoid if we can.
> >
> > I don't know. This sounds like a fundamental behavior change.
> > In current upstream FAN_DELETE_SELF is a dying breath of an
> > inode and no more events on this inode are expected IIUC.
> >
> > Applications could be relying on this.
> > In fact I have an application that almost relies on this, but IIRC
> > it does one last open_by_handle_at() to make sure before calling
> > the inode dead. Other applications could be less strict.
> >
> > Changing behavior so events could happen after FAN_DELETE_SELF
> > sounds risky.
>
> I was probably imprecise with my wording. What I meant is that *currently*
> when you have a hardlinked inode, you open and unlink through B/y and then
> the unlink though A/x races with close of the B/y file, then you could get
> two FAN_DELETE_SELF events if the race works out right. The only change of
> behavior I was planning was avoiding this race if possible :).
>

Sorry I misread.

> > > Now there's the question about the best fix for the problem. I've realized
> > > we cannot just remove the call to fsnotify_inode_delete() from the place
> > > where fsnotify_inoderemove() is called (and rely on the call from
> > > __destroy_inode()) because the connector holds inode reference and so
> > > __destroy_inode() would never get called. Hrm, this is a hairy issue and so
> > > far I don't see a good place to reliably trigger detaching of connector
> > > from the inode. One reliable place would be iput() but that's really ugly
> > > and would slow down every iput() user. The cleanest solution I can
> > > currently see is to stop holding inode reference from a connector as I've
> > > proposed in the past. Then we can reliably perform the cleanup in
> > > __destroy_inode(). But that will be a rather non-trivial undertaking. Amir,
> > > do you see some other option?
> >
> > What about the option I proposed to live with the orphan marks
> > Is it so bad? I mean it's confusing and removing the assertion could
> > hide real bugs/races, but maybe it's worth not complicating the code
> > until we move forward to re-attachable marks.
>
> I'm not so much worried about the case when placing of the mark races with
> unlink. There I agree we can somehow paper over the problem for now. What
> I'm more worried about is the case where someone places a precontent /
> permission mark on the inode and a malicious user does hardlink + open +
> unlink trick and after unlinking the original link to the inode, he has an
> open fd to the inode stripped of any notification marks which I'd say
> violates the promise of precontent / permission marks.

I see. so we need a point where there are no more hardlinks and no more
dentry aliases to the inode.

How about we check for this condition?

Am I missing something?

Thanks,
Amir.

--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -471,7 +471,7 @@ static void dentry_unlink_inode(struct dentry * dentry)
        raw_write_seqcount_end(&dentry->d_seq);
        spin_unlock(&dentry->d_lock);
        spin_unlock(&inode->i_lock);
-       if (!inode->i_nlink)
+       if (!inode->i_nlink && hlist_empty(&inode->i_dentry))
                fsnotify_inoderemove(inode);
        if (dentry->d_op && dentry->d_op->d_iput)
                dentry->d_op->d_iput(dentry, inode);

Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Jan Kara]

On Sat 29-08-26 17:04:19, Amir Goldstein wrote:
> On Fri, Aug 28, 2026 at 7:36 PM Jan Kara <jack@suse.cz> wrote:
> > > > Now there's the question about the best fix for the problem. I've realized
> > > > we cannot just remove the call to fsnotify_inode_delete() from the place
> > > > where fsnotify_inoderemove() is called (and rely on the call from
> > > > __destroy_inode()) because the connector holds inode reference and so
> > > > __destroy_inode() would never get called. Hrm, this is a hairy issue and so
> > > > far I don't see a good place to reliably trigger detaching of connector
> > > > from the inode. One reliable place would be iput() but that's really ugly
> > > > and would slow down every iput() user. The cleanest solution I can
> > > > currently see is to stop holding inode reference from a connector as I've
> > > > proposed in the past. Then we can reliably perform the cleanup in
> > > > __destroy_inode(). But that will be a rather non-trivial undertaking. Amir,
> > > > do you see some other option?
> > >
> > > What about the option I proposed to live with the orphan marks
> > > Is it so bad? I mean it's confusing and removing the assertion could
> > > hide real bugs/races, but maybe it's worth not complicating the code
> > > until we move forward to re-attachable marks.
> >
> > I'm not so much worried about the case when placing of the mark races with
> > unlink. There I agree we can somehow paper over the problem for now. What
> > I'm more worried about is the case where someone places a precontent /
> > permission mark on the inode and a malicious user does hardlink + open +
> > unlink trick and after unlinking the original link to the inode, he has an
> > open fd to the inode stripped of any notification marks which I'd say
> > violates the promise of precontent / permission marks.
> 
> I see. so we need a point where there are no more hardlinks and no more
> dentry aliases to the inode.
> 
> How about we check for this condition?
>
> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -471,7 +471,7 @@ static void dentry_unlink_inode(struct dentry * dentry)
>         raw_write_seqcount_end(&dentry->d_seq);
>         spin_unlock(&dentry->d_lock);
>         spin_unlock(&inode->i_lock);
> -       if (!inode->i_nlink)
> +       if (!inode->i_nlink && hlist_empty(&inode->i_dentry))
>                 fsnotify_inoderemove(inode);

This looks like an intriguing idea. But I think it still has a hole. Until
the last inode reference is put, open_by_handle() can still get you a new
dentry and an open file pointing to that inode.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Amir Goldstein]

On Mon, Aug 31, 2026 at 1:39 PM Jan Kara <jack@suse.cz> wrote:
>
> On Sat 29-08-26 17:04:19, Amir Goldstein wrote:
> > On Fri, Aug 28, 2026 at 7:36 PM Jan Kara <jack@suse.cz> wrote:
> > > > > Now there's the question about the best fix for the problem. I've realized
> > > > > we cannot just remove the call to fsnotify_inode_delete() from the place
> > > > > where fsnotify_inoderemove() is called (and rely on the call from
> > > > > __destroy_inode()) because the connector holds inode reference and so
> > > > > __destroy_inode() would never get called. Hrm, this is a hairy issue and so
> > > > > far I don't see a good place to reliably trigger detaching of connector
> > > > > from the inode. One reliable place would be iput() but that's really ugly
> > > > > and would slow down every iput() user. The cleanest solution I can
> > > > > currently see is to stop holding inode reference from a connector as I've
> > > > > proposed in the past. Then we can reliably perform the cleanup in
> > > > > __destroy_inode(). But that will be a rather non-trivial undertaking. Amir,
> > > > > do you see some other option?
> > > >
> > > > What about the option I proposed to live with the orphan marks
> > > > Is it so bad? I mean it's confusing and removing the assertion could
> > > > hide real bugs/races, but maybe it's worth not complicating the code
> > > > until we move forward to re-attachable marks.
> > >
> > > I'm not so much worried about the case when placing of the mark races with
> > > unlink. There I agree we can somehow paper over the problem for now. What
> > > I'm more worried about is the case where someone places a precontent /
> > > permission mark on the inode and a malicious user does hardlink + open +
> > > unlink trick and after unlinking the original link to the inode, he has an
> > > open fd to the inode stripped of any notification marks which I'd say
> > > violates the promise of precontent / permission marks.
> >
> > I see. so we need a point where there are no more hardlinks and no more
> > dentry aliases to the inode.
> >
> > How about we check for this condition?
> >
> > --- a/fs/dcache.c
> > +++ b/fs/dcache.c
> > @@ -471,7 +471,7 @@ static void dentry_unlink_inode(struct dentry * dentry)
> >         raw_write_seqcount_end(&dentry->d_seq);
> >         spin_unlock(&dentry->d_lock);
> >         spin_unlock(&inode->i_lock);
> > -       if (!inode->i_nlink)
> > +       if (!inode->i_nlink && hlist_empty(&inode->i_dentry))
> >                 fsnotify_inoderemove(inode);
>
> This looks like an intriguing idea. But I think it still has a hole. Until
> the last inode reference is put, open_by_handle() can still get you a new
> dentry and an open file pointing to that inode.

Maybe we can close this hole, because whether open_by_handle()
should or should not succeed in this case is not well defined,
We can make it defined.
Something like this?
NFS should have a sillyrenamed link to avoid undefined behavior
around unlinked open files.

Thanks,
Amir.

--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -459,6 +459,7 @@ static void dentry_unlink_inode(struct dentry * dentry)
        __releases(dentry->d_inode->i_lock)
 {
        struct inode *inode = dentry->d_inode;
+       bool dead;

        raw_write_seqcount_begin(&dentry->d_seq);
        __d_clear_type_and_inode(dentry);
@@ -469,9 +470,12 @@ static void dentry_unlink_inode(struct dentry * dentry)
         */
        dentry->waiters = NULL;
        raw_write_seqcount_end(&dentry->d_seq);
+       dead = !inode->i_nlink && hlist_empty(&inode->i_dentry);
+       if (dead)
+               inode->i_state |= I_ZOMBIE;
        spin_unlock(&dentry->d_lock);
        spin_unlock(&inode->i_lock);
-       if (!inode->i_nlink)
+       if (dead)
                fsnotify_inoderemove(inode);
        if (dentry->d_op && dentry->d_op->d_iput)
                dentry->d_op->d_iput(dentry, inode);
@@ -2244,6 +2248,12 @@ static struct dentry *__d_obtain_alias(struct
inode *inode, bool disconnected)

        security_d_instantiate(new, inode);
        spin_lock(&inode->i_lock);
+       if (inode->i_state & I_ZOMBIE) {
+               spin_unlock(&inode->i_lock);
+               dput(new);
+               iput(inode);
+               return ERR_PTR(-ESTALE);
+       }
        res = __d_find_any_alias(inode); /* recheck under lock */
        if (likely(!res)) { /* still no alias, attach a disconnected dentry */
                unsigned add_flags = d_flags_for_inode(inode);

Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Jan Kara]

On Mon 31-08-26 16:54:55, Amir Goldstein wrote:
> On Mon, Aug 31, 2026 at 1:39 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Sat 29-08-26 17:04:19, Amir Goldstein wrote:
> > > On Fri, Aug 28, 2026 at 7:36 PM Jan Kara <jack@suse.cz> wrote:
> > > > > > Now there's the question about the best fix for the problem. I've realized
> > > > > > we cannot just remove the call to fsnotify_inode_delete() from the place
> > > > > > where fsnotify_inoderemove() is called (and rely on the call from
> > > > > > __destroy_inode()) because the connector holds inode reference and so
> > > > > > __destroy_inode() would never get called. Hrm, this is a hairy issue and so
> > > > > > far I don't see a good place to reliably trigger detaching of connector
> > > > > > from the inode. One reliable place would be iput() but that's really ugly
> > > > > > and would slow down every iput() user. The cleanest solution I can
> > > > > > currently see is to stop holding inode reference from a connector as I've
> > > > > > proposed in the past. Then we can reliably perform the cleanup in
> > > > > > __destroy_inode(). But that will be a rather non-trivial undertaking. Amir,
> > > > > > do you see some other option?
> > > > >
> > > > > What about the option I proposed to live with the orphan marks
> > > > > Is it so bad? I mean it's confusing and removing the assertion could
> > > > > hide real bugs/races, but maybe it's worth not complicating the code
> > > > > until we move forward to re-attachable marks.
> > > >
> > > > I'm not so much worried about the case when placing of the mark races with
> > > > unlink. There I agree we can somehow paper over the problem for now. What
> > > > I'm more worried about is the case where someone places a precontent /
> > > > permission mark on the inode and a malicious user does hardlink + open +
> > > > unlink trick and after unlinking the original link to the inode, he has an
> > > > open fd to the inode stripped of any notification marks which I'd say
> > > > violates the promise of precontent / permission marks.
> > >
> > > I see. so we need a point where there are no more hardlinks and no more
> > > dentry aliases to the inode.
> > >
> > > How about we check for this condition?
> > >
> > > --- a/fs/dcache.c
> > > +++ b/fs/dcache.c
> > > @@ -471,7 +471,7 @@ static void dentry_unlink_inode(struct dentry * dentry)
> > >         raw_write_seqcount_end(&dentry->d_seq);
> > >         spin_unlock(&dentry->d_lock);
> > >         spin_unlock(&inode->i_lock);
> > > -       if (!inode->i_nlink)
> > > +       if (!inode->i_nlink && hlist_empty(&inode->i_dentry))
> > >                 fsnotify_inoderemove(inode);
> >
> > This looks like an intriguing idea. But I think it still has a hole. Until
> > the last inode reference is put, open_by_handle() can still get you a new
> > dentry and an open file pointing to that inode.
> 
> Maybe we can close this hole, because whether open_by_handle()
> should or should not succeed in this case is not well defined,
> We can make it defined.
> Something like this?
> NFS should have a sillyrenamed link to avoid undefined behavior
> around unlinked open files.

Hum, right. That could work.

> --- a/fs/dcache.c
> +++ b/fs/dcache.c
> @@ -459,6 +459,7 @@ static void dentry_unlink_inode(struct dentry * dentry)
>         __releases(dentry->d_inode->i_lock)
>  {
>         struct inode *inode = dentry->d_inode;
> +       bool dead;
> 
>         raw_write_seqcount_begin(&dentry->d_seq);
>         __d_clear_type_and_inode(dentry);
> @@ -469,9 +470,12 @@ static void dentry_unlink_inode(struct dentry * dentry)
>          */
>         dentry->waiters = NULL;
>         raw_write_seqcount_end(&dentry->d_seq);
> +       dead = !inode->i_nlink && hlist_empty(&inode->i_dentry);
> +       if (dead)
> +               inode->i_state |= I_ZOMBIE;

Do we really need the I_ZOMBIE state? I think we can just check i_nlink and
hlist_empty() directly in __d_obtain_alias(), cannot we? Once this
condition is true, it cannot change until we go through
__d_obtain_alias()...

>         spin_unlock(&dentry->d_lock);
>         spin_unlock(&inode->i_lock);
> -       if (!inode->i_nlink)
> +       if (dead)
>                 fsnotify_inoderemove(inode);
>         if (dentry->d_op && dentry->d_op->d_iput)
>                 dentry->d_op->d_iput(dentry, inode);
> @@ -2244,6 +2248,12 @@ static struct dentry *__d_obtain_alias(struct
> inode *inode, bool disconnected)
> 
>         security_d_instantiate(new, inode);
>         spin_lock(&inode->i_lock);
> +       if (inode->i_state & I_ZOMBIE) {
> +               spin_unlock(&inode->i_lock);
> +               dput(new);
> +               iput(inode);
> +               return ERR_PTR(-ESTALE);

I think a failure return after security_d_instantiate() could be unexpected
so we need to do this check before calling it.

> +       }
>         res = __d_find_any_alias(inode); /* recheck under lock */
>         if (likely(!res)) { /* still no alias, attach a disconnected dentry */
>                 unsigned add_flags = d_flags_for_inode(inode);

Thanks for looking into this!

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Amir Goldstein]

[-- Attachment #1: Type: text/plain, Size: 5730 bytes --]

On Tue, Sep 1, 2026 at 6:57 PM Jan Kara <jack@suse.cz> wrote:
>
> On Mon 31-08-26 16:54:55, Amir Goldstein wrote:
> > On Mon, Aug 31, 2026 at 1:39 PM Jan Kara <jack@suse.cz> wrote:
> > >
> > > On Sat 29-08-26 17:04:19, Amir Goldstein wrote:
> > > > On Fri, Aug 28, 2026 at 7:36 PM Jan Kara <jack@suse.cz> wrote:
> > > > > > > Now there's the question about the best fix for the problem. I've realized
> > > > > > > we cannot just remove the call to fsnotify_inode_delete() from the place
> > > > > > > where fsnotify_inoderemove() is called (and rely on the call from
> > > > > > > __destroy_inode()) because the connector holds inode reference and so
> > > > > > > __destroy_inode() would never get called. Hrm, this is a hairy issue and so
> > > > > > > far I don't see a good place to reliably trigger detaching of connector
> > > > > > > from the inode. One reliable place would be iput() but that's really ugly
> > > > > > > and would slow down every iput() user. The cleanest solution I can
> > > > > > > currently see is to stop holding inode reference from a connector as I've
> > > > > > > proposed in the past. Then we can reliably perform the cleanup in
> > > > > > > __destroy_inode(). But that will be a rather non-trivial undertaking. Amir,
> > > > > > > do you see some other option?
> > > > > >
> > > > > > What about the option I proposed to live with the orphan marks
> > > > > > Is it so bad? I mean it's confusing and removing the assertion could
> > > > > > hide real bugs/races, but maybe it's worth not complicating the code
> > > > > > until we move forward to re-attachable marks.
> > > > >
> > > > > I'm not so much worried about the case when placing of the mark races with
> > > > > unlink. There I agree we can somehow paper over the problem for now. What
> > > > > I'm more worried about is the case where someone places a precontent /
> > > > > permission mark on the inode and a malicious user does hardlink + open +
> > > > > unlink trick and after unlinking the original link to the inode, he has an
> > > > > open fd to the inode stripped of any notification marks which I'd say
> > > > > violates the promise of precontent / permission marks.
> > > >
> > > > I see. so we need a point where there are no more hardlinks and no more
> > > > dentry aliases to the inode.
> > > >
> > > > How about we check for this condition?
> > > >
> > > > --- a/fs/dcache.c
> > > > +++ b/fs/dcache.c
> > > > @@ -471,7 +471,7 @@ static void dentry_unlink_inode(struct dentry * dentry)
> > > >         raw_write_seqcount_end(&dentry->d_seq);
> > > >         spin_unlock(&dentry->d_lock);
> > > >         spin_unlock(&inode->i_lock);
> > > > -       if (!inode->i_nlink)
> > > > +       if (!inode->i_nlink && hlist_empty(&inode->i_dentry))
> > > >                 fsnotify_inoderemove(inode);
> > >
> > > This looks like an intriguing idea. But I think it still has a hole. Until
> > > the last inode reference is put, open_by_handle() can still get you a new
> > > dentry and an open file pointing to that inode.
> >
> > Maybe we can close this hole, because whether open_by_handle()
> > should or should not succeed in this case is not well defined,
> > We can make it defined.
> > Something like this?
> > NFS should have a sillyrenamed link to avoid undefined behavior
> > around unlinked open files.
>
> Hum, right. That could work.
>
> > --- a/fs/dcache.c
> > +++ b/fs/dcache.c
> > @@ -459,6 +459,7 @@ static void dentry_unlink_inode(struct dentry * dentry)
> >         __releases(dentry->d_inode->i_lock)
> >  {
> >         struct inode *inode = dentry->d_inode;
> > +       bool dead;
> >
> >         raw_write_seqcount_begin(&dentry->d_seq);
> >         __d_clear_type_and_inode(dentry);
> > @@ -469,9 +470,12 @@ static void dentry_unlink_inode(struct dentry * dentry)
> >          */
> >         dentry->waiters = NULL;
> >         raw_write_seqcount_end(&dentry->d_seq);
> > +       dead = !inode->i_nlink && hlist_empty(&inode->i_dentry);
> > +       if (dead)
> > +               inode->i_state |= I_ZOMBIE;
>
> Do we really need the I_ZOMBIE state? I think we can just check i_nlink and
> hlist_empty() directly in __d_obtain_alias(), cannot we? Once this
> condition is true, it cannot change until we go through
> __d_obtain_alias()...
>

OK I added convenience helper inode_is_dead()
names are debatable...

> >         spin_unlock(&dentry->d_lock);
> >         spin_unlock(&inode->i_lock);
> > -       if (!inode->i_nlink)
> > +       if (dead)
> >                 fsnotify_inoderemove(inode);
> >         if (dentry->d_op && dentry->d_op->d_iput)
> >                 dentry->d_op->d_iput(dentry, inode);
> > @@ -2244,6 +2248,12 @@ static struct dentry *__d_obtain_alias(struct
> > inode *inode, bool disconnected)
> >
> >         security_d_instantiate(new, inode);
> >         spin_lock(&inode->i_lock);
> > +       if (inode->i_state & I_ZOMBIE) {
> > +               spin_unlock(&inode->i_lock);
> > +               dput(new);
> > +               iput(inode);
> > +               return ERR_PTR(-ESTALE);
>
> I think a failure return after security_d_instantiate() could be unexpected
> so we need to do this check before calling it.

You mean before and after.

>
> > +       }
> >         res = __d_find_any_alias(inode); /* recheck under lock */
> >         if (likely(!res)) { /* still no alias, attach a disconnected dentry */
> >                 unsigned add_flags = d_flags_for_inode(inode);
>
> Thanks for looking into this!
>

See attached patch (again helper named debatable).

Daehyeon, care to test this patch with your reproducer?

Thanks,
Amir.

[-- Attachment #2: 0001-fs-make-sure-to-call-fsnotify_removeinode-once-when-.patch --]
[-- Type: text/x-patch, Size: 3385 bytes --]

From 2d3d1938373bfcb27545a58fff1600f2f2ccdd31 Mon Sep 17 00:00:00 2001
From: Amir Goldstein <amir73il@gmail.com>
Date: Tue, 1 Sep 2026 23:40:43 +0200
Subject: [PATCH] fs: make sure to call fsnotify_removeinode() once when inode
 is dead

Daehyeon reported a race wherein fsnotify_removeinode() can be called
when inode still have a live alias.  This can lead to several undesired
outcomes.

Check that inode is really "dead", meaning no nlink and no more aliases
before calling fsnotify_removeinode() for final cleanup of inode marks.

Also do not allow open_by_handle() to add new aliases to a "dead" inode.

Reported-by: Daehyeon Ko <4ncienth@gmail.com>
Closes: https://lore.kernel.org/linux-fsdevel/20260827045007.3831259-1-4ncienth@gmail.com/
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/dcache.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 3e9af9de70746..a76260092e26a 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -450,6 +450,12 @@ static void dentry_free(struct dentry *dentry)
 		call_rcu(&dentry->d_rcu, __d_free);
 }
 
+/* Inode may be kept alive by a positive nlink or by a dentry alias */
+static bool inode_is_dead(struct inode *inode)
+{
+	return !inode->i_nlink && hlist_empty(&inode->i_dentry);
+}
+
 /*
  * Release the dentry's inode, using the filesystem
  * d_iput() operation if defined.
@@ -459,6 +465,7 @@ static void dentry_unlink_inode(struct dentry * dentry)
 	__releases(dentry->d_inode->i_lock)
 {
 	struct inode *inode = dentry->d_inode;
+	bool dead;
 
 	raw_write_seqcount_begin(&dentry->d_seq);
 	__d_clear_type_and_inode(dentry);
@@ -469,9 +476,10 @@ static void dentry_unlink_inode(struct dentry * dentry)
 	 */
 	dentry->waiters = NULL;
 	raw_write_seqcount_end(&dentry->d_seq);
+	dead = inode_is_dead(inode);
 	spin_unlock(&dentry->d_lock);
 	spin_unlock(&inode->i_lock);
-	if (!inode->i_nlink)
+	if (dead)
 		fsnotify_inoderemove(inode);
 	if (dentry->d_op && dentry->d_op->d_iput)
 		dentry->d_op->d_iput(dentry, inode);
@@ -1160,6 +1168,18 @@ static struct dentry * __d_find_any_alias(struct inode *inode)
 	return NULL;
 }
 
+/*
+ * Same as __d_find_any_alias(), but returns ERR_PTR(-ESTALE) if the inode is
+ * dead and should not accept any new aliases.
+ */
+static struct dentry * __d_find_alias_of_live_inode(struct inode *inode)
+{
+	struct dentry *alias = __d_find_any_alias(inode);
+	if (!alias && inode_is_dead(inode))
+		return ERR_PTR(-ESTALE);
+	return alias;
+}
+
 /**
  * d_find_any_alias - find any alias for a given inode
  * @inode: inode to find an alias for
@@ -2232,7 +2252,9 @@ static struct dentry *__d_obtain_alias(struct inode *inode, bool disconnected)
 
 	sb = inode->i_sb;
 
-	res = d_find_any_alias(inode); /* existing alias? */
+	spin_lock(&inode->i_lock);
+	res = __d_find_alias_of_live_inode(inode); /* existing alias? */
+	spin_unlock(&inode->i_lock);
 	if (res)
 		goto out;
 
@@ -2244,7 +2266,7 @@ static struct dentry *__d_obtain_alias(struct inode *inode, bool disconnected)
 
 	security_d_instantiate(new, inode);
 	spin_lock(&inode->i_lock);
-	res = __d_find_any_alias(inode); /* recheck under lock */
+	res = __d_find_alias_of_live_inode(inode); /* recheck under lock */
 	if (likely(!res)) { /* still no alias, attach a disconnected dentry */
 		unsigned add_flags = d_flags_for_inode(inode);
 
-- 
2.55.0

Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Daehyeon Ko]

Hi Amir,

I tested the exact attached patch (SHA-256
9fa187507909ae14c8de8a117e775122e3daa425c69081b7115b3dc979fca14d)
on Linux v7.2, commit 8d3ae59288f1, with an x86_64 KASAN kernel.

The capless prerequisite probe passed as UID/GID 65534 with CapEff=0 and
NoNewPrivs=1. I then ran the original reproducer command

  fanotify_destroy_add_race 10000 1 1024 256

in three fresh 2-vCPU boots. All three runs completed 10,000 iterations with
10,000 successful adds, zero add errors, and 10,000 fdinfo reads. The complete
serial logs contain no fsnotify_conn_mask() warning, other kernel warning,
KASAN report, Oops, panic, or kernel BUG, and both guest and host exited 0.

This tests the originally reported race reproducer. I did not yet test the
new permission/pre-content, duplicate FAN_DELETE_SELF, open_by_handle_at(), or
NFS/export-filesystem semantics discussed later in the thread.

Tested-by: Daehyeon Ko <4ncienth@gmail.com>

Thanks,
Daehyeon

Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Amir Goldstein]

On Wed, Sep 2, 2026 at 9:04 AM Daehyeon Ko <4ncienth@gmail.com> wrote:
>
> Hi Amir,
>
> I tested the exact attached patch (SHA-256
> 9fa187507909ae14c8de8a117e775122e3daa425c69081b7115b3dc979fca14d)
> on Linux v7.2, commit 8d3ae59288f1, with an x86_64 KASAN kernel.
>
> The capless prerequisite probe passed as UID/GID 65534 with CapEff=0 and
> NoNewPrivs=1. I then ran the original reproducer command
>
>   fanotify_destroy_add_race 10000 1 1024 256
>
> in three fresh 2-vCPU boots. All three runs completed 10,000 iterations with
> 10,000 successful adds, zero add errors, and 10,000 fdinfo reads. The complete
> serial logs contain no fsnotify_conn_mask() warning, other kernel warning,
> KASAN report, Oops, panic, or kernel BUG, and both guest and host exited 0.
>
> This tests the originally reported race reproducer. I did not yet test the
> new permission/pre-content, duplicate FAN_DELETE_SELF, open_by_handle_at(), or
> NFS/export-filesystem semantics discussed later in the thread.
>
> Tested-by: Daehyeon Ko <4ncienth@gmail.com>

Thank you!

If you can think of a way to trigger the race in a reliable LTP regression test
that would be great.

By "reliable" I mean works reliably with some fs at the moment, for example
if ext4 or some other fs practically keeps the inode on disk long
enough for the test
to open_by_handle and get a new dentry for an unlink inode after
FAN_DELETE_SELF,
and then close the fd to get another FAN_DELETE_SELF that's a reliable test.

Jan,

If you end up picking this patch as is please fix the typo
s/fsnotify_removeinode/fsnotify_inoderemove in commit subject/body.
Also, do feel free to rename "dead"/"live" to any other terms that you
find less confusing because yeh the lifetime of inode on-disk/in-core
is a very overloaded namespace.

Thanks,
Amir.

Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Jan Kara]

On Tue 01-09-26 23:53:48, Amir Goldstein wrote:
> On Tue, Sep 1, 2026 at 6:57 PM Jan Kara <jack@suse.cz> wrote:
> >
> > > +       }
> > >         res = __d_find_any_alias(inode); /* recheck under lock */
> > >         if (likely(!res)) { /* still no alias, attach a disconnected dentry */
> > >                 unsigned add_flags = d_flags_for_inode(inode);
> >
> > Thanks for looking into this!
> >
> 
> See attached patch (again helper named debatable).

Thanks for the patch! I have somewhat massaged the changelog and names and
sent it for inclusion to Christian. Since this is messing with dcache code,
it should better go through his tree (plus I want to give him a chance to
stop this somewhat hacky workaround ;)).

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Amir Goldstein]

On Wed, Sep 2, 2026 at 6:52 PM Jan Kara <jack@suse.cz> wrote:
>
> On Tue 01-09-26 23:53:48, Amir Goldstein wrote:
> > On Tue, Sep 1, 2026 at 6:57 PM Jan Kara <jack@suse.cz> wrote:
> > >
> > > > +       }
> > > >         res = __d_find_any_alias(inode); /* recheck under lock */
> > > >         if (likely(!res)) { /* still no alias, attach a disconnected dentry */
> > > >                 unsigned add_flags = d_flags_for_inode(inode);
> > >
> > > Thanks for looking into this!
> > >
> >
> > See attached patch (again helper named debatable).
>
> Thanks for the patch! I have somewhat massaged the changelog and names and

FYIW, I like your simplifications
and changelog definitely needed a touch up.

> sent it for inclusion to Christian. Since this is messing with dcache code,
> it should better go through his tree (plus I want to give him a chance to
> stop this somewhat hacky workaround ;)).

We'll see...

Thanks,
Amir.

Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Daehyeon Ko]

[-- Attachment #1: Type: text/plain, Size: 919 bytes --]

Hi Amir,

I was able to make the handle-reopen sequence reliable on ext4 and drafted
an LTP test for it.

The test keeps 120 auxiliary inode marks on the victim to widen the interval
after the first FAN_DELETE_SELF, pins the unlink and handle-reopen workers to
different CPUs, and has the reopen worker read the event queue nonblocking
before calling open_by_handle_at().

I ran the exact final test for 100 iterations in each of two fresh 2-vCPU
KASAN boots per kernel:

  - unmodified v7.2: 200/200 iterations reopened the unlinked inode, and
    closing the reopened fd generated a second FAN_DELETE_SELF;
  - v7.2 with your patch: 200/200 iterations returned ESTALE from
    open_by_handle_at(), with no duplicate FAN_DELETE_SELF.

The test is based on LTP master commit
12724413534a6d4160ff9694ba6f09daa4ccb6bd. The attached patch adds
fanotify26 and the corresponding build and runtest entries.

Thanks,
Daehyeon

[-- Attachment #2: 0001-fanotify-test-duplicate-FAN_DELETE_SELF-after-handle.patch --]
[-- Type: application/x-patch, Size: 11025 bytes --]

Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Amir Goldstein]

On Wed, Sep 2, 2026 at 11:51 PM Daehyeon Ko <4ncienth@gmail.com> wrote:
>
> Hi Amir,
>
> I was able to make the handle-reopen sequence reliable on ext4 and drafted
> an LTP test for it.
>
> The test keeps 120 auxiliary inode marks on the victim to widen the interval
> after the first FAN_DELETE_SELF, pins the unlink and handle-reopen workers to
> different CPUs, and has the reopen worker read the event queue nonblocking
> before calling open_by_handle_at().
>
> I ran the exact final test for 100 iterations in each of two fresh 2-vCPU
> KASAN boots per kernel:
>
>   - unmodified v7.2: 200/200 iterations reopened the unlinked inode, and
>     closing the reopened fd generated a second FAN_DELETE_SELF;
>   - v7.2 with your patch: 200/200 iterations returned ESTALE from
>     open_by_handle_at(), with no duplicate FAN_DELETE_SELF.
>
> The test is based on LTP master commit
> 12724413534a6d4160ff9694ba6f09daa4ccb6bd. The attached patch adds
> fanotify26 and the corresponding build and runtest entries.

Very nice!

I think we should wait for Christian to pick up the fix or suggest an
alternative, but after that please post this test to LTP.

Thanks for following up on this!
Amir.