Re: [BUG] ntfs: task hung in find_inode due to self-deadlock in ntfs_attrlist_update() during unmount

[Hyunchul Lee]

2026년 7월 2일 (목) 오후 2:01, Peiyang He <peiyang_he@xxxxxxxxxxxxxxxx>님이 작성:
>
>
>
> On 2026/7/2 08:58, Hyunchul Lee wrote:
> > Hi Peiyang,
> >
> >> ==========
> >> Root cause
> >> ==========
> >>
> >> A = the NTFS base inode whose delayed allocation / mapping pairs are being
> >>     written back.
> >>
> >> B = the fake NTFS attribute inode for A's unnamed $ATTRIBUTE_LIST attribute,
> >>     i.e. the inode returned by ntfs_attr_iget(VFS_I(A), AT_ATTRIBUTE_LIST, AT_UNNAMED, 0)
> >>
> >> Timeflow (all code should be executed within one task):
> >>
> >> 1. The task starts unmounting the NTFS filesystem that contains A:
> >>
> >>         cleanup_mnt()                    fs/namespace.c:1292
> >>         --> deactivate_super()           fs/super.c:505
> >>         --> deactivate_locked_super()    fs/super.c:471
> >>             --> fs->kill_sb(s)           fs/super.c:476
> >>                 /* ntfs_fs_type.kill_sb is kill_block_super. */
> >>                                            fs/ntfs/super.c:2645
> >>         --> kill_block_super()           fs/super.c:1721
> >>         --> generic_shutdown_super()     fs/super.c:618
> >>
> >>    generic_shutdown_super() clears SB_ACTIVE and then evicts cached inodes:
> >>
> >>         sb->s_flags &= ~SB_ACTIVE        fs/super.c:626
> >>         evict_inodes(sb)                 fs/super.c:632
> >>
> >> 2. evict_inodes(sb) selects B, marks it I_FREEING, and the same task starts
> >>    evicting B:
> >>
> >>         evict_inodes(sb)                 fs/inode.c:897
> >>         --> inode_state_set(B, I_FREEING)
> >>                                            fs/inode.c:918
> >>         --> dispose_list(&dispose)       fs/inode.c:875
> >>         --> evict(B)                     fs/inode.c:883
> >>         --> ntfs_evict_big_inode(B)      fs/ntfs/inode.c:2261
> >>
> >>    B is a fake attribute inode.  During its eviction, NTFS drops B's reference
> >>    on its base inode A:
> >>
> >>         iput(VFS_I(B->ext.base_ntfs_ino))
> >>                                            fs/ntfs/inode.c:2307
> >>
> >>    At this point B is still in the inode hash.  VFS removes B from the hash
> >>    and wakes __wait_on_freeing_inode() waiters only after
> >>    ntfs_evict_big_inode(B) returns:
> >>
> >>         remove_inode_hash(B)             fs/inode.c:849
> >>         inode_wake_up_bit(B, __I_NEW)    fs/inode.c:862
> >>
> >> 3. The iput(A) from ntfs_evict_big_inode(B) starts synchronous writeback of A:
> >>
> >>         iput(A)                         fs/inode.c:1972
> >>         --> iput_final(A)               fs/inode.c:1916
> >>         --> write_inode_now(A, 1)       fs/fs-writeback.c:2961
> >>         --> ntfs_writepages(A->i_mapping)
> >>                                           fs/ntfs/aops.c:244
> >>         --> ntfs_writeback_range(... A ...)
> >>                                           fs/ntfs/iomap.c:861
> >>
> >>    SB_ACTIVE is already clear, so iput_final(A) cannot keep A on the inode LRU.
> >>    It has to write and evict A.
> >>
> >> 4. A's writeback updates mapping pairs and then updates A's attribute list:
> >>
> >>         __ntfs_write_iomap_begin(A, ...)
> >>                                           fs/ntfs/iomap.c:868
> >>         --> ntfs_attr_map_cluster(A, ...)
> >>                                           fs/ntfs/attrib.c:4948
> >>         --> ntfs_attr_update_mapping_pairs(A, 0)
> >>                                           fs/ntfs/attrib.c:3535
> >>         --> ntfs_attrlist_update(A)      fs/ntfs/attrib.c:3712
> >>         --> ntfs_attr_iget(VFS_I(A),
> >>                            AT_ATTRIBUTE_LIST, AT_UNNAMED, 0)
> >>                                           fs/ntfs/attrlist.c:60
> >>                                           fs/ntfs/inode.c:209
> >>
> >>    This attempts to get B *again* while B is still being evicted.
> >>
> >> 5. ntfs_attr_iget() reaches find_inode().  find_inode() finds B, sees that B
> >>    already has I_FREEING set, and sleeps waiting for B's eviction to finish:
> >>
> >>         iget5_locked()                  fs/ntfs/inode.c:224
> >>                                          fs/inode.c:1375
> >>         --> find_inode()                fs/inode.c:1040
> >>         --> __wait_on_freeing_inode(B)  fs/inode.c:1061
> >>                                          fs/inode.c:2537
> >>         --> schedule()                  fs/inode.c:2560
> >>
> >>    This is a self-deadlock.  The task waits for B's eviction to complete, but
> >>    the only code that can complete B's eviction is the later part of evict(B),
> >>    after ntfs_evict_big_inode(B) returns.  The same task cannot return there
> >>    because it is now sleeping in the nested find_inode() path.
> >
> > Thank you for the detailed root cause analysis.
> >
> > Your analysis looks correct. However we would like to discuss whether
> > there is another way to handle this situation.
> >
> > The call stack you provided seems unlikely to occur under normal conditions,
> > because generic_shutdown_super() unconditionally runs sync_filesystems()
> > before calling evict_inodes().
> >
> > Does this issue occur when sync_filesystem() fails, for example due to
> > I/O errors or disk removal?
>
> Hi Hyunchul, thanks for your reply.
>
> I have looked into the kernel log, and YES, this issue occurred because sync_filesystem() hit an unusual path.
> (BTW sorry for not including the kernel log in the attachments at first)
>
> From the kernel log (line 1992~1993), ntfs_attr_add() somehow failed and executed
> ntfs_error(sb, "Failed to add resident attribute"); (fs/ntfs/attrib.c:2757).
>
> ntfs_handle_error() was then called, and when ON_ERRORS_REMOUNT_RO was set, it set the file system to *read-only* (fs/ntfs/super.c:310),
> which matches line 1994~1995 of the kernel log.
>
> (from line 307 and 356 of the kernel log, we can indeed see that `@errors_remount` was enabled in two Syzkaller testcases and thus ON_ERRORS_REMOUNT_RO was set:
> https://github.com/google/syzkaller/blob/27192279868f7027c85197e80335f160406d6280/sys/linux/filesystem.txt#L612.
> But since multiple testcases were running concurrently, we cannot confirm which testcase triggers the task hung)
>
> When the file system is read-only, sync_filesystem() would return *directly* without writing back (fs/sync.c:43) so inodes are still dirty.
> All functions in the call stack of this task hung don't check if the file system is read-only.
>
> >
> > If so, would it make sense to mark volume as shutdown (NVolShutdown) or
> Yes, I think marking volume as NVolShutdown would be better. ntfs_writepages() already checks this.> switch the filesystem to read-only when a writeback error is encountered?
> >

> Do you mean we check the return value of sync_filesystem() in generic_shutdown_super()?
> For now the return value of sync_filesystem() is indeed ignored in this function.
> But only checking 'error' may not be enough, since sync_filesystem() returns 0 directly when the file system is read-only.
>

I initially considered setting NV_Shutdown inside ntfs_handle_error().
However, the flag also blocks open() and readdir() and does not align
with the "errors=continue" policy.

Because of these side effects, we will need to redesign the error
handling policy in the long term.

For now, to resolve this deadlock, I suggest we proceed with your
proposed patch with returning -EIO.

> Best,
> Peiyang



-- 
Thanks,
Hyunchul