Linux filesystem development
 help / color / mirror / Atom feed
* Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup
       [not found]                           ` <CAOQ4uxi0yRavUeztRh-Eb8_HvpfmqkpPbWT9JgQhUn7EKXnryw@mail.gmail.com>
@ 2026-06-10  8:17                             ` Gao Xiang
  2026-06-10 12:21                               ` Stacking filesystem deadlocks [was Re: Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup] Jan Kara
  0 siblings, 1 reply; 14+ messages in thread
From: Gao Xiang @ 2026-06-10  8:17 UTC (permalink / raw)
  To: linux-fsdevel@vger.kernel.org
  Cc: Christian Brauner, Miklos Szeredi, Gao Xiang, Song Liu,
	Amir Goldstein, Jan Kara

Hi all,

Amir just suggested that I posted a long off-list discussion
on the list right now, but since it seems to relate to
`sb->s_writers.rw_sem` locking design, I am not quite sure
how deep I could help generally: But here it goes.

The background is that we have discussed a generic deadlock
timing for several months as below:

fsA is a filesystem which supports fsfreeze, such as EXT4/XFS/...

fsB is a filesystem which have some relationship with a
userspace daemon (e.g. a filesystem with fanotify HSM hooks /
   fanotify + EROFS file-backed mounts / a FUSE filesystem or
   a filesystem backed by a virtual block device)

Thread A                           Thread B                                 Userspace deamon
  write(fsA_fd, mmap(fsB_fd))
   file_start_write()
    (take SB_FREEZE_WRITE read lock)

    handle fsB mmap fault read
     -> notify userspace and wait

                                    freeze_super
                                    (try to take SB_FREEZE_WRITE write lock)
                                                                             received/handling fsB mmap read request
                                                                             (do random something...)
                                                                             write(fsA_fd2)
                                                                              (take SB_FREEZE_WRITE read lock)
The problem timing here is thread A does
`write(fsA_fd, mmap(fsB_fd))` => file_start_write() (rwsem read),
then hits page fault and wait for userspace deamon to finish
the request;

Thread B does fsfreeze on fsA so it is waiting a write lock
(sb_wait_write(), rwsem write) and blocked on thread A;

And the userspace deamon is a handler handling page fault,
and trying to write to another file (fsA_fd2) on fsA again
and blocked on thread B (file_start_write(), rwsem read).

because of the specific locking timing is `R->W->R`, at least
the whole workflow won't proceed so the related processes
(and fsA above) will be stuck.


Since the issue is complex, I can only give my own thought
from the perspective of fanotify+EROFS use cases on this:

  - fsfreeze is typically unneeded, especially on typical
    cloud + container environment;

  - Also we could isolate the image backing file into another
    totally different local filesystem so that
    "write(fd, mmap(),..)" + fsfreeze won't be a practical
    issue.

  - In the worst case, user programs can be killed to recover
    the system, and fanotify already supports this way, so
    it shouldn't be too harmful;

  - this vector impacts all userspace approaches.


I also gave my own preliminary idea at LSF to Jan and Amir
(just for reference):

diff --git a/fs/super.c b/fs/super.c
index 378e81efe643..2897d3572e9e 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -2069,6 +2069,7 @@ static inline bool may_unfreeze(struct super_block *sb, enum freeze_holder who,
   */
  int freeze_super(struct super_block *sb, enum freeze_holder who, const void *freeze_owner)
  {
+    bool retried;
      int ret;

      if (!super_lock_excl(sb)) {
@@ -2111,7 +2112,15 @@ int freeze_super(struct super_block *sb, enum freeze_holder who, const void *fre
      sb->s_writers.frozen = SB_FREEZE_WRITE;
      /* Release s_umount to preserve sb_start_write -> s_umount ordering */
      super_unlock_excl(sb);
-    sb_wait_write(sb, SB_FREEZE_WRITE);
+    while (ret = sb_wait_write_timeout(sb, SB_FREEZE_WRITE)) {
+        if (retried) {
+            sb->s_writers.frozen = SB_UNFROZEN;
+            sb_freeze_unlock(sb, SB_FREEZE_FS);
+            deactivate_locked_super(sb);
+            return ret;
+        }
+        retried = true;
+    }
      __super_lock_excl(sb);

      /* Now we go and block page faults... */

It was simply to add a new sb_wait_write_timeout()
interface so that the writer (freeze_super) can be woken
up even when the locking order is already (R->W->R),
allowing the W (freeze_super) to be woken and the (R->R)
to be merged.

I wonder if this is a practical way since it allows
"write(fd, mmap(),..)" and workable for all approaches.
However, the main issue at least in my opinion is that
percpu_rwsem doesn't have percpu_down_write_timeout()
(but as for rwsem since down_write_killable() exists, I
think down_write_timeout() should be doable at least),
and I tried to vibe-code a version but have no idea if
it performs well.


Amir and other people may have other better ideas for
further discussion, but as Amir suggested, I've written
this...

Thanks,
Gao Xiang

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Stacking filesystem deadlocks [was Re: Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup]
  2026-06-10  8:17                             ` Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup Gao Xiang
@ 2026-06-10 12:21                               ` Jan Kara
  2026-06-10 13:18                                 ` Gao Xiang
  0 siblings, 1 reply; 14+ messages in thread
From: Jan Kara @ 2026-06-10 12:21 UTC (permalink / raw)
  To: Gao Xiang
  Cc: linux-fsdevel@vger.kernel.org, Christian Brauner, Miklos Szeredi,
	Gao Xiang, Song Liu, Amir Goldstein, Jan Kara

Hi!

On Wed 10-06-26 16:17:33, Gao Xiang wrote:
> Amir just suggested that I posted a long off-list discussion
> on the list right now, but since it seems to relate to
> `sb->s_writers.rw_sem` locking design, I am not quite sure
> how deep I could help generally: But here it goes.
> 
> The background is that we have discussed a generic deadlock
> timing for several months as below:
> 
> fsA is a filesystem which supports fsfreeze, such as EXT4/XFS/...
> 
> fsB is a filesystem which have some relationship with a
> userspace daemon (e.g. a filesystem with fanotify HSM hooks /
>   fanotify + EROFS file-backed mounts / a FUSE filesystem or
>   a filesystem backed by a virtual block device)
> 
> Thread A                           Thread B                                 Userspace deamon
>  write(fsA_fd, mmap(fsB_fd))
>   file_start_write()
>    (take SB_FREEZE_WRITE read lock)
> 
>    handle fsB mmap fault read
>     -> notify userspace and wait
> 
>                                    freeze_super
>                                    (try to take SB_FREEZE_WRITE write lock)
>                                                                             received/handling fsB mmap read request
>                                                                             (do random something...)
>                                                                             write(fsA_fd2)
>                                                                              (take SB_FREEZE_WRITE read lock)
> The problem timing here is thread A does
> `write(fsA_fd, mmap(fsB_fd))` => file_start_write() (rwsem read),
> then hits page fault and wait for userspace deamon to finish
> the request;
> 
> Thread B does fsfreeze on fsA so it is waiting a write lock
> (sb_wait_write(), rwsem write) and blocked on thread A;
> 
> And the userspace deamon is a handler handling page fault,
> and trying to write to another file (fsA_fd2) on fsA again
> and blocked on thread B (file_start_write(), rwsem read).
> 
> because of the specific locking timing is `R->W->R`, at least
> the whole workflow won't proceed so the related processes
> (and fsA above) will be stuck.

Thanks a lot for the good summary! It has helped me to look at the problem
from a bit different angle and I don't think it's actually specific to
filesystem freezing or fanotify HSM. Consider the following simple setup:

You have XFS filesystem fsA. On it you have a file imageA and you setup
loop device loop0 over imageA and mount it as some filesystem fsB. Now you
do write(imageA_fd, mmap(fsB_fd)) and you get a nice system deadlock (tried
that and it really works :)). The problem is that write to imageA_fd
acquires exclusively i_rwsem in imageA, then goes on to fault page on fsB
which maps to a read from loop0 which maps to a read from imageA and
xfs_file_read_iter() wants to acquire i_rwsem for imageA again.

I would not consider this a real DoS vector since you generally need
priviledge to perform such nasty write but still isn't not great the system
can be deadlocked like this and I think we should fix cases like this.

Arguably, to address this particular case, we could block writes (and
reads!) to a file that's used as a backing loop device file similarly
as we do that for swapfiles but here I'm seriously worried of userspace
regressions. I've seen userspace scripts setting up loop device and *then*
running mkfs or editing partition table directly on the backing file.

Also with the use of filesystem freezing on fsA, write to any target
file (not only imageA) in fsA can create such deadlock if the page fault on
the stacked filesystem ends up wanting to do some modification of imageA.
So I don't think blocking IO to the loop device backing file will be even a
complete fix.

I think the underlying reason why that write(imageA_fd, mmap(fsB_fd))
pattern causes issues is that it essentially creates a loop in the stacked
filesystem graph which can cause all sorts of lock inversions compared to
the natural order of taking locks for the stacked filesystems. So can we
somehow forbid that? Or do we simply not care and if priviledged user does
something stupid, he gets to reboot the machine? Any other ideas?

								Honza

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Stacking filesystem deadlocks [was Re: Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup]
  2026-06-10 12:21                               ` Stacking filesystem deadlocks [was Re: Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup] Jan Kara
@ 2026-06-10 13:18                                 ` Gao Xiang
  2026-06-10 15:40                                   ` Amir Goldstein
  0 siblings, 1 reply; 14+ messages in thread
From: Gao Xiang @ 2026-06-10 13:18 UTC (permalink / raw)
  To: Jan Kara
  Cc: linux-fsdevel@vger.kernel.org, Christian Brauner, Miklos Szeredi,
	Gao Xiang, Song Liu, Amir Goldstein

Hi Jan,

On 2026/6/10 20:21, Jan Kara wrote:
> Hi!
> 
> On Wed 10-06-26 16:17:33, Gao Xiang wrote:
>> Amir just suggested that I posted a long off-list discussion
>> on the list right now, but since it seems to relate to
>> `sb->s_writers.rw_sem` locking design, I am not quite sure
>> how deep I could help generally: But here it goes.
>>
>> The background is that we have discussed a generic deadlock
>> timing for several months as below:
>>
>> fsA is a filesystem which supports fsfreeze, such as EXT4/XFS/...
>>
>> fsB is a filesystem which have some relationship with a
>> userspace daemon (e.g. a filesystem with fanotify HSM hooks /
>>    fanotify + EROFS file-backed mounts / a FUSE filesystem or
>>    a filesystem backed by a virtual block device)
>>
>> Thread A                           Thread B                                 Userspace deamon
>>   write(fsA_fd, mmap(fsB_fd))
>>    file_start_write()
>>     (take SB_FREEZE_WRITE read lock)
>>
>>     handle fsB mmap fault read
>>      -> notify userspace and wait
>>
>>                                     freeze_super
>>                                     (try to take SB_FREEZE_WRITE write lock)
>>                                                                              received/handling fsB mmap read request
>>                                                                              (do random something...)
>>                                                                              write(fsA_fd2)
>>                                                                               (take SB_FREEZE_WRITE read lock)
>> The problem timing here is thread A does
>> `write(fsA_fd, mmap(fsB_fd))` => file_start_write() (rwsem read),
>> then hits page fault and wait for userspace deamon to finish
>> the request;
>>
>> Thread B does fsfreeze on fsA so it is waiting a write lock
>> (sb_wait_write(), rwsem write) and blocked on thread A;
>>
>> And the userspace deamon is a handler handling page fault,
>> and trying to write to another file (fsA_fd2) on fsA again
>> and blocked on thread B (file_start_write(), rwsem read).
>>
>> because of the specific locking timing is `R->W->R`, at least
>> the whole workflow won't proceed so the related processes
>> (and fsA above) will be stuck.
> 
> Thanks a lot for the good summary! It has helped me to look at the problem
> from a bit different angle and I don't think it's actually specific to
> filesystem freezing or fanotify HSM. Consider the following simple setup:
> 
> You have XFS filesystem fsA. On it you have a file imageA and you setup
> loop device loop0 over imageA and mount it as some filesystem fsB. Now you
> do write(imageA_fd, mmap(fsB_fd)) and you get a nice system deadlock (tried
> that and it really works :)). The problem is that write to imageA_fd
> acquires exclusively i_rwsem in imageA, then goes on to fault page on fsB
> which maps to a read from loop0 which maps to a read from imageA and
> xfs_file_read_iter() wants to acquire i_rwsem for imageA again.

Ah, great catch, but mainly because XFS takes i_rwsem read lock
on read paths LOL.

Yes, yet this deadlock sounds more like a fixed flow/pattern, which
is somewhat slightly different than fsfreeze issue since fanotify
needs racy between threads (causing RWR order casually and cause
Linux rwsem model deadlock) and may not need stacking filesystems.

I'm not sure how this deadlock can be avoided easily since it's
a deterministic single-thread timing, but as you said, at least it
needs privilege (like fanotify which needs privilege too).

btw, I remembered Miklos once mentioned some i_rwsem deadlock cases
too, maybe i_rwsem could cause some generic issue.

> 
> I would not consider this a real DoS vector since you generally need
> priviledge to perform such nasty write but still isn't not great the system
> can be deadlocked like this and I think we should fix cases like this.
> 
> Arguably, to address this particular case, we could block writes (and
> reads!) to a file that's used as a backing loop device file similarly
> as we do that for swapfiles but here I'm seriously worried of userspace
> regressions. I've seen userspace scripts setting up loop device and *then*
> running mkfs or editing partition table directly on the backing file.

but I think erofs file-backed mounts can just simply block write to
the backing file to avoid this: I don't think support this is useful,
but yes, it's also priviledged...

Thanks,
Gao Xiang

> 
> Also with the use of filesystem freezing on fsA, write to any target
> file (not only imageA) in fsA can create such deadlock if the page fault on
> the stacked filesystem ends up wanting to do some modification of imageA.
> So I don't think blocking IO to the loop device backing file will be even a
> complete fix.
> 
> I think the underlying reason why that write(imageA_fd, mmap(fsB_fd))
> pattern causes issues is that it essentially creates a loop in the stacked
> filesystem graph which can cause all sorts of lock inversions compared to
> the natural order of taking locks for the stacked filesystems. So can we
> somehow forbid that? Or do we simply not care and if priviledged user does
> something stupid, he gets to reboot the machine? Any other ideas?
> 
> 								Honza
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Stacking filesystem deadlocks [was Re: Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup]
  2026-06-10 13:18                                 ` Gao Xiang
@ 2026-06-10 15:40                                   ` Amir Goldstein
  2026-06-10 15:58                                     ` Gao Xiang
  2026-06-11 11:32                                     ` Jan Kara
  0 siblings, 2 replies; 14+ messages in thread
From: Amir Goldstein @ 2026-06-10 15:40 UTC (permalink / raw)
  To: Gao Xiang
  Cc: Jan Kara, linux-fsdevel@vger.kernel.org, Christian Brauner,
	Miklos Szeredi, Gao Xiang, Song Liu

On Wed, Jun 10, 2026 at 3:18 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
>
> Hi Jan,
>
> On 2026/6/10 20:21, Jan Kara wrote:
> > Hi!
> >
> > On Wed 10-06-26 16:17:33, Gao Xiang wrote:
> >> Amir just suggested that I posted a long off-list discussion
> >> on the list right now, but since it seems to relate to
> >> `sb->s_writers.rw_sem` locking design, I am not quite sure
> >> how deep I could help generally: But here it goes.
> >>
> >> The background is that we have discussed a generic deadlock
> >> timing for several months as below:
> >>
> >> fsA is a filesystem which supports fsfreeze, such as EXT4/XFS/...
> >>
> >> fsB is a filesystem which have some relationship with a
> >> userspace daemon (e.g. a filesystem with fanotify HSM hooks /
> >>    fanotify + EROFS file-backed mounts / a FUSE filesystem or
> >>    a filesystem backed by a virtual block device)
> >>
> >> Thread A                           Thread B                                 Userspace deamon
> >>   write(fsA_fd, mmap(fsB_fd))
> >>    file_start_write()
> >>     (take SB_FREEZE_WRITE read lock)
> >>
> >>     handle fsB mmap fault read
> >>      -> notify userspace and wait
> >>
> >>                                     freeze_super
> >>                                     (try to take SB_FREEZE_WRITE write lock)
> >>                                                                              received/handling fsB mmap read request
> >>                                                                              (do random something...)
> >>                                                                              write(fsA_fd2)
> >>                                                                               (take SB_FREEZE_WRITE read lock)
> >> The problem timing here is thread A does
> >> `write(fsA_fd, mmap(fsB_fd))` => file_start_write() (rwsem read),
> >> then hits page fault and wait for userspace deamon to finish
> >> the request;
> >>
> >> Thread B does fsfreeze on fsA so it is waiting a write lock
> >> (sb_wait_write(), rwsem write) and blocked on thread A;
> >>
> >> And the userspace deamon is a handler handling page fault,
> >> and trying to write to another file (fsA_fd2) on fsA again
> >> and blocked on thread B (file_start_write(), rwsem read).
> >>
> >> because of the specific locking timing is `R->W->R`, at least
> >> the whole workflow won't proceed so the related processes
> >> (and fsA above) will be stuck.
> >
> > Thanks a lot for the good summary! It has helped me to look at the problem
> > from a bit different angle and I don't think it's actually specific to
> > filesystem freezing or fanotify HSM. Consider the following simple setup:
> >
> > You have XFS filesystem fsA. On it you have a file imageA and you setup
> > loop device loop0 over imageA and mount it as some filesystem fsB. Now you
> > do write(imageA_fd, mmap(fsB_fd)) and you get a nice system deadlock (tried
> > that and it really works :)). The problem is that write to imageA_fd
> > acquires exclusively i_rwsem in imageA, then goes on to fault page on fsB
> > which maps to a read from loop0 which maps to a read from imageA and
> > xfs_file_read_iter() wants to acquire i_rwsem for imageA again.
>
> Ah, great catch, but mainly because XFS takes i_rwsem read lock
> on read paths LOL.
>
> Yes, yet this deadlock sounds more like a fixed flow/pattern, which
> is somewhat slightly different than fsfreeze issue since fanotify
> needs racy between threads (causing RWR order casually and cause
> Linux rwsem model deadlock) and may not need stacking filesystems.
>
> I'm not sure how this deadlock can be avoided easily since it's
> a deterministic single-thread timing, but as you said, at least it
> needs privilege (like fanotify which needs privilege too).
>
> btw, I remembered Miklos once mentioned some i_rwsem deadlock cases
> too, maybe i_rwsem could cause some generic issue.
>
> >
> > I would not consider this a real DoS vector since you generally need
> > priviledge to perform such nasty write but still isn't not great the system
> > can be deadlocked like this and I think we should fix cases like this.
> >
> > Arguably, to address this particular case, we could block writes (and
> > reads!) to a file that's used as a backing loop device file similarly
> > as we do that for swapfiles but here I'm seriously worried of userspace
> > regressions. I've seen userspace scripts setting up loop device and *then*
> > running mkfs or editing partition table directly on the backing file.
>
> but I think erofs file-backed mounts can just simply block write to
> the backing file to avoid this: I don't think support this is useful,
> but yes, it's also priviledged...

Block write to backing file? Didn't you want to use pre-content events
to fill the backing file lazily? How will you fill it without writing to it?

> >
> > Also with the use of filesystem freezing on fsA, write to any target
> > file (not only imageA) in fsA can create such deadlock if the page fault on
> > the stacked filesystem ends up wanting to do some modification of imageA.
> > So I don't think blocking IO to the loop device backing file will be even a
> > complete fix.
> >
> > I think the underlying reason why that write(imageA_fd, mmap(fsB_fd))
> > pattern causes issues is that it essentially creates a loop in the stacked
> > filesystem graph which can cause all sorts of lock inversions compared to
> > the natural order of taking locks for the stacked filesystems. So can we
> > somehow forbid that? Or do we simply not care and if priviledged user does
> > something stupid, he gets to reboot the machine? Any other ideas?

Very unbacked idea, but something along the lines of:

/*
 * sb_{start,end}_write() scope an "sb write transaction"
 */

@@ -1283,6 +1283,8 @@ struct task_struct {
        /* Journalling filesystem info: */
        void                            *journal_info;

+       /* Stacked filesystem write transactions: */
+       struct super_block
*sb_write_stack[SB_WRITE_MAX_STACK_DEPTH];
+       int                                         sb_write_stack_depth;
+
        /* Stacked block device info: */
        struct bio_list                 *bio_list;

I don't know what a decent value of SB_WRITE_MAX_STACK_DEPTH
would be but its not the same as FILESYSTEM_MAX_STACK_DEPTH,
because loop/nbd/backing device also contributes to sb_write_stack_depth.

The rule is that sb_start_write(sb) pushes the sb to the tx stack
after checking no loops in the stack.

In the common non-stacked case, there are no checks so the push/pop
should be quite cheap compared to the sb_writers locks.

For the read side I hope that we do not need to track "read transactions"
in the stack and that it is enough to check the write stack in page faults and
stacked reads in loop/nbd/backing.

Note that the unscoped kiocb_start_write() in aio/uring will need to be
extended so that the write transaction scope will cover the io submission
to cover the page faults in input buffers.

I have no idea about the feasibility of this and how much of the holes
this actually closes but if looking at the narrow use case of pre-content
events it might just be enough.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Stacking filesystem deadlocks [was Re: Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup]
  2026-06-10 15:40                                   ` Amir Goldstein
@ 2026-06-10 15:58                                     ` Gao Xiang
  2026-06-11 11:32                                     ` Jan Kara
  1 sibling, 0 replies; 14+ messages in thread
From: Gao Xiang @ 2026-06-10 15:58 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Jan Kara, linux-fsdevel@vger.kernel.org, Christian Brauner,
	Miklos Szeredi, Gao Xiang, Song Liu



On 2026/6/10 23:40, Amir Goldstein wrote:
> On Wed, Jun 10, 2026 at 3:18 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
>>
>> Hi Jan,
>>
>> On 2026/6/10 20:21, Jan Kara wrote:
>>> Hi!
>>>
>>> On Wed 10-06-26 16:17:33, Gao Xiang wrote:
>>>> Amir just suggested that I posted a long off-list discussion
>>>> on the list right now, but since it seems to relate to
>>>> `sb->s_writers.rw_sem` locking design, I am not quite sure
>>>> how deep I could help generally: But here it goes.
>>>>
>>>> The background is that we have discussed a generic deadlock
>>>> timing for several months as below:
>>>>
>>>> fsA is a filesystem which supports fsfreeze, such as EXT4/XFS/...
>>>>
>>>> fsB is a filesystem which have some relationship with a
>>>> userspace daemon (e.g. a filesystem with fanotify HSM hooks /
>>>>     fanotify + EROFS file-backed mounts / a FUSE filesystem or
>>>>     a filesystem backed by a virtual block device)
>>>>
>>>> Thread A                           Thread B                                 Userspace deamon
>>>>    write(fsA_fd, mmap(fsB_fd))
>>>>     file_start_write()
>>>>      (take SB_FREEZE_WRITE read lock)
>>>>
>>>>      handle fsB mmap fault read
>>>>       -> notify userspace and wait
>>>>
>>>>                                      freeze_super
>>>>                                      (try to take SB_FREEZE_WRITE write lock)
>>>>                                                                               received/handling fsB mmap read request
>>>>                                                                               (do random something...)
>>>>                                                                               write(fsA_fd2)
>>>>                                                                                (take SB_FREEZE_WRITE read lock)
>>>> The problem timing here is thread A does
>>>> `write(fsA_fd, mmap(fsB_fd))` => file_start_write() (rwsem read),
>>>> then hits page fault and wait for userspace deamon to finish
>>>> the request;
>>>>
>>>> Thread B does fsfreeze on fsA so it is waiting a write lock
>>>> (sb_wait_write(), rwsem write) and blocked on thread A;
>>>>
>>>> And the userspace deamon is a handler handling page fault,
>>>> and trying to write to another file (fsA_fd2) on fsA again
>>>> and blocked on thread B (file_start_write(), rwsem read).
>>>>
>>>> because of the specific locking timing is `R->W->R`, at least
>>>> the whole workflow won't proceed so the related processes
>>>> (and fsA above) will be stuck.
>>>
>>> Thanks a lot for the good summary! It has helped me to look at the problem
>>> from a bit different angle and I don't think it's actually specific to
>>> filesystem freezing or fanotify HSM. Consider the following simple setup:
>>>
>>> You have XFS filesystem fsA. On it you have a file imageA and you setup
>>> loop device loop0 over imageA and mount it as some filesystem fsB. Now you
>>> do write(imageA_fd, mmap(fsB_fd)) and you get a nice system deadlock (tried
>>> that and it really works :)). The problem is that write to imageA_fd
>>> acquires exclusively i_rwsem in imageA, then goes on to fault page on fsB
>>> which maps to a read from loop0 which maps to a read from imageA and
>>> xfs_file_read_iter() wants to acquire i_rwsem for imageA again.
>>
>> Ah, great catch, but mainly because XFS takes i_rwsem read lock
>> on read paths LOL.
>>
>> Yes, yet this deadlock sounds more like a fixed flow/pattern, which
>> is somewhat slightly different than fsfreeze issue since fanotify
>> needs racy between threads (causing RWR order casually and cause
>> Linux rwsem model deadlock) and may not need stacking filesystems.
>>
>> I'm not sure how this deadlock can be avoided easily since it's
>> a deterministic single-thread timing, but as you said, at least it
>> needs privilege (like fanotify which needs privilege too).
>>
>> btw, I remembered Miklos once mentioned some i_rwsem deadlock cases
>> too, maybe i_rwsem could cause some generic issue.
>>
>>>
>>> I would not consider this a real DoS vector since you generally need
>>> priviledge to perform such nasty write but still isn't not great the system
>>> can be deadlocked like this and I think we should fix cases like this.
>>>
>>> Arguably, to address this particular case, we could block writes (and
>>> reads!) to a file that's used as a backing loop device file similarly
>>> as we do that for swapfiles but here I'm seriously worried of userspace
>>> regressions. I've seen userspace scripts setting up loop device and *then*
>>> running mkfs or editing partition table directly on the backing file.
>>
>> but I think erofs file-backed mounts can just simply block write to
>> the backing file to avoid this: I don't think support this is useful,
>> but yes, it's also priviledged...
> 
> Block write to backing file? Didn't you want to use pre-content events
> to fill the backing file lazily? How will you fill it without writing to it?

Sigh, yes and sorry, I think at least we could find a way to mark
the image file (image files can be tagged like swapfiles) write
and block such mmap read for example: I don't think support this
is useful; but maybe just ignore my comment on this whole thing
for now.

Thanks,
Gao Xiang

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Stacking filesystem deadlocks [was Re: Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup]
  2026-06-10 15:40                                   ` Amir Goldstein
  2026-06-10 15:58                                     ` Gao Xiang
@ 2026-06-11 11:32                                     ` Jan Kara
  2026-06-11 15:25                                       ` Amir Goldstein
  1 sibling, 1 reply; 14+ messages in thread
From: Jan Kara @ 2026-06-11 11:32 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Gao Xiang, Jan Kara, linux-fsdevel@vger.kernel.org,
	Christian Brauner, Miklos Szeredi, Gao Xiang, Song Liu

On Wed 10-06-26 17:40:27, Amir Goldstein wrote:
> On Wed, Jun 10, 2026 at 3:18 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> > On 2026/6/10 20:21, Jan Kara wrote:
> > > On Wed 10-06-26 16:17:33, Gao Xiang wrote:
> > >> Amir just suggested that I posted a long off-list discussion
> > >> on the list right now, but since it seems to relate to
> > >> `sb->s_writers.rw_sem` locking design, I am not quite sure
> > >> how deep I could help generally: But here it goes.
> > >>
> > >> The background is that we have discussed a generic deadlock
> > >> timing for several months as below:
> > >>
> > >> fsA is a filesystem which supports fsfreeze, such as EXT4/XFS/...
> > >>
> > >> fsB is a filesystem which have some relationship with a
> > >> userspace daemon (e.g. a filesystem with fanotify HSM hooks /
> > >>    fanotify + EROFS file-backed mounts / a FUSE filesystem or
> > >>    a filesystem backed by a virtual block device)
> > >>
> > >> Thread A                           Thread B                                 Userspace deamon
> > >>   write(fsA_fd, mmap(fsB_fd))
> > >>    file_start_write()
> > >>     (take SB_FREEZE_WRITE read lock)
> > >>
> > >>     handle fsB mmap fault read
> > >>      -> notify userspace and wait
> > >>
> > >>                                     freeze_super
> > >>                                     (try to take SB_FREEZE_WRITE write lock)
> > >>                                                                              received/handling fsB mmap read request
> > >>                                                                              (do random something...)
> > >>                                                                              write(fsA_fd2)
> > >>                                                                               (take SB_FREEZE_WRITE read lock)
> > >> The problem timing here is thread A does
> > >> `write(fsA_fd, mmap(fsB_fd))` => file_start_write() (rwsem read),
> > >> then hits page fault and wait for userspace deamon to finish
> > >> the request;
> > >>
> > >> Thread B does fsfreeze on fsA so it is waiting a write lock
> > >> (sb_wait_write(), rwsem write) and blocked on thread A;
> > >>
> > >> And the userspace deamon is a handler handling page fault,
> > >> and trying to write to another file (fsA_fd2) on fsA again
> > >> and blocked on thread B (file_start_write(), rwsem read).
> > >>
> > >> because of the specific locking timing is `R->W->R`, at least
> > >> the whole workflow won't proceed so the related processes
> > >> (and fsA above) will be stuck.
> > >
> > > Thanks a lot for the good summary! It has helped me to look at the problem
> > > from a bit different angle and I don't think it's actually specific to
> > > filesystem freezing or fanotify HSM. Consider the following simple setup:
> > >
> > > You have XFS filesystem fsA. On it you have a file imageA and you setup
> > > loop device loop0 over imageA and mount it as some filesystem fsB. Now you
> > > do write(imageA_fd, mmap(fsB_fd)) and you get a nice system deadlock (tried
> > > that and it really works :)). The problem is that write to imageA_fd
> > > acquires exclusively i_rwsem in imageA, then goes on to fault page on fsB
> > > which maps to a read from loop0 which maps to a read from imageA and
> > > xfs_file_read_iter() wants to acquire i_rwsem for imageA again.
> >
> > Ah, great catch, but mainly because XFS takes i_rwsem read lock
> > on read paths LOL.

Well, yes, that makes things particularly easy to hit but e.g. with direct
IO much more filesystems (in particular everybody using iomap) than just XFS
are prone to similar issues.

> > Yes, yet this deadlock sounds more like a fixed flow/pattern, which
> > is somewhat slightly different than fsfreeze issue since fanotify
> > needs racy between threads (causing RWR order casually and cause
> > Linux rwsem model deadlock) and may not need stacking filesystems.

This example demostrates that the problem isn't specific to HSM or filesystem
freezing. Yes, filesystem freezing given its filesystem wide scope is
expanding the deadlock potential to more cases but the underlying problem
really is that if you manage to do a syscall that holds locks on filesystem
A and then ends up accessing filesystem B that's stacked on top of A, you
are violating the expected lock ordering and this can lead to deadlocks.

> > > Also with the use of filesystem freezing on fsA, write to any target
> > > file (not only imageA) in fsA can create such deadlock if the page fault on
> > > the stacked filesystem ends up wanting to do some modification of imageA.
> > > So I don't think blocking IO to the loop device backing file will be even a
> > > complete fix.
> > >
> > > I think the underlying reason why that write(imageA_fd, mmap(fsB_fd))
> > > pattern causes issues is that it essentially creates a loop in the stacked
> > > filesystem graph which can cause all sorts of lock inversions compared to
> > > the natural order of taking locks for the stacked filesystems. So can we
> > > somehow forbid that? Or do we simply not care and if priviledged user does
> > > something stupid, he gets to reboot the machine? Any other ideas?
> 
> Very unbacked idea, but something along the lines of:
> 
> /*
>  * sb_{start,end}_write() scope an "sb write transaction"
>  */
> 
> @@ -1283,6 +1283,8 @@ struct task_struct {
>         /* Journalling filesystem info: */
>         void                            *journal_info;
> 
> +       /* Stacked filesystem write transactions: */
> +       struct super_block
> *sb_write_stack[SB_WRITE_MAX_STACK_DEPTH];
> +       int                                         sb_write_stack_depth;
> +
>         /* Stacked block device info: */
>         struct bio_list                 *bio_list;
> 
> I don't know what a decent value of SB_WRITE_MAX_STACK_DEPTH
> would be but its not the same as FILESYSTEM_MAX_STACK_DEPTH,
> because loop/nbd/backing device also contributes to sb_write_stack_depth.

I was thinking about something similar but then realized this won't fly -
the deadlock chain is often split among multiple processes. Even in the
case I've shown above there's the task doing the nasty write which holds
i_rwsem on imageA and then a workqueue thread that actually does the loopback
device IO which is trying to take it as well. No single process creates
a loop, only all of them together form the loop.

> The rule is that sb_start_write(sb) pushes the sb to the tx stack
> after checking no loops in the stack.
> 
> In the common non-stacked case, there are no checks so the push/pop
> should be quite cheap compared to the sb_writers locks.
> 
> For the read side I hope that we do not need to track "read transactions"
> in the stack and that it is enough to check the write stack in page faults and
> stacked reads in loop/nbd/backing.

That's another problem. Reads can create deadlocks as well. If you do
read(imageA_fd, mmap(fsB_fd)) in the setup I've outlined in my previous
email on XFS, you are holding i_rwsem for read for imageA and then doing a
write fault to fsB which translates to a read from imageA which needs
shared i_rwsem for imageA again. If you are lucky, this can pass but if
someone tries to acquire imageA's i_rwsem for write inbetween (and blocks),
your second shared i_rwsem acquisition will block and you have a deadlock.

> Note that the unscoped kiocb_start_write() in aio/uring will need to be
> extended so that the write transaction scope will cover the io submission
> to cover the page faults in input buffers.
> 
> I have no idea about the feasibility of this and how much of the holes
> this actually closes but if looking at the narrow use case of pre-content
> events it might just be enough.

What looks like the most feasible approach to me is to record in struct
task we are doing a read/write syscall for fsA and in the page fault
handler once we have the VMA we can check that fsB isn't fsA's ancestor in
the "filesystem dependency graph". Making the check cheap enough should be
doable (we could for example do the loop check traversal only if the
stacking depth of fsB is higher than that of fsA, caching the stacking
depth in the superblock, which would deal with 99.99% cases) but maintaining
the dependency graph will take some work...

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Stacking filesystem deadlocks [was Re: Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup]
  2026-06-11 11:32                                     ` Jan Kara
@ 2026-06-11 15:25                                       ` Amir Goldstein
  2026-06-17 13:37                                         ` Christian Brauner
  2026-06-17 17:33                                         ` Jan Kara
  0 siblings, 2 replies; 14+ messages in thread
From: Amir Goldstein @ 2026-06-11 15:25 UTC (permalink / raw)
  To: Jan Kara
  Cc: Gao Xiang, linux-fsdevel@vger.kernel.org, Christian Brauner,
	Miklos Szeredi, Gao Xiang, Song Liu

On Thu, Jun 11, 2026 at 1:32 PM Jan Kara <jack@suse.cz> wrote:
>
> On Wed 10-06-26 17:40:27, Amir Goldstein wrote:
> > On Wed, Jun 10, 2026 at 3:18 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> > > On 2026/6/10 20:21, Jan Kara wrote:
> > > > On Wed 10-06-26 16:17:33, Gao Xiang wrote:
> > > >> Amir just suggested that I posted a long off-list discussion
> > > >> on the list right now, but since it seems to relate to
> > > >> `sb->s_writers.rw_sem` locking design, I am not quite sure
> > > >> how deep I could help generally: But here it goes.
> > > >>
> > > >> The background is that we have discussed a generic deadlock
> > > >> timing for several months as below:
> > > >>
> > > >> fsA is a filesystem which supports fsfreeze, such as EXT4/XFS/...
> > > >>
> > > >> fsB is a filesystem which have some relationship with a
> > > >> userspace daemon (e.g. a filesystem with fanotify HSM hooks /
> > > >>    fanotify + EROFS file-backed mounts / a FUSE filesystem or
> > > >>    a filesystem backed by a virtual block device)
> > > >>
> > > >> Thread A                           Thread B                                 Userspace deamon
> > > >>   write(fsA_fd, mmap(fsB_fd))
> > > >>    file_start_write()
> > > >>     (take SB_FREEZE_WRITE read lock)
> > > >>
> > > >>     handle fsB mmap fault read
> > > >>      -> notify userspace and wait
> > > >>
> > > >>                                     freeze_super
> > > >>                                     (try to take SB_FREEZE_WRITE write lock)
> > > >>                                                                              received/handling fsB mmap read request
> > > >>                                                                              (do random something...)
> > > >>                                                                              write(fsA_fd2)
> > > >>                                                                               (take SB_FREEZE_WRITE read lock)
> > > >> The problem timing here is thread A does
> > > >> `write(fsA_fd, mmap(fsB_fd))` => file_start_write() (rwsem read),
> > > >> then hits page fault and wait for userspace deamon to finish
> > > >> the request;
> > > >>
> > > >> Thread B does fsfreeze on fsA so it is waiting a write lock
> > > >> (sb_wait_write(), rwsem write) and blocked on thread A;
> > > >>
> > > >> And the userspace deamon is a handler handling page fault,
> > > >> and trying to write to another file (fsA_fd2) on fsA again
> > > >> and blocked on thread B (file_start_write(), rwsem read).
> > > >>
> > > >> because of the specific locking timing is `R->W->R`, at least
> > > >> the whole workflow won't proceed so the related processes
> > > >> (and fsA above) will be stuck.
> > > >
> > > > Thanks a lot for the good summary! It has helped me to look at the problem
> > > > from a bit different angle and I don't think it's actually specific to
> > > > filesystem freezing or fanotify HSM. Consider the following simple setup:
> > > >
> > > > You have XFS filesystem fsA. On it you have a file imageA and you setup
> > > > loop device loop0 over imageA and mount it as some filesystem fsB. Now you
> > > > do write(imageA_fd, mmap(fsB_fd)) and you get a nice system deadlock (tried
> > > > that and it really works :)). The problem is that write to imageA_fd
> > > > acquires exclusively i_rwsem in imageA, then goes on to fault page on fsB
> > > > which maps to a read from loop0 which maps to a read from imageA and
> > > > xfs_file_read_iter() wants to acquire i_rwsem for imageA again.
> > >
> > > Ah, great catch, but mainly because XFS takes i_rwsem read lock
> > > on read paths LOL.
>
> Well, yes, that makes things particularly easy to hit but e.g. with direct
> IO much more filesystems (in particular everybody using iomap) than just XFS
> are prone to similar issues.
>
> > > Yes, yet this deadlock sounds more like a fixed flow/pattern, which
> > > is somewhat slightly different than fsfreeze issue since fanotify
> > > needs racy between threads (causing RWR order casually and cause
> > > Linux rwsem model deadlock) and may not need stacking filesystems.
>
> This example demostrates that the problem isn't specific to HSM or filesystem
> freezing. Yes, filesystem freezing given its filesystem wide scope is
> expanding the deadlock potential to more cases but the underlying problem
> really is that if you manage to do a syscall that holds locks on filesystem
> A and then ends up accessing filesystem B that's stacked on top of A, you
> are violating the expected lock ordering and this can lead to deadlocks.
>
> > > > Also with the use of filesystem freezing on fsA, write to any target
> > > > file (not only imageA) in fsA can create such deadlock if the page fault on
> > > > the stacked filesystem ends up wanting to do some modification of imageA.
> > > > So I don't think blocking IO to the loop device backing file will be even a
> > > > complete fix.
> > > >
> > > > I think the underlying reason why that write(imageA_fd, mmap(fsB_fd))
> > > > pattern causes issues is that it essentially creates a loop in the stacked
> > > > filesystem graph which can cause all sorts of lock inversions compared to
> > > > the natural order of taking locks for the stacked filesystems. So can we
> > > > somehow forbid that? Or do we simply not care and if priviledged user does
> > > > something stupid, he gets to reboot the machine? Any other ideas?
> >
> > Very unbacked idea, but something along the lines of:
> >
> > /*
> >  * sb_{start,end}_write() scope an "sb write transaction"
> >  */
> >
> > @@ -1283,6 +1283,8 @@ struct task_struct {
> >         /* Journalling filesystem info: */
> >         void                            *journal_info;
> >
> > +       /* Stacked filesystem write transactions: */
> > +       struct super_block
> > *sb_write_stack[SB_WRITE_MAX_STACK_DEPTH];
> > +       int                                         sb_write_stack_depth;
> > +
> >         /* Stacked block device info: */
> >         struct bio_list                 *bio_list;
> >
> > I don't know what a decent value of SB_WRITE_MAX_STACK_DEPTH
> > would be but its not the same as FILESYSTEM_MAX_STACK_DEPTH,
> > because loop/nbd/backing device also contributes to sb_write_stack_depth.
>
> I was thinking about something similar but then realized this won't fly -
> the deadlock chain is often split among multiple processes. Even in the
> case I've shown above there's the task doing the nasty write which holds
> i_rwsem on imageA and then a workqueue thread that actually does the loopback
> device IO which is trying to take it as well. No single process creates
> a loop, only all of them together form the loop.

Yap. I was considering whether we could copy the sb_tx_stack to the kiocb..

>
> > The rule is that sb_start_write(sb) pushes the sb to the tx stack
> > after checking no loops in the stack.
> >
> > In the common non-stacked case, there are no checks so the push/pop
> > should be quite cheap compared to the sb_writers locks.
> >
> > For the read side I hope that we do not need to track "read transactions"
> > in the stack and that it is enough to check the write stack in page faults and
> > stacked reads in loop/nbd/backing.
>
> That's another problem. Reads can create deadlocks as well. If you do
> read(imageA_fd, mmap(fsB_fd)) in the setup I've outlined in my previous
> email on XFS, you are holding i_rwsem for read for imageA and then doing a
> write fault to fsB which translates to a read from imageA which needs
> shared i_rwsem for imageA again. If you are lucky, this can pass but if
> someone tries to acquire imageA's i_rwsem for write inbetween (and blocks),
> your second shared i_rwsem acquisition will block and you have a deadlock.
>

Yeh we could introduce sb_read(sb) scopes but that would be a bit nasty.
The two-fds syscalls (e.g. sendfile()) makes this scoping even more challenging.

> > Note that the unscoped kiocb_start_write() in aio/uring will need to be
> > extended so that the write transaction scope will cover the io submission
> > to cover the page faults in input buffers.
> >
> > I have no idea about the feasibility of this and how much of the holes
> > this actually closes but if looking at the narrow use case of pre-content
> > events it might just be enough.
>
> What looks like the most feasible approach to me is to record in struct
> task we are doing a read/write syscall for fsA

read/write_iter could be nested within stacked fs, so we really need a stack
of transactions, but this one could be [FILESYSTEM_MAX_STACK_DEPTH]

> and in the page fault
> handler once we have the VMA we can check that fsB isn't fsA's ancestor in
> the "filesystem dependency graph". Making the check cheap enough should be
> doable (we could for example do the loop check traversal only if the
> stacking depth of fsB is higher than that of fsA, caching the stacking
> depth in the superblock, which would deal with 99.99% cases) but maintaining
> the dependency graph will take some work...

My crystal ball tells me we are probably not going to do that :-/
But if we did, it would have been nice to validate that LOOP_CHANGE_FD
does not set the backing file of loopA to a file on fsB ;)

