Hi Jaegeuk,
On 2017/5/17 4:28, Jaegeuk Kim wrote:
> Hi Tom,
>
> On 05/16, Tom Yan wrote:
>> Hi all.
>>
>> Just happen to notice that a file on my f2fs filesystem marked as immutable
>> is actually "mutable". At first I thought it was because of file corruption
>> or so, but after some test it appears that the effect will be gone anyway
>> once the filesystem is remounted. The flag remains though, as per the output
>> of lsattr.
>>
>
> Thank you so much for the report.
>
>> Here is a test case:
>>
>> [tom@localhost ~]$ truncate -s 256M f2fs.img
>> [tom@localhost ~]$ sudo losetup -f f2fs.img
>> [tom@localhost ~]$ sudo mkfs.f2fs /dev/loop0
>>
>> F2FS-tools: mkfs.f2fs Ver: 1.8.0 (2017-02-03)
>>
>> Info: Debug level = 0
>> Info: Label =
>> Info: Trim is enabled
>> Info: Segments per section = 1
>> Info: Sections per zone = 1
>> Info: sector size = 512
>> Info: total sectors = 524288 (256 MB)
>> Info: zone aligned segment0 blkaddr: 512
>> Info: format version with
>> "Linux version 4.10.13-1-ARCH (builduser@tobias) (gcc version 6.3.1
>> 20170306 (GCC) ) #1 SMP PREEMPT Thu Apr 27 12:15:09 CEST 2017"
>> Info: [/dev/loop0] Discarding device
>> Info: This device doesn't support BLKSECDISCARD
>> Info: Discarded 256 MB
>> Info: Overprovision ratio = 15.000%
>> Info: Overprovision segments = 35 (GC reserved = 21)
>> Info: format successful
>> [tom@localhost ~]$ sudo mount /dev/loop0 /mnt/
>> [tom@localhost ~]$ sudo chown tom:tom /mnt/
>> [tom@localhost ~]$ touch /mnt/testfile
>> [tom@localhost ~]$ sudo chattr +i /mnt/testfile
>> [tom@localhost ~]$ echo test > /mnt/testfile
>> bash: /mnt/testfile: Operation not permitted
>> [tom@localhost ~]$ rm /mnt/testfile
>> rm: cannot remove '/mnt/testfile': Operation not permitted
>> [tom@localhost ~]$ sudo umount /mnt/
>> [tom@localhost ~]$ sudo mount /dev/loop0 /mnt/
>> [tom@localhost ~]$ lsattr /mnt/testfile
>> ----i-------------- /mnt/testfile
>> [tom@localhost ~]$ echo test > /mnt/testfile
>> [tom@localhost ~]$ rm /mnt/testfile
>> [tom@localhost ~]$ sudo umount /mnt/
>> [tom@localhost ~]$ uname -a
>> Linux localhost 4.10.13-1-ARCH #1 SMP PREEMPT Thu Apr 27 12:15:09 CEST 2017
>> x86_64 GNU/Linux
>>
>> As you can see, mkfs.f2fs is of version 1.8.0, and the kernel is as of
>> 4.10.13. Let me know if you need further information. Thanks in advance.
>
> I could simply reproduce this, and wrote a patch to fix it like below. ;)
> Could you please check this patch as well?
>
> Thanks,
>
>>From 8aee3d3a0244802b141d1b2ae95c568d1aa26118 Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim <jaeg...@kernel.org>
> Date: Tue, 16 May 2017 13:20:16 -0700
> Subject: [PATCH] f2fs: load inode's flag from disk
>
> This patch fixes missing inode flag loaded from disk, reported by Tom.
>
> [tom@localhost ~]$ sudo mount /dev/loop0 /mnt/
> [tom@localhost ~]$ sudo chown tom:tom /mnt/
> [tom@localhost ~]$ touch /mnt/testfile
> [tom@localhost ~]$ sudo chattr +i /mnt/testfile
> [tom@localhost ~]$ echo test > /mnt/testfile
> bash: /mnt/testfile: Operation not permitted
> [tom@localhost ~]$ rm /mnt/testfile
> rm: cannot remove '/mnt/testfile': Operation not permitted
> [tom@localhost ~]$ sudo umount /mnt/
> [tom@localhost ~]$ sudo mount /dev/loop0 /mnt/
> [tom@localhost ~]$ lsattr /mnt/testfile
> ----i-------------- /mnt/testfile
> [tom@localhost ~]$ echo test > /mnt/testfile
> [tom@localhost ~]$ rm /mnt/testfile
> [tom@localhost ~]$ sudo umount /mnt/
>
> Cc: sta...@vger.kernel.org
> Reported-by: Tom Yan <tom.t...@outlook.com>
> Signed-off-by: Jaegeuk Kim <jaeg...@kernel.org>
> ---
> fs/f2fs/inode.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index 518f49643092..7aab8837b6b4 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -226,6 +226,7 @@ struct inode *f2fs_iget(struct super_block *sb, unsigned
> long ino)
> ret = -EIO;
> goto bad_inode;
> }
> + f2fs_set_inode_flags(inode);
f2fs_iget will make inode dirty after f2fs_set_inode_flags, it's not needed. So
how about changing like this?
---
fs/f2fs/file.c | 1 +
fs/f2fs/inode.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index e20a1c01e556..8ccbfe53c03c 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1514,6 +1514,7 @@ static int f2fs_ioc_setflags(struct file *filp, unsigned
long arg)
inode->i_ctime = current_time(inode);
f2fs_set_inode_flags(inode);
+ f2fs_mark_inode_dirty_sync(inode, false);
inode_unlock(inode);
out:
diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
index 518f49643092..e53c784ab11e 100644
--- a/fs/f2fs/inode.c
+++ b/fs/f2fs/inode.c
@@ -44,7 +44,6 @@ void f2fs_set_inode_flags(struct inode *inode)
new_fl |= S_DIRSYNC;
inode_set_flags(inode, new_fl,
S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC);
- f2fs_mark_inode_dirty_sync(inode, false);
}
static void __get_inode_rdev(struct inode *inode, struct f2fs_inode *ri)
@@ -226,6 +225,7 @@ struct inode *f2fs_iget(struct super_block *sb, unsigned
long ino)
ret = -EIO;
goto bad_inode;
}
+ f2fs_set_inode_flags(inode);
unlock_new_inode(inode);
trace_f2fs_iget(inode);
return inode;
--
2.13.0.67.g10c78a162fa8
> unlock_new_inode(inode);
> trace_f2fs_iget(inode);
> return inode;
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
