From: Ye Bin A writable open of a regular file reaches the filesystem's ->open() callback without holding any superblock write (freeze) protection. ->open() can nevertheless write to the filesystem: ext4_file_open() -> dquot_file_open() -> dquot_initialize() may acquire a dquot and write the quota file, which starts a jbd2 transaction. If the filesystem reaches SB_FREEZE_COMPLETE in that window, ext4_journal_check_start() hits WARN_ON(sb->s_writers.frozen == SB_FREEZE_COMPLETE); and the open then sleeps in jbd2 until the filesystem is thawed. The hole was created over time: - Commit 907f4554e252 ("dquot: move dquot initialization responsibility into the filesystem") moved quota initialization into ->open() and introduced dquot_file_open(), making ->open() a potentially writing operation for writable opens. - Commit eb04c28288bb ("fs: Add freezing handling to mnt_want_write() / mnt_drop_write()") added sb_start_write() to mnt_want_write() but deliberately kept the write reference held by an open file on the non-freezing __mnt_want_write() variant, so the file's own write reference does not protect its ->open(). - Commit 64894cf84327 ("simplify lookup_open()/atomic_open() - do the temporary mnt_want_write() early") moved the temporary write count into do_last() and dropped it before finish_open(), so a writable open that is not O_TRUNC reaches ->open() with no sb write count at all. Since then only O_TRUNC opens take mnt_want_write() in do_open(). Fix this by taking mnt_want_write() for any writable open of a regular file and holding it across vfs_open()/->open(). Regular files are checked explicitly so that opening device nodes on a read-only mount is not newly rejected. Read-only opens are unaffected. Fixes: eb04c28288bb ("fs: Add freezing handling to mnt_want_write() / mnt_drop_write()") Signed-off-by: Ye Bin --- fs/namei.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index f573fd0aacaf..0ee4cf282cf5 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -4906,6 +4906,7 @@ static int do_open(struct nameidata *nd, const struct mnt_idmap *idmap; int open_flag = op->open_flag; bool do_truncate; + bool got_write; int acc_mode; int error; @@ -4935,12 +4936,23 @@ static int do_open(struct nameidata *nd, return -ENOTDIR; do_truncate = false; + got_write = false; acc_mode = op->acc_mode; if (file->f_mode & FMODE_CREATED) { /* Don't check for write permission, don't truncate */ open_flag &= ~O_TRUNC; acc_mode = 0; } else if (d_is_reg(nd->path.dentry) && open_flag & O_TRUNC) { + do_truncate = true; + } + + /* + * ->open() can write to the filesystem, e.g. when it initializes + * quota, so writable opens of regular files need freeze protection + * as well. O_TRUNC opens already need it for handle_truncate(). + */ + if (do_truncate || + (d_is_reg(nd->path.dentry) && (file->f_mode & FMODE_WRITE))) { error = mnt_want_write(nd->path.mnt); if (error) return error; @@ -4950,7 +4962,7 @@ static int do_open(struct nameidata *nd, * while we need to undo write access below. */ mnt = mntget(nd->path.mnt); - do_truncate = true; + got_write = true; } error = may_open(idmap, &nd->path, acc_mode, open_flag); if (!error && !(file->f_mode & FMODE_OPENED)) @@ -4963,7 +4975,7 @@ static int do_open(struct nameidata *nd, WARN_ON(1); error = -EINVAL; } - if (do_truncate) { + if (got_write) { mnt_drop_write(mnt); mntput(mnt); } -- 2.34.1