If we only wanted to protect the case of pre-content events in erofs
backing file (readonly), is there any solution that you see which is less
complicated?

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Stacking filesystem deadlocks [was Re: Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup]
  2026-06-11 15:25                                       ` Amir Goldstein
@ 2026-06-17 13:37                                         ` Christian Brauner
  2026-06-17 14:23                                           ` Gao Xiang
  2026-06-17 17:09                                           ` Jan Kara
  2026-06-17 17:33                                         ` Jan Kara
  1 sibling, 2 replies; 14+ messages in thread
From: Christian Brauner @ 2026-06-17 13:37 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Jan Kara, Gao Xiang, linux-fsdevel@vger.kernel.org,
	Miklos Szeredi, Gao Xiang, Song Liu

On Thu, Jun 11, 2026 at 05:25:56PM +0200, Amir Goldstein wrote:
> On Thu, Jun 11, 2026 at 1:32 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Wed 10-06-26 17:40:27, Amir Goldstein wrote:
> > > On Wed, Jun 10, 2026 at 3:18 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> > > > On 2026/6/10 20:21, Jan Kara wrote:
> > > > > On Wed 10-06-26 16:17:33, Gao Xiang wrote:
> > > > >> Amir just suggested that I posted a long off-list discussion
> > > > >> on the list right now, but since it seems to relate to
> > > > >> `sb->s_writers.rw_sem` locking design, I am not quite sure
> > > > >> how deep I could help generally: But here it goes.
> > > > >>
> > > > >> The background is that we have discussed a generic deadlock
> > > > >> timing for several months as below:
> > > > >>
> > > > >> fsA is a filesystem which supports fsfreeze, such as EXT4/XFS/...
> > > > >>
> > > > >> fsB is a filesystem which have some relationship with a
> > > > >> userspace daemon (e.g. a filesystem with fanotify HSM hooks /
> > > > >>    fanotify + EROFS file-backed mounts / a FUSE filesystem or
> > > > >>    a filesystem backed by a virtual block device)
> > > > >>
> > > > >> Thread A                           Thread B                                 Userspace deamon
> > > > >>   write(fsA_fd, mmap(fsB_fd))
> > > > >>    file_start_write()
> > > > >>     (take SB_FREEZE_WRITE read lock)
> > > > >>
> > > > >>     handle fsB mmap fault read
> > > > >>      -> notify userspace and wait
> > > > >>
> > > > >>                                     freeze_super
> > > > >>                                     (try to take SB_FREEZE_WRITE write lock)
> > > > >>                                                                              received/handling fsB mmap read request
> > > > >>                                                                              (do random something...)
> > > > >>                                                                              write(fsA_fd2)
> > > > >>                                                                               (take SB_FREEZE_WRITE read lock)
> > > > >> The problem timing here is thread A does
> > > > >> `write(fsA_fd, mmap(fsB_fd))` => file_start_write() (rwsem read),
> > > > >> then hits page fault and wait for userspace deamon to finish
> > > > >> the request;
> > > > >>
> > > > >> Thread B does fsfreeze on fsA so it is waiting a write lock
> > > > >> (sb_wait_write(), rwsem write) and blocked on thread A;
> > > > >>
> > > > >> And the userspace deamon is a handler handling page fault,
> > > > >> and trying to write to another file (fsA_fd2) on fsA again
> > > > >> and blocked on thread B (file_start_write(), rwsem read).
> > > > >>
> > > > >> because of the specific locking timing is `R->W->R`, at least
> > > > >> the whole workflow won't proceed so the related processes
> > > > >> (and fsA above) will be stuck.
> > > > >
> > > > > Thanks a lot for the good summary! It has helped me to look at the problem
> > > > > from a bit different angle and I don't think it's actually specific to
> > > > > filesystem freezing or fanotify HSM. Consider the following simple setup:
> > > > >
> > > > > You have XFS filesystem fsA. On it you have a file imageA and you setup
> > > > > loop device loop0 over imageA and mount it as some filesystem fsB. Now you
> > > > > do write(imageA_fd, mmap(fsB_fd)) and you get a nice system deadlock (tried
> > > > > that and it really works :)). The problem is that write to imageA_fd
> > > > > acquires exclusively i_rwsem in imageA, then goes on to fault page on fsB
> > > > > which maps to a read from loop0 which maps to a read from imageA and
> > > > > xfs_file_read_iter() wants to acquire i_rwsem for imageA again.
> > > >
> > > > Ah, great catch, but mainly because XFS takes i_rwsem read lock
> > > > on read paths LOL.
> >
> > Well, yes, that makes things particularly easy to hit but e.g. with direct
> > IO much more filesystems (in particular everybody using iomap) than just XFS
> > are prone to similar issues.
> >
> > > > Yes, yet this deadlock sounds more like a fixed flow/pattern, which
> > > > is somewhat slightly different than fsfreeze issue since fanotify
> > > > needs racy between threads (causing RWR order casually and cause
> > > > Linux rwsem model deadlock) and may not need stacking filesystems.
> >
> > This example demostrates that the problem isn't specific to HSM or filesystem
> > freezing. Yes, filesystem freezing given its filesystem wide scope is
> > expanding the deadlock potential to more cases but the underlying problem
> > really is that if you manage to do a syscall that holds locks on filesystem
> > A and then ends up accessing filesystem B that's stacked on top of A, you
> > are violating the expected lock ordering and this can lead to deadlocks.
> >
> > > > > Also with the use of filesystem freezing on fsA, write to any target
> > > > > file (not only imageA) in fsA can create such deadlock if the page fault on
> > > > > the stacked filesystem ends up wanting to do some modification of imageA.
> > > > > So I don't think blocking IO to the loop device backing file will be even a
> > > > > complete fix.
> > > > >
> > > > > I think the underlying reason why that write(imageA_fd, mmap(fsB_fd))
> > > > > pattern causes issues is that it essentially creates a loop in the stacked
> > > > > filesystem graph which can cause all sorts of lock inversions compared to
> > > > > the natural order of taking locks for the stacked filesystems. So can we
> > > > > somehow forbid that? Or do we simply not care and if priviledged user does
> > > > > something stupid, he gets to reboot the machine? Any other ideas?
> > >
> > > Very unbacked idea, but something along the lines of:
> > >
> > > /*
> > >  * sb_{start,end}_write() scope an "sb write transaction"
> > >  */
> > >
> > > @@ -1283,6 +1283,8 @@ struct task_struct {
> > >         /* Journalling filesystem info: */
> > >         void                            *journal_info;
> > >
> > > +       /* Stacked filesystem write transactions: */
> > > +       struct super_block
> > > *sb_write_stack[SB_WRITE_MAX_STACK_DEPTH];
> > > +       int                                         sb_write_stack_depth;
> > > +
> > >         /* Stacked block device info: */
> > >         struct bio_list                 *bio_list;
> > >
> > > I don't know what a decent value of SB_WRITE_MAX_STACK_DEPTH
> > > would be but its not the same as FILESYSTEM_MAX_STACK_DEPTH,
> > > because loop/nbd/backing device also contributes to sb_write_stack_depth.
> >
> > I was thinking about something similar but then realized this won't fly -
> > the deadlock chain is often split among multiple processes. Even in the
> > case I've shown above there's the task doing the nasty write which holds
> > i_rwsem on imageA and then a workqueue thread that actually does the loopback
> > device IO which is trying to take it as well. No single process creates
> > a loop, only all of them together form the loop.
> 
> Yap. I was considering whether we could copy the sb_tx_stack to the kiocb..
> 
> >
> > > The rule is that sb_start_write(sb) pushes the sb to the tx stack
> > > after checking no loops in the stack.
> > >
> > > In the common non-stacked case, there are no checks so the push/pop
> > > should be quite cheap compared to the sb_writers locks.
> > >
> > > For the read side I hope that we do not need to track "read transactions"
> > > in the stack and that it is enough to check the write stack in page faults and
> > > stacked reads in loop/nbd/backing.
> >
> > That's another problem. Reads can create deadlocks as well. If you do
> > read(imageA_fd, mmap(fsB_fd)) in the setup I've outlined in my previous
> > email on XFS, you are holding i_rwsem for read for imageA and then doing a
> > write fault to fsB which translates to a read from imageA which needs
> > shared i_rwsem for imageA again. If you are lucky, this can pass but if
> > someone tries to acquire imageA's i_rwsem for write inbetween (and blocks),
> > your second shared i_rwsem acquisition will block and you have a deadlock.
> >
> 
> Yeh we could introduce sb_read(sb) scopes but that would be a bit nasty.
> The two-fds syscalls (e.g. sendfile()) makes this scoping even more challenging.
> 
> > > Note that the unscoped kiocb_start_write() in aio/uring will need to be
> > > extended so that the write transaction scope will cover the io submission
> > > to cover the page faults in input buffers.
> > >
> > > I have no idea about the feasibility of this and how much of the holes
> > > this actually closes but if looking at the narrow use case of pre-content
> > > events it might just be enough.
> >
> > What looks like the most feasible approach to me is to record in struct
> > task we are doing a read/write syscall for fsA
> 
> read/write_iter could be nested within stacked fs, so we really need a stack
> of transactions, but this one could be [FILESYSTEM_MAX_STACK_DEPTH]
> 
> > and in the page fault
> > handler once we have the VMA we can check that fsB isn't fsA's ancestor in
> > the "filesystem dependency graph". Making the check cheap enough should be
> > doable (we could for example do the loop check traversal only if the
> > stacking depth of fsB is higher than that of fsA, caching the stacking
> > depth in the superblock, which would deal with 99.99% cases) but maintaining
> > the dependency graph will take some work...
> 
> My crystal ball tells me we are probably not going to do that :-/
> But if we did, it would have been nice to validate that LOOP_CHANGE_FD
> does not set the backing file of loopA to a file on fsB ;)

LOOP_CHANGE_FD should be removed. I proposed that years ago because it's
problematic in other ways. Everyone agreed as it was effectively unused
I just never bothered pushing it through. So let's not worry about that
let's kill it instead. Here's the patch from 2023:

From 46db77d781d329b7fc7b3245dd295a69026f6ec0 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Mon, 6 Nov 2023 16:29:32 +0100
Subject: [PATCH] loop: remove support for swapping loop fds

We did not find any users of LOOP_CHANGE_FD.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 drivers/block/loop.c | 100 ++-----------------------------------------
 1 file changed, 4 insertions(+), 96 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 9f2d412fc560..ce6cb2125187 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -106,7 +106,7 @@ static DEFINE_MUTEX(loop_validate_mutex);
  *
  * Since loop_validate_file() traverses on other "struct loop_device" if
  * is_loop_device() is true, we need a global lock for serializing concurrent
- * loop_configure()/loop_change_fd()/__loop_clr_fd() calls.
+ * loop_configure()/__loop_clr_fd() calls.
  */
 static int loop_global_lock_killable(struct loop_device *lo, bool global)
 {
@@ -554,97 +554,6 @@ static int loop_validate_file(struct file *file, struct block_device *bdev)
 	return 0;
 }
 
-/*
- * loop_change_fd switched the backing store of a loopback device to
- * a new file. This is useful for operating system installers to free up
- * the original file and in High Availability environments to switch to
- * an alternative location for the content in case of server meltdown.
- * This can only work if the loop device is used read-only, and if the
- * new backing store is the same size and type as the old backing store.
- */
-static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
-			  unsigned int arg)
-{
-	struct file *file = fget(arg);
-	struct file *old_file;
-	int error;
-	bool partscan;
-	bool is_loop;
-
-	if (!file)
-		return -EBADF;
-
-	/* suppress uevents while reconfiguring the device */
-	dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 1);
-
-	is_loop = is_loop_device(file);
-	error = loop_global_lock_killable(lo, is_loop);
-	if (error)
-		goto out_putf;
-	error = -ENXIO;
-	if (lo->lo_state != Lo_bound)
-		goto out_err;
-
-	/* the loop device has to be read-only */
-	error = -EINVAL;
-	if (!(lo->lo_flags & LO_FLAGS_READ_ONLY))
-		goto out_err;
-
-	error = loop_validate_file(file, bdev);
-	if (error)
-		goto out_err;
-
-	old_file = lo->lo_backing_file;
-
-	error = -EINVAL;
-
-	/* size of the new backing store needs to be the same */
-	if (get_loop_size(lo, file) != get_loop_size(lo, old_file))
-		goto out_err;
-
-	/* and ... switch */
-	disk_force_media_change(lo->lo_disk);
-	blk_mq_freeze_queue(lo->lo_queue);
-	mapping_set_gfp_mask(old_file->f_mapping, lo->old_gfp_mask);
-	lo->lo_backing_file = file;
-	lo->old_gfp_mask = mapping_gfp_mask(file->f_mapping);
-	mapping_set_gfp_mask(file->f_mapping,
-			     lo->old_gfp_mask & ~(__GFP_IO|__GFP_FS));
-	loop_update_dio(lo);
-	blk_mq_unfreeze_queue(lo->lo_queue);
-	partscan = lo->lo_flags & LO_FLAGS_PARTSCAN;
-	loop_global_unlock(lo, is_loop);
-
-	/*
-	 * Flush loop_validate_file() before fput(), for l->lo_backing_file
-	 * might be pointing at old_file which might be the last reference.
-	 */
-	if (!is_loop) {
-		mutex_lock(&loop_validate_mutex);
-		mutex_unlock(&loop_validate_mutex);
-	}
-	/*
-	 * We must drop file reference outside of lo_mutex as dropping
-	 * the file ref can take open_mutex which creates circular locking
-	 * dependency.
-	 */
-	fput(old_file);
-	if (partscan)
-		loop_reread_partitions(lo);
-
-	error = 0;
-done:
-	/* enable and uncork uevent now that we are done */
-	dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 0);
-	return error;
-
-out_err:
-	loop_global_unlock(lo, is_loop);
-out_putf:
-	fput(file);
-	goto done;
-}
-
 /* loop sysfs attributes */
 
 static ssize_t loop_attr_show(struct device *dev, char *page,
@@ -1222,12 +1131,11 @@ static int loop_clr_fd(struct loop_device *lo)
 
 	/*
 	 * Since lo_ioctl() is called without locks held, it is possible that
-	 * loop_configure()/loop_change_fd() and loop_clr_fd() run in parallel.
+	 * loop_configure() and loop_clr_fd() run in parallel.
 	 *
 	 * Therefore, use global lock when setting Lo_rundown state in order to
 	 * make sure that loop_validate_file() will fail if the "struct file"
-	 * which loop_configure()/loop_change_fd() found via fget() was this
-	 * loop device.
+	 * which loop_configure() found via fget() was this loop device.
 	 */
 	err = loop_global_lock_killable(lo, true);
 	if (err)
@@ -1558,7 +1466,7 @@ static int lo_ioctl(struct block_device *bdev, blk_mode_t mode,
 		return loop_configure(lo, mode, bdev, &config);
 	}
 	case LOOP_CHANGE_FD:
-		return loop_change_fd(lo, bdev, arg);
+		return -EOPNOTSUPP;
 	case LOOP_CLR_FD:
 		return loop_clr_fd(lo);
 	case LOOP_SET_STATUS:
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: Stacking filesystem deadlocks [was Re: Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup]
  2026-06-17 13:37                                         ` Christian Brauner
