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

David Sterba
2024-02-07T09:52:38Z

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.