Re: [PATCH 0/3] btrfs-progs: hunt down the stray fd close causing race with udev scan

[David Sterba <dsterba@suse.cz>]

On Fri, Feb 02, 2024 at 11:29:18AM +1030, Qu Wenruo wrote:
> Although my previous flock() based solution is preventing udev scan to
> get pre-mature super blocks, it in fact masks the root problem:
> 
>   There is a stray close() on writeable fd.
> 
> Commit b2a1be83b85f ("btrfs-progs: mkfs: keep file descriptors open
> during whole time") tries to solve the problem by extending the lifespan
> of writeable fds, so that when the fds are closed, the fs is ensured to
> be properly populated.
> 
> The problem is, that patch is not covering all cases, there is a stray
> fd close just under our noses: open_ctree_fs_info().
> 
> The function would open the initial device, then use that initial fd to
> open the btrfs, then we immediately close the initial fd, as later IO
> would all go with the device fd.
> 
> That close() call is causing problem, especially for mkfs, as at that
> stage the fs is still using a temporary super block, not using the valid
> btrfs super magic number.
> 
> Thus udev scan would race with mkfs, and if udev wins the race, it would
> get the temporary super block, making libblk not to detect the new
> btrfs.
> 
> This patchset would address the problem by:
> 
> - Make sure open_ctree*() calls all have corresponding close_ctree()
>   The first patch, as later we will only close the initial fd caused by
>   open_ctree_fs_info() during close_ctree().
> 
> - Save the initial fd into btrfs_fs_info for open_ctree_fs_info()
>   And later close the initial fd during close_ctree().
> 
> - Make sure open_ctree_fd() callers to properly close the fd
>   Just an extra cleanup.
> 
> This patchset would work even without the usage of flock() to block udev
> scan, and since without flock() calls, the procedure is much simpler.
> 
> Qu Wenruo (3):
>   btrfs-progs: rescue: properly close the fs for clear-ino-cache
>   btrfs-progs: tune: fix the missing close()
>   btrfs-progs: fix the stray fd close in open_ctree_fs_info()

Thanks, added to devel.