@ 2026-06-17 14:23                                           ` Gao Xiang
  2026-06-17 17:09                                           ` Jan Kara
  1 sibling, 0 replies; 14+ messages in thread
From: Gao Xiang @ 2026-06-17 14:23 UTC (permalink / raw)
  To: Christian Brauner, Amir Goldstein
  Cc: Jan Kara, linux-fsdevel@vger.kernel.org, Miklos Szeredi,
	Gao Xiang, Song Liu



On 2026/6/17 21:37, Christian Brauner wrote:
> On Thu, Jun 11, 2026 at 05:25:56PM +0200, Amir Goldstein wrote:
>> On Thu, Jun 11, 2026 at 1:32 PM Jan Kara <jack@suse.cz> wrote:

...

>>
>> My crystal ball tells me we are probably not going to do that :-/
>> But if we did, it would have been nice to validate that LOOP_CHANGE_FD
>> does not set the backing file of loopA to a file on fsB ;)
> 
> LOOP_CHANGE_FD should be removed. I proposed that years ago because it's
> problematic in other ways. Everyone agreed as it was effectively unused
> I just never bothered pushing it through. So let's not worry about that
> let's kill it instead. Here's the patch from 2023:
> 
>  From 46db77d781d329b7fc7b3245dd295a69026f6ec0 Mon Sep 17 00:00:00 2001
> From: Christian Brauner <brauner@kernel.org>
> Date: Mon, 6 Nov 2023 16:29:32 +0100
> Subject: [PATCH] loop: remove support for swapping loop fds
> 
> We did not find any users of LOOP_CHANGE_FD.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

I am glad if it could be dropped so at least I will not
worry about missing features if fs shutdown is
implemented in erofs.

Thanks,
Gao Xiang

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Stacking filesystem deadlocks [was Re: Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup]
  2026-06-17 13:37                                         ` Christian Brauner
  2026-06-17 14:23                                           ` Gao Xiang
@ 2026-06-17 17:09                                           ` Jan Kara
  2026-06-19  7:25                                             ` Christian Brauner
  1 sibling, 1 reply; 14+ messages in thread
From: Jan Kara @ 2026-06-17 17:09 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Amir Goldstein, Jan Kara, Gao Xiang,
	linux-fsdevel@vger.kernel.org, Miklos Szeredi, Gao Xiang,
	Song Liu

On Wed 17-06-26 15:37:33, Christian Brauner wrote:
> On Thu, Jun 11, 2026 at 05:25:56PM +0200, Amir Goldstein wrote:
> > On Thu, Jun 11, 2026 at 1:32 PM Jan Kara <jack@suse.cz> wrote:
> > >
> > > On Wed 10-06-26 17:40:27, Amir Goldstein wrote:
> > > > On Wed, Jun 10, 2026 at 3:18 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> > > > > On 2026/6/10 20:21, Jan Kara wrote:
> > > > > > On Wed 10-06-26 16:17:33, Gao Xiang wrote:
> > > > > >> Amir just suggested that I posted a long off-list discussion
> > > > > >> on the list right now, but since it seems to relate to
> > > > > >> `sb->s_writers.rw_sem` locking design, I am not quite sure
> > > > > >> how deep I could help generally: But here it goes.
> > > > > >>
> > > > > >> The background is that we have discussed a generic deadlock
> > > > > >> timing for several months as below:
> > > > > >>
> > > > > >> fsA is a filesystem which supports fsfreeze, such as EXT4/XFS/...
> > > > > >>
> > > > > >> fsB is a filesystem which have some relationship with a
> > > > > >> userspace daemon (e.g. a filesystem with fanotify HSM hooks /
> > > > > >>    fanotify + EROFS file-backed mounts / a FUSE filesystem or
> > > > > >>    a filesystem backed by a virtual block device)
> > > > > >>
> > > > > >> Thread A                           Thread B                                 Userspace deamon
> > > > > >>   write(fsA_fd, mmap(fsB_fd))
> > > > > >>    file_start_write()
> > > > > >>     (take SB_FREEZE_WRITE read lock)
> > > > > >>
> > > > > >>     handle fsB mmap fault read
> > > > > >>      -> notify userspace and wait
> > > > > >>
> > > > > >>                                     freeze_super
> > > > > >>                                     (try to take SB_FREEZE_WRITE write lock)
> > > > > >>                                                                              received/handling fsB mmap read request
> > > > > >>                                                                              (do random something...)
> > > > > >>                                                                              write(fsA_fd2)
> > > > > >>                                                                               (take SB_FREEZE_WRITE read lock)
> > > > > >> The problem timing here is thread A does
> > > > > >> `write(fsA_fd, mmap(fsB_fd))` => file_start_write() (rwsem read),
> > > > > >> then hits page fault and wait for userspace deamon to finish
> > > > > >> the request;
> > > > > >>
> > > > > >> Thread B does fsfreeze on fsA so it is waiting a write lock
> > > > > >> (sb_wait_write(), rwsem write) and blocked on thread A;
> > > > > >>
> > > > > >> And the userspace deamon is a handler handling page fault,
> > > > > >> and trying to write to another file (fsA_fd2) on fsA again
> > > > > >> and blocked on thread B (file_start_write(), rwsem read).
> > > > > >>
> > > > > >> because of the specific locking timing is `R->W->R`, at least
> > > > > >> the whole workflow won't proceed so the related processes
> > > > > >> (and fsA above) will be stuck.
> > > > > >
> > > > > > Thanks a lot for the good summary! It has helped me to look at the problem
> > > > > > from a bit different angle and I don't think it's actually specific to
> > > > > > filesystem freezing or fanotify HSM. Consider the following simple setup:
> > > > > >
> > > > > > You have XFS filesystem fsA. On it you have a file imageA and you setup
> > > > > > loop device loop0 over imageA and mount it as some filesystem fsB. Now you
> > > > > > do write(imageA_fd, mmap(fsB_fd)) and you get a nice system deadlock (tried
> > > > > > that and it really works :)). The problem is that write to imageA_fd
> > > > > > acquires exclusively i_rwsem in imageA, then goes on to fault page on fsB
> > > > > > which maps to a read from loop0 which maps to a read from imageA and
> > > > > > xfs_file_read_iter() wants to acquire i_rwsem for imageA again.
> > > > >
> > > > > Ah, great catch, but mainly because XFS takes i_rwsem read lock
> > > > > on read paths LOL.
> > >
> > > Well, yes, that makes things particularly easy to hit but e.g. with direct
> > > IO much more filesystems (in particular everybody using iomap) than just XFS
> > > are prone to similar issues.
> > >
> > > > > Yes, yet this deadlock sounds more like a fixed flow/pattern, which
> > > > > is somewhat slightly different than fsfreeze issue since fanotify
> > > > > needs racy between threads (causing RWR order casually and cause
> > > > > Linux rwsem model deadlock) and may not need stacking filesystems.
> > >
> > > This example demostrates that the problem isn't specific to HSM or filesystem
> > > freezing. Yes, filesystem freezing given its filesystem wide scope is
> > > expanding the deadlock potential to more cases but the underlying problem
> > > really is that if you manage to do a syscall that holds locks on filesystem
> > > A and then ends up accessing filesystem B that's stacked on top of A, you
> > > are violating the expected lock ordering and this can lead to deadlocks.
> > >
> > > > > > Also with the use of filesystem freezing on fsA, write to any target
> > > > > > file (not only imageA) in fsA can create such deadlock if the page fault on
> > > > > > the stacked filesystem ends up wanting to do some modification of imageA.
> > > > > > So I don't think blocking IO to the loop device backing file will be even a
> > > > > > complete fix.
> > > > > >
> > > > > > I think the underlying reason why that write(imageA_fd, mmap(fsB_fd))
> > > > > > pattern causes issues is that it essentially creates a loop in the stacked
> > > > > > filesystem graph which can cause all sorts of lock inversions compared to
> > > > > > the natural order of taking locks for the stacked filesystems. So can we
> > > > > > somehow forbid that? Or do we simply not care and if priviledged user does
> > > > > > something stupid, he gets to reboot the machine? Any other ideas?
> > > >
> > > > Very unbacked idea, but something along the lines of:
> > > >
> > > > /*
> > > >  * sb_{start,end}_write() scope an "sb write transaction"
> > > >  */
> > > >
> > > > @@ -1283,6 +1283,8 @@ struct task_struct {
> > > >         /* Journalling filesystem info: */
> > > >         void                            *journal_info;
> > > >
> > > > +       /* Stacked filesystem write transactions: */
> > > > +       struct super_block
> > > > *sb_write_stack[SB_WRITE_MAX_STACK_DEPTH];
> > > > +       int                                         sb_write_stack_depth;
> > > > +
> > > >         /* Stacked block device info: */
> > > >         struct bio_list                 *bio_list;
> > > >
> > > > I don't know what a decent value of SB_WRITE_MAX_STACK_DEPTH
> > > > would be but its not the same as FILESYSTEM_MAX_STACK_DEPTH,
> > > > because loop/nbd/backing device also contributes to sb_write_stack_depth.
> > >
> > > I was thinking about something similar but then realized this won't fly -
> > > the deadlock chain is often split among multiple processes. Even in the
> > > case I've shown above there's the task doing the nasty write which holds
> > > i_rwsem on imageA and then a workqueue thread that actually does the loopback
> > > device IO which is trying to take it as well. No single process creates
> > > a loop, only all of them together form the loop.
> > 
> > Yap. I was considering whether we could copy the sb_tx_stack to the kiocb..
> > 
> > >
> > > > The rule is that sb_start_write(sb) pushes the sb to the tx stack
> > > > after checking no loops in the stack.
> > > >
> > > > In the common non-stacked case, there are no checks so the push/pop
> > > > should be quite cheap compared to the sb_writers locks.
> > > >
> > > > For the read side I hope that we do not need to track "read transactions"
> > > > in the stack and that it is enough to check the write stack in page faults and
> > > > stacked reads in loop/nbd/backing.
> > >
> > > That's another problem. Reads can create deadlocks as well. If you do
> > > read(imageA_fd, mmap(fsB_fd)) in the setup I've outlined in my previous
> > > email on XFS, you are holding i_rwsem for read for imageA and then doing a
> > > write fault to fsB which translates to a read from imageA which needs
> > > shared i_rwsem for imageA again. If you are lucky, this can pass but if
> > > someone tries to acquire imageA's i_rwsem for write inbetween (and blocks),
> > > your second shared i_rwsem acquisition will block and you have a deadlock.
> > >
> > 
> > Yeh we could introduce sb_read(sb) scopes but that would be a bit nasty.
> > The two-fds syscalls (e.g. sendfile()) makes this scoping even more challenging.
> > 
> > > > Note that the unscoped kiocb_start_write() in aio/uring will need to be
> > > > extended so that the write transaction scope will cover the io submission
> > > > to cover the page faults in input buffers.
> > > >
> > > > I have no idea about the feasibility of this and how much of the holes
> > > > this actually closes but if looking at the narrow use case of pre-content
> > > > events it might just be enough.
> > >
> > > What looks like the most feasible approach to me is to record in struct
> > > task we are doing a read/write syscall for fsA
> > 
> > read/write_iter could be nested within stacked fs, so we really need a stack
> > of transactions, but this one could be [FILESYSTEM_MAX_STACK_DEPTH]
> > 
> > > and in the page fault
> > > handler once we have the VMA we can check that fsB isn't fsA's ancestor in
> > > the "filesystem dependency graph". Making the check cheap enough should be
> > > doable (we could for example do the loop check traversal only if the
> > > stacking depth of fsB is higher than that of fsA, caching the stacking
> > > depth in the superblock, which would deal with 99.99% cases) but maintaining
> > > the dependency graph will take some work...
> > 
> > My crystal ball tells me we are probably not going to do that :-/
> > But if we did, it would have been nice to validate that LOOP_CHANGE_FD
> > does not set the backing file of loopA to a file on fsB ;)
> 
> LOOP_CHANGE_FD should be removed. I proposed that years ago because it's
> problematic in other ways. Everyone agreed as it was effectively unused
> I just never bothered pushing it through. So let's not worry about that
> let's kill it instead. Here's the patch from 2023:
> 
> From 46db77d781d329b7fc7b3245dd295a69026f6ec0 Mon Sep 17 00:00:00 2001
> From: Christian Brauner <brauner@kernel.org>
> Date: Mon, 6 Nov 2023 16:29:32 +0100
> Subject: [PATCH] loop: remove support for swapping loop fds
> 
> We did not find any users of LOOP_CHANGE_FD.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>

I'm for trying this. Just maybe we could print a deprecation warning if
LOOP_CHANGE_FD is used for a while before deleting it completely? To not
outright break possible users if there are some by any chance?

								Honza

