Re: [PATCH] fs: Protect reconfiguration of sb read-write from racing writes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu 15-06-23 10:10:40, Theodore Ts'o wrote:
> On Thu, Jun 15, 2023 at 02:53:53PM +0200, Christian Brauner wrote:
> > 
> > So looking at the ext4 code this can only happen when you clear
> > SB_RDONLY in ->reconfigure() too early (and the mount isn't
> > MNT_READONLY). Afaict, this was fixed in:
> > 
> > a44be64bbecb ("ext4: don't clear SB_RDONLY when remounting r/w until quota is re-enabled")
> > 
> > by clearing SB_RDONLY late, right before returning from ->reconfigure()
> > when everything's ready. So your change is not about fixing that bug in
> > [1] it's about making the vfs give the guarantee that an fs is free to
> > clear SB_RDONLY because any ro<->rw transitions are protected via
> > s_readonly_remount. Correct? It seems ok to me just making sure.
> 
> Unfortunately we had to revert that commit because that broke
> r/o->r/w writes when quota was enabled.  The problem is we need a way
> of enabling file system writes for internal purposes (e.g., because
> quota needs to set up quota inodes) but *not* allow userspace file
> system writes to occur until we are fully done with the remount process.
> 
> See the discussion here:
> 
> 	https://lore.kernel.org/all/20230608044056.GA1418535@xxxxxxx/
> 
> The problem with the current state of the tree is commit dea9d8f7643f
> ("ext4: only check dquot_initialize_needed() when debugging") has
> caught real bugs in the past where the caller of
> ext4_xattr_block_set() failed to call dquot_initialize(inode).  In
> addition, shutting up the warning doesn't fix the problem that while
> we hit this race where we have started remounting r/w, quota hasn't
> been initialized, quota tracking will get silently dropped, leading to
> the quota usage tracking no longer reflecting reality.
> 
> Jan's patch will fix this problem.

Yes, and to fully reveal the situation, we can indeed introduce
ext4-private superblock state "only internal writes allowed" which can be
used for quota setup. It just requires a bit of tidying and helper
adjustment (in fact I have such patches on my private branch). But it has
occured to me that generally it is easier if the filesystem's remount code
doesn't have to care about racing writers on rw<->ro transitions since we
have many filesystems and making sure all are getting this correct is ...
tedious.

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux