| Message ID | 1462150237-20701-1-git-send-email-daeho.jeong@samsung.com |
|---|---|
| State | Deferred |
| Headers | show |
On Mon 02-05-16 09:50:37, Daeho Jeong wrote: > We check whether a new handle can be started through > ext4_journal_check_start() and the function refuses to start the handle > when the filesystem is mounted with read-only. But now, when we remount > the filesystem with read-only option, already started handles are > allowed to be written on disk, but the subsequent metadata modification > using the handles are refused by ext4_journal_check_start(). > > As an example, in ext4_evict_inode(), i_size can be set to 0 using > a successfully started handle, but, when we remount the filesystem > with read-only option at that time, the subsequent ext4_truncate() > will be failed and the filesystem integrity will be damaged. > > Therefore, we need to permit the metadata modification using already > started handles to be proceeded, even if s_flags of the filesystem is > set to MS_RDONLY. > > Kitae found the problem and suggested the solution. So can you share a reproducer for this issue? Because my initial thinking is that checks during remount should fail the remount with EBUSY if there is any modification outstanding... If they don't we have a racy remount and fs-freezing code, which is a bug. Honza
On Mon, May 02, 2016 at 09:50:37AM +0900, Daeho Jeong wrote: > We check whether a new handle can be started through > ext4_journal_check_start() and the function refuses to start the handle > when the filesystem is mounted with read-only. But now, when we remount > the filesystem with read-only option, already started handles are > allowed to be written on disk, but the subsequent metadata modification > using the handles are refused by ext4_journal_check_start(). > > As an example, in ext4_evict_inode(), i_size can be set to 0 using > a successfully started handle, but, when we remount the filesystem > with read-only option at that time, the subsequent ext4_truncate() > will be failed and the filesystem integrity will be damaged. > > Therefore, we need to permit the metadata modification using already > started handles to be proceeded, even if s_flags of the filesystem is > set to MS_RDONLY. > > Kitae found the problem and suggested the solution. > > Signed-off-by: Kitae Lee <kitae87.lee@samsung.com> > Signed-off-by: Daeho Jeong <daeho.jeong@samsung.com> Hmm, I'm not really comfortable with putting this hack in, since this is papering over the real problem, which is that Android is trying to use the emergency remount read-only sysrq option and this is fundamentally unsafe. I'm not sure what else could break if it is situation normal that there is active processes busily writing to the file system and sysrq-u followed by reboot is the normal way the Android kernel does a reboot. A much better solution would be to change the Android userspace to call the FIFREEZE ioctl on each mounted file system, and then call for a reboot. - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/ext4/ext4_jbd2.c b/fs/ext4/ext4_jbd2.c index e770c1ee..3b59e11 100644 --- a/fs/ext4/ext4_jbd2.c +++ b/fs/ext4/ext4_jbd2.c @@ -43,7 +43,7 @@ static int ext4_journal_check_start(struct super_block *sb) journal_t *journal; might_sleep(); - if (sb->s_flags & MS_RDONLY) + if (sb->s_flags & MS_RDONLY && ext4_journal_current_handle() == NULL) return -EROFS; WARN_ON(sb->s_writers.frozen == SB_FREEZE_COMPLETE); journal = EXT4_SB(sb)->s_journal;