> ---
>  drivers/block/loop.c | 100 ++-----------------------------------------
>  1 file changed, 4 insertions(+), 96 deletions(-)
> 
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 9f2d412fc560..ce6cb2125187 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -106,7 +106,7 @@ static DEFINE_MUTEX(loop_validate_mutex);
>   *
>   * Since loop_validate_file() traverses on other "struct loop_device" if
>   * is_loop_device() is true, we need a global lock for serializing concurrent
> - * loop_configure()/loop_change_fd()/__loop_clr_fd() calls.
> + * loop_configure()/__loop_clr_fd() calls.
>   */
>  static int loop_global_lock_killable(struct loop_device *lo, bool global)
>  {
> @@ -554,97 +554,6 @@ static int loop_validate_file(struct file *file, struct block_device *bdev)
>  	return 0;
>  }
>  
> -/*
> - * loop_change_fd switched the backing store of a loopback device to
> - * a new file. This is useful for operating system installers to free up
> - * the original file and in High Availability environments to switch to
> - * an alternative location for the content in case of server meltdown.
> - * This can only work if the loop device is used read-only, and if the
> - * new backing store is the same size and type as the old backing store.
> - */
> -static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
> -			  unsigned int arg)
> -{
> -	struct file *file = fget(arg);
> -	struct file *old_file;
> -	int error;
> -	bool partscan;
> -	bool is_loop;
> -
> -	if (!file)
> -		return -EBADF;
> -
> -	/* suppress uevents while reconfiguring the device */
> -	dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 1);
> -
> -	is_loop = is_loop_device(file);
> -	error = loop_global_lock_killable(lo, is_loop);
> -	if (error)
> -		goto out_putf;
> -	error = -ENXIO;
> -	if (lo->lo_state != Lo_bound)
> -		goto out_err;
> -
> -	/* the loop device has to be read-only */
> -	error = -EINVAL;
> -	if (!(lo->lo_flags & LO_FLAGS_READ_ONLY))
> -		goto out_err;
> -
> -	error = loop_validate_file(file, bdev);
> -	if (error)
> -		goto out_err;
> -
> -	old_file = lo->lo_backing_file;
> -
> -	error = -EINVAL;
> -
> -	/* size of the new backing store needs to be the same */
> -	if (get_loop_size(lo, file) != get_loop_size(lo, old_file))
> -		goto out_err;
> -
> -	/* and ... switch */
> -	disk_force_media_change(lo->lo_disk);
> -	blk_mq_freeze_queue(lo->lo_queue);
> -	mapping_set_gfp_mask(old_file->f_mapping, lo->old_gfp_mask);
> -	lo->lo_backing_file = file;
> -	lo->old_gfp_mask = mapping_gfp_mask(file->f_mapping);
> -	mapping_set_gfp_mask(file->f_mapping,
> -			     lo->old_gfp_mask & ~(__GFP_IO|__GFP_FS));
> -	loop_update_dio(lo);
> -	blk_mq_unfreeze_queue(lo->lo_queue);
> -	partscan = lo->lo_flags & LO_FLAGS_PARTSCAN;
> -	loop_global_unlock(lo, is_loop);
> -
> -	/*
> -	 * Flush loop_validate_file() before fput(), for l->lo_backing_file
> -	 * might be pointing at old_file which might be the last reference.
> -	 */
> -	if (!is_loop) {
> -		mutex_lock(&loop_validate_mutex);
> -		mutex_unlock(&loop_validate_mutex);
> -	}
> -	/*
> -	 * We must drop file reference outside of lo_mutex as dropping
> -	 * the file ref can take open_mutex which creates circular locking
> -	 * dependency.
> -	 */
> -	fput(old_file);
> -	if (partscan)
> -		loop_reread_partitions(lo);
> -
> -	error = 0;
> -done:
> -	/* enable and uncork uevent now that we are done */
> -	dev_set_uevent_suppress(disk_to_dev(lo->lo_disk), 0);
> -	return error;
> -
> -out_err:
> -	loop_global_unlock(lo, is_loop);
> -out_putf:
> -	fput(file);
> -	goto done;
> -}
> -
>  /* loop sysfs attributes */
>  
>  static ssize_t loop_attr_show(struct device *dev, char *page,
> @@ -1222,12 +1131,11 @@ static int loop_clr_fd(struct loop_device *lo)
>  
>  	/*
>  	 * Since lo_ioctl() is called without locks held, it is possible that
> -	 * loop_configure()/loop_change_fd() and loop_clr_fd() run in parallel.
> +	 * loop_configure() and loop_clr_fd() run in parallel.
>  	 *
>  	 * Therefore, use global lock when setting Lo_rundown state in order to
>  	 * make sure that loop_validate_file() will fail if the "struct file"
> -	 * which loop_configure()/loop_change_fd() found via fget() was this
> -	 * loop device.
> +	 * which loop_configure() found via fget() was this loop device.
>  	 */
>  	err = loop_global_lock_killable(lo, true);
>  	if (err)
> @@ -1558,7 +1466,7 @@ static int lo_ioctl(struct block_device *bdev, blk_mode_t mode,
>  		return loop_configure(lo, mode, bdev, &config);
>  	}
>  	case LOOP_CHANGE_FD:
> -		return loop_change_fd(lo, bdev, arg);
> +		return -EOPNOTSUPP;
>  	case LOOP_CLR_FD:
>  		return loop_clr_fd(lo);
>  	case LOOP_SET_STATUS:
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Stacking filesystem deadlocks [was Re: Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup]
  2026-06-11 15:25                                       ` Amir Goldstein
  2026-06-17 13:37                                         ` Christian Brauner
@ 2026-06-17 17:33                                         ` Jan Kara
  2026-06-17 18:04                                           ` Gao Xiang
  2026-06-19  7:27                                           ` Christian Brauner
  1 sibling, 2 replies; 14+ messages in thread
From: Jan Kara @ 2026-06-17 17:33 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Jan Kara, Gao Xiang, linux-fsdevel@vger.kernel.org,
	Christian Brauner, Miklos Szeredi, Gao Xiang, Song Liu

On Thu 11-06-26 17:25:56, Amir Goldstein wrote:
> On Thu, Jun 11, 2026 at 1:32 PM Jan Kara <jack@suse.cz> wrote:
> > > The rule is that sb_start_write(sb) pushes the sb to the tx stack
> > > after checking no loops in the stack.
> > >
> > > In the common non-stacked case, there are no checks so the push/pop
> > > should be quite cheap compared to the sb_writers locks.
> > >
> > > For the read side I hope that we do not need to track "read transactions"
> > > in the stack and that it is enough to check the write stack in page faults and
> > > stacked reads in loop/nbd/backing.
> >
> > That's another problem. Reads can create deadlocks as well. If you do
> > read(imageA_fd, mmap(fsB_fd)) in the setup I've outlined in my previous
> > email on XFS, you are holding i_rwsem for read for imageA and then doing a
> > write fault to fsB which translates to a read from imageA which needs
> > shared i_rwsem for imageA again. If you are lucky, this can pass but if
> > someone tries to acquire imageA's i_rwsem for write inbetween (and blocks),
> > your second shared i_rwsem acquisition will block and you have a deadlock.
> 
> Yeh we could introduce sb_read(sb) scopes but that would be a bit nasty.
> The two-fds syscalls (e.g. sendfile()) makes this scoping even more challenging.

Well, for stuff like sendfile() or splice we take care not to entangle the
read and write parts:
  while there's data:
    read into pipe
    write from pipe

So these shouldn't be a problem.

> > > Note that the unscoped kiocb_start_write() in aio/uring will need to be
> > > extended so that the write transaction scope will cover the io submission
> > > to cover the page faults in input buffers.
> > >
> > > I have no idea about the feasibility of this and how much of the holes
> > > this actually closes but if looking at the narrow use case of pre-content
> > > events it might just be enough.
> >
> > What looks like the most feasible approach to me is to record in struct
> > task we are doing a read/write syscall for fsA
> 
> read/write_iter could be nested within stacked fs, so we really need a stack
> of transactions, but this one could be [FILESYSTEM_MAX_STACK_DEPTH]
> 
> > and in the page fault
> > handler once we have the VMA we can check that fsB isn't fsA's ancestor in
> > the "filesystem dependency graph". Making the check cheap enough should be
> > doable (we could for example do the loop check traversal only if the
> > stacking depth of fsB is higher than that of fsA, caching the stacking
> > depth in the superblock, which would deal with 99.99% cases) but maintaining
> > the dependency graph will take some work...
> 
> My crystal ball tells me we are probably not going to do that :-/

Yeah, it won't be *that* complex and for stuff like reliable system
hibernation we would need that as well but so far it looks like too much
effort for too little gain...

> But if we did, it would have been nice to validate that LOOP_CHANGE_FD
> does not set the backing file of loopA to a file on fsB ;)
> 
> If we only wanted to protect the case of pre-content events in erofs
> backing file (readonly), is there any solution that you see which is less
> complicated?

Two ideas:
1) erofs on mmap could issue events to download the all mapped content into
the underlying image.

2) Just ignore the problem since I don't think it's really a practical
one. I mean userspace would have to issue a write to fs carrying
erofs image with buffer mmaped from the mounted erofs. If you have sane
access policies, apps on erofs shouldn't have write access to the
underlying filesystem carrying the image. And unpriviledged apps cannot
trigger freezing of underlying filesystem.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Stacking filesystem deadlocks [was Re: Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup]
  2026-06-17 17:33                                         ` Jan Kara
@ 2026-06-17 18:04                                           ` Gao Xiang
  2026-06-19  7:27                                           ` Christian Brauner
  1 sibling, 0 replies; 14+ messages in thread
From: Gao Xiang @ 2026-06-17 18:04 UTC (permalink / raw)
  To: Jan Kara, Amir Goldstein
  Cc: linux-fsdevel@vger.kernel.org, Christian Brauner, Miklos Szeredi,
	Gao Xiang, Song Liu

Hi Jan,

On 2026/6/18 01:33, Jan Kara wrote:
> On Thu 11-06-26 17:25:56, Amir Goldstein wrote:
>> On Thu, Jun 11, 2026 at 1:32 PM Jan Kara <jack@suse.cz> wrote:

...

> 
>> But if we did, it would have been nice to validate that LOOP_CHANGE_FD
>> does not set the backing file of loopA to a file on fsB ;)
>>
>> If we only wanted to protect the case of pre-content events in erofs
>> backing file (readonly), is there any solution that you see which is less
>> complicated?
> 
> Two ideas:
> 1) erofs on mmap could issue events to download the all mapped content into
> the underlying image.

Personally I don't think it's feasible in practial since
the deadlock vector is pretty common among all approaches
(userspace/loop) but the impact (drawback) of this way is
too negative for all end users (considering executable
files in the container images) even it's just an overkill
for our real workloads.

> 
> 2) Just ignore the problem since I don't think it's really a practical
> one. I mean userspace would have to issue a write to fs carrying
> erofs image with buffer mmaped from the mounted erofs. If you have sane
> access policies, apps on erofs shouldn't have write access to the
> underlying filesystem carrying the image. And unpriviledged apps cannot
> trigger freezing of underlying filesystem.

Yes, I think this thread is mainly used to come up
more clean solutions, but honestly erofs typical
applications never face the issues mentioned in
practice: just because both erofs and fanotify need
privilege, which means the system solution will be
formed for sysadmins to avoid these common deadlock
vectors.

For example, image files will be isolated by mount
namespaces so container workloads won't write image
files with mmap read like what mentioned before. Also
fsfreeze can also be resolved by 1) image files stored
in another local filesystem (separated from the
container rootfs) or 2) just don't think fsfreeze
happens in typical cloud workloads (and as you said
fsfreeze needs priviledge too) and 3) sysadmin can
kill the application to recover from deadlock and 4)
other userspace approaches (fuse, nbd) also have this
issue.

I think just because erofs+fanotify needs privilege,
and these vectors are common (so again I don't think
we need a harsh approach in advance considering
there are other easier unpriviledged way to lock down
the system e.g. by using fuse): we could just mention/
document these somewhere for admins explicitly.

Thanks,
Gao Xiang

> 
> 								Honza


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Stacking filesystem deadlocks [was Re: Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup]
  2026-06-17 17:09                                           ` Jan Kara
@ 2026-06-19  7:25                                             ` Christian Brauner
  0 siblings, 0 replies; 14+ messages in thread
From: Christian Brauner @ 2026-06-19  7:25 UTC (permalink / raw)
  To: Jan Kara
  Cc: Amir Goldstein, Gao Xiang, linux-fsdevel@vger.kernel.org,
	Miklos Szeredi, Gao Xiang, Song Liu

On Wed, Jun 17, 2026 at 07:09:43PM +0200, Jan Kara wrote:
> On Wed 17-06-26 15:37:33, Christian Brauner wrote:
> > On Thu, Jun 11, 2026 at 05:25:56PM +0200, Amir Goldstein wrote:
> > > On Thu, Jun 11, 2026 at 1:32 PM Jan Kara <jack@suse.cz> wrote:
> > > >
> > > > On Wed 10-06-26 17:40:27, Amir Goldstein wrote:
> > > > > On Wed, Jun 10, 2026 at 3:18 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
> > > > > > On 2026/6/10 20:21, Jan Kara wrote:
> > > > > > > On Wed 10-06-26 16:17:33, Gao Xiang wrote:
> > > > > > >> Amir just suggested that I posted a long off-list discussion
> > > > > > >> on the list right now, but since it seems to relate to
> > > > > > >> `sb->s_writers.rw_sem` locking design, I am not quite sure
> > > > > > >> how deep I could help generally: But here it goes.
> > > > > > >>
> > > > > > >> The background is that we have discussed a generic deadlock
> > > > > > >> timing for several months as below:
> > > > > > >>
> > > > > > >> fsA is a filesystem which supports fsfreeze, such as EXT4/XFS/...
> > > > > > >>
> > > > > > >> fsB is a filesystem which have some relationship with a
> > > > > > >> userspace daemon (e.g. a filesystem with fanotify HSM hooks /
> > > > > > >>    fanotify + EROFS file-backed mounts / a FUSE filesystem or
> > > > > > >>    a filesystem backed by a virtual block device)
> > > > > > >>
> > > > > > >> Thread A                           Thread B                                 Userspace deamon
> > > > > > >>   write(fsA_fd, mmap(fsB_fd))
> > > > > > >>    file_start_write()
> > > > > > >>     (take SB_FREEZE_WRITE read lock)
> > > > > > >>
> > > > > > >>     handle fsB mmap fault read
> > > > > > >>      -> notify userspace and wait
> > > > > > >>
> > > > > > >>                                     freeze_super
> > > > > > >>                                     (try to take SB_FREEZE_WRITE write lock)
> > > > > > >>                                                                              received/handling fsB mmap read request
> > > > > > >>                                                                              (do random something...)
> > > > > > >>                                                                              write(fsA_fd2)
> > > > > > >>                                                                               (take SB_FREEZE_WRITE read lock)
> > > > > > >> The problem timing here is thread A does
> > > > > > >> `write(fsA_fd, mmap(fsB_fd))` => file_start_write() (rwsem read),
> > > > > > >> then hits page fault and wait for userspace deamon to finish
> > > > > > >> the request;
> > > > > > >>
> > > > > > >> Thread B does fsfreeze on fsA so it is waiting a write lock
> > > > > > >> (sb_wait_write(), rwsem write) and blocked on thread A;
> > > > > > >>
> > > > > > >> And the userspace deamon is a handler handling page fault,
> > > > > > >> and trying to write to another file (fsA_fd2) on fsA again
> > > > > > >> and blocked on thread B (file_start_write(), rwsem read).
> > > > > > >>
> > > > > > >> because of the specific locking timing is `R->W->R`, at least
> > > > > > >> the whole workflow won't proceed so the related processes
> > > > > > >> (and fsA above) will be stuck.
> > > > > > >
> > > > > > > Thanks a lot for the good summary! It has helped me to look at the problem
> > > > > > > from a bit different angle and I don't think it's actually specific to
> > > > > > > filesystem freezing or fanotify HSM. Consider the following simple setup:
> > > > > > >
> > > > > > > You have XFS filesystem fsA. On it you have a file imageA and you setup
> > > > > > > loop device loop0 over imageA and mount it as some filesystem fsB. Now you
> > > > > > > do write(imageA_fd, mmap(fsB_fd)) and you get a nice system deadlock (tried
> > > > > > > that and it really works :)). The problem is that write to imageA_fd
> > > > > > > acquires exclusively i_rwsem in imageA, then goes on to fault page on fsB
> > > > > > > which maps to a read from loop0 which maps to a read from imageA and
> > > > > > > xfs_file_read_iter() wants to acquire i_rwsem for imageA again.
> > > > > >
> > > > > > Ah, great catch, but mainly because XFS takes i_rwsem read lock
> > > > > > on read paths LOL.
> > > >
> > > > Well, yes, that makes things particularly easy to hit but e.g. with direct
> > > > IO much more filesystems (in particular everybody using iomap) than just XFS
> > > > are prone to similar issues.
> > > >
> > > > > > Yes, yet this deadlock sounds more like a fixed flow/pattern, which
> > > > > > is somewhat slightly different than fsfreeze issue since fanotify
> > > > > > needs racy between threads (causing RWR order casually and cause
> > > > > > Linux rwsem model deadlock) and may not need stacking filesystems.
> > > >
> > > > This example demostrates that the problem isn't specific to HSM or filesystem
> > > > freezing. Yes, filesystem freezing given its filesystem wide scope is
> > > > expanding the deadlock potential to more cases but the underlying problem
> > > > really is that if you manage to do a syscall that holds locks on filesystem
> > > > A and then ends up accessing filesystem B that's stacked on top of A, you
> > > > are violating the expected lock ordering and this can lead to deadlocks.
> > > >
> > > > > > > Also with the use of filesystem freezing on fsA, write to any target
> > > > > > > file (not only imageA) in fsA can create such deadlock if the page fault on
> > > > > > > the stacked filesystem ends up wanting to do some modification of imageA.
> > > > > > > So I don't think blocking IO to the loop device backing file will be even a
> > > > > > > complete fix.
> > > > > > >
> > > > > > > I think the underlying reason why that write(imageA_fd, mmap(fsB_fd))
> > > > > > > pattern causes issues is that it essentially creates a loop in the stacked
> > > > > > > filesystem graph which can cause all sorts of lock inversions compared to
> > > > > > > the natural order of taking locks for the stacked filesystems. So can we
> > > > > > > somehow forbid that? Or do we simply not care and if priviledged user does
> > > > > > > something stupid, he gets to reboot the machine? Any other ideas?
> > > > >
> > > > > Very unbacked idea, but something along the lines of:
> > > > >
> > > > > /*
> > > > >  * sb_{start,end}_write() scope an "sb write transaction"
> > > > >  */
> > > > >
> > > > > @@ -1283,6 +1283,8 @@ struct task_struct {
> > > > >         /* Journalling filesystem info: */
> > > > >         void                            *journal_info;
> > > > >
> > > > > +       /* Stacked filesystem write transactions: */
> > > > > +       struct super_block
> > > > > *sb_write_stack[SB_WRITE_MAX_STACK_DEPTH];
> > > > > +       int                                         sb_write_stack_depth;
> > > > > +
> > > > >         /* Stacked block device info: */
> > > > >         struct bio_list                 *bio_list;
> > > > >
> > > > > I don't know what a decent value of SB_WRITE_MAX_STACK_DEPTH
> > > > > would be but its not the same as FILESYSTEM_MAX_STACK_DEPTH,
> > > > > because loop/nbd/backing device also contributes to sb_write_stack_depth.
> > > >
> > > > I was thinking about something similar but then realized this won't fly -
> > > > the deadlock chain is often split among multiple processes. Even in the
> > > > case I've shown above there's the task doing the nasty write which holds
> > > > i_rwsem on imageA and then a workqueue thread that actually does the loopback
> > > > device IO which is trying to take it as well. No single process creates
> > > > a loop, only all of them together form the loop.
> > > 
> > > Yap. I was considering whether we could copy the sb_tx_stack to the kiocb..
> > > 
> > > >
> > > > > The rule is that sb_start_write(sb) pushes the sb to the tx stack
> > > > > after checking no loops in the stack.
> > > > >
> > > > > In the common non-stacked case, there are no checks so the push/pop
> > > > > should be quite cheap compared to the sb_writers locks.
> > > > >
> > > > > For the read side I hope that we do not need to track "read transactions"
> > > > > in the stack and that it is enough to check the write stack in page faults and
> > > > > stacked reads in loop/nbd/backing.
> > > >
> > > > That's another problem. Reads can create deadlocks as well. If you do
> > > > read(imageA_fd, mmap(fsB_fd)) in the setup I've outlined in my previous
> > > > email on XFS, you are holding i_rwsem for read for imageA and then doing a
> > > > write fault to fsB which translates to a read from imageA which needs
> > > > shared i_rwsem for imageA again. If you are lucky, this can pass but if
> > > > someone tries to acquire imageA's i_rwsem for write inbetween (and blocks),
> > > > your second shared i_rwsem acquisition will block and you have a deadlock.
> > > >
> > > 
> > > Yeh we could introduce sb_read(sb) scopes but that would be a bit nasty.
> > > The two-fds syscalls (e.g. sendfile()) makes this scoping even more challenging.
> > > 
> > > > > Note that the unscoped kiocb_start_write() in aio/uring will need to be
> > > > > extended so that the write transaction scope will cover the io submission
> > > > > to cover the page faults in input buffers.
> > > > >
> > > > > I have no idea about the feasibility of this and how much of the holes
> > > > > this actually closes but if looking at the narrow use case of pre-content
> > > > > events it might just be enough.
> > > >
> > > > What looks like the most feasible approach to me is to record in struct
> > > > task we are doing a read/write syscall for fsA
> > > 
> > > read/write_iter could be nested within stacked fs, so we really need a stack
> > > of transactions, but this one could be [FILESYSTEM_MAX_STACK_DEPTH]
> > > 
> > > > and in the page fault
> > > > handler once we have the VMA we can check that fsB isn't fsA's ancestor in
> > > > the "filesystem dependency graph". Making the check cheap enough should be
> > > > doable (we could for example do the loop check traversal only if the
> > > > stacking depth of fsB is higher than that of fsA, caching the stacking
> > > > depth in the superblock, which would deal with 99.99% cases) but maintaining
> > > > the dependency graph will take some work...
> > > 
> > > My crystal ball tells me we are probably not going to do that :-/
> > > But if we did, it would have been nice to validate that LOOP_CHANGE_FD
> > > does not set the backing file of loopA to a file on fsB ;)
> > 
> > LOOP_CHANGE_FD should be removed. I proposed that years ago because it's
> > problematic in other ways. Everyone agreed as it was effectively unused
> > I just never bothered pushing it through. So let's not worry about that
> > let's kill it instead. Here's the patch from 2023:
> > 
> > From 46db77d781d329b7fc7b3245dd295a69026f6ec0 Mon Sep 17 00:00:00 2001
> > From: Christian Brauner <brauner@kernel.org>
> > Date: Mon, 6 Nov 2023 16:29:32 +0100
> > Subject: [PATCH] loop: remove support for swapping loop fds
> > 
> > We did not find any users of LOOP_CHANGE_FD.
> > 
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> 
> I'm for trying this. Just maybe we could print a deprecation warning if
> LOOP_CHANGE_FD is used for a while before deleting it completely? To not
> outright break possible users if there are some by any chance?

Sure, works too. I think the use-cases for this should mostly be gone by
now though. If someone picks up my patch I'm happy to let it go.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: Stacking filesystem deadlocks [was Re: Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup]
  2026-06-17 17:33                                         ` Jan Kara
  2026-06-17 18:04                                           ` Gao Xiang
@ 2026-06-19  7:27                                           ` Christian Brauner
  1 sibling, 0 replies; 14+ messages in thread
From: Christian Brauner @ 2026-06-19  7:27 UTC (permalink / raw)
  To: Jan Kara
  Cc: Amir Goldstein, Gao Xiang, linux-fsdevel@vger.kernel.org,
	Miklos Szeredi, Gao Xiang, Song Liu

On Wed, Jun 17, 2026 at 07:33:21PM +0200, Jan Kara wrote:
> On Thu 11-06-26 17:25:56, Amir Goldstein wrote:
> > On Thu, Jun 11, 2026 at 1:32 PM Jan Kara <jack@suse.cz> wrote:
> > > > The rule is that sb_start_write(sb) pushes the sb to the tx stack
> > > > after checking no loops in the stack.
> > > >
> > > > In the common non-stacked case, there are no checks so the push/pop
> > > > should be quite cheap compared to the sb_writers locks.
> > > >
> > > > For the read side I hope that we do not need to track "read transactions"
> > > > in the stack and that it is enough to check the write stack in page faults and
> > > > stacked reads in loop/nbd/backing.
> > >
> > > That's another problem. Reads can create deadlocks as well. If you do
> > > read(imageA_fd, mmap(fsB_fd)) in the setup I've outlined in my previous
> > > email on XFS, you are holding i_rwsem for read for imageA and then doing a
> > > write fault to fsB which translates to a read from imageA which needs
> > > shared i_rwsem for imageA again. If you are lucky, this can pass but if
> > > someone tries to acquire imageA's i_rwsem for write inbetween (and blocks),
> > > your second shared i_rwsem acquisition will block and you have a deadlock.
> > 
> > Yeh we could introduce sb_read(sb) scopes but that would be a bit nasty.
> > The two-fds syscalls (e.g. sendfile()) makes this scoping even more challenging.
> 
> Well, for stuff like sendfile() or splice we take care not to entangle the
> read and write parts:
>   while there's data:
>     read into pipe
>     write from pipe
> 
> So these shouldn't be a problem.
> 
> > > > Note that the unscoped kiocb_start_write() in aio/uring will need to be
> > > > extended so that the write transaction scope will cover the io submission
> > > > to cover the page faults in input buffers.
> > > >
> > > > I have no idea about the feasibility of this and how much of the holes
> > > > this actually closes but if looking at the narrow use case of pre-content
> > > > events it might just be enough.
> > >
> > > What looks like the most feasible approach to me is to record in struct
> > > task we are doing a read/write syscall for fsA
> > 
> > read/write_iter could be nested within stacked fs, so we really need a stack
> > of transactions, but this one could be [FILESYSTEM_MAX_STACK_DEPTH]
> > 
> > > and in the page fault
> > > handler once we have the VMA we can check that fsB isn't fsA's ancestor in
> > > the "filesystem dependency graph". Making the check cheap enough should be
> > > doable (we could for example do the loop check traversal only if the
> > > stacking depth of fsB is higher than that of fsA, caching the stacking
> > > depth in the superblock, which would deal with 99.99% cases) but maintaining
> > > the dependency graph will take some work...
> > 
> > My crystal ball tells me we are probably not going to do that :-/
> 
> Yeah, it won't be *that* complex and for stuff like reliable system
> hibernation we would need that as well but so far it looks like too much
> effort for too little gain...

I think we should see the code first, before dismissing it. A reliably
system is almost always better imho and maybe it isn't even that
difficult to do.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-06-19  7:27 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <271c1c26-f170-462b-8a7f-686530fdd449@linux.alibaba.com>
     [not found] ` <53d0e50a-faa9-49e4-a8f5-d4d37bcba0c6@linux.alibaba.com>
     [not found]   ` <CAOQ4uxh-kmoO03yCPaFnWcMjeOgqUi_19cGg4qdeY86ccz9ZyA@mail.gmail.com>
     [not found]     ` <f0adaed4-1412-4722-8bdd-53f5e3823209@linux.alibaba.com>
     [not found]       ` <CAOQ4uxj7gP=qpUvVbMbxbf6LdrYLeB4dX=vkUfOpi1ZfU0542Q@mail.gmail.com>
     [not found]         ` <CAJfpeguXSh3-_3cbpzS-s4+Bq+7MVwazD+3P-diG=Qq70E2qrQ@mail.gmail.com>
     [not found]           ` <d5f62836-62d1-4b8e-9694-bbab27112e7e@linux.alibaba.com>
     [not found]             ` <649fdbbb-64f7-43d9-afd5-a3076e3ec946@linux.alibaba.com>
     [not found]               ` <CAOQ4uxj+WAe+99FFi9RfMd5tpSwwP=rSH4dNuJEdhV1wL+5CDA@mail.gmail.com>
     [not found]                 ` <dff837f6-7480-4984-8728-802ddd78dda4@linux.alibaba.com>
     [not found]                   ` <CAOQ4uxi7DceKuh6+qGjuQJEFiE=HEFu4dMrc++VXUknRnX26JQ@mail.gmail.com>
     [not found]                     ` <95371379-97e9-4cb4-8358-ec014b765b74@linux.alibaba.com>
     [not found]                       ` <CAOQ4uxjh5VYkKLAKxhMsy-ZmrbRrgX4pFgBLFqsDqWMXeiqg3w@mail.gmail.com>
     [not found]                         ` <ac05da56-eb2a-4862-b887-8081f249d8ce@linux.alibaba.com>
     [not found]                           ` <CAOQ4uxi0yRavUeztRh-Eb8_HvpfmqkpPbWT9JgQhUn7EKXnryw@mail.gmail.com>
2026-06-10  8:17                             ` Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup Gao Xiang
2026-06-10 12:21                               ` Stacking filesystem deadlocks [was Re: Fanotify (hsm or erofs) / fuse / nbd / ... + write(mmap) deadlock vector followup] Jan Kara
2026-06-10 13:18                                 ` Gao Xiang
2026-06-10 15:40                                   ` Amir Goldstein
2026-06-10 15:58                                     ` Gao Xiang
2026-06-11 11:32                                     ` Jan Kara
2026-06-11 15:25                                       ` Amir Goldstein
2026-06-17 13:37                                         ` Christian Brauner
2026-06-17 14:23                                           ` Gao Xiang
2026-06-17 17:09                                           ` Jan Kara
2026-06-19  7:25                                             ` Christian Brauner
2026-06-17 17:33                                         ` Jan Kara
2026-06-17 18:04                                           ` Gao Xiang
2026-06-19  7:27                                           ` Christian Brauner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox