Bug 202637 - Chmod'ed directory permission is not persisted with fsync
Summary: Chmod'ed directory permission is not persisted with fsync
Status: RESOLVED CODE_FIX
Alias: None
Product: File System
Classification: Unclassified
Component: f2fs (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Default virtual assignee for f2fs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-02-21 03:05 UTC by Seulbae Kim
Modified: 2019-03-16 08:03 UTC (History)
1 user (show)

See Also:
Kernel Version: v5.0.0-rc7 (latest)
Tree: Mainline
Regression: No


Attachments
Proof of Concept (635 bytes, text/x-csrc)
2019-02-21 03:05 UTC, Seulbae Kim
Details

Description Seulbae Kim 2019-02-21 03:05:57 UTC
Created attachment 281251 [details]
Proof of Concept

[Kernel version]
This bug can be reproduced on
kernel 5.0.0-rc7.

[Reproduce] * Use a VM, since our PoC simulates a crash by triggering a SysRq!
1. Download base image
$ wget https://gts3.org/~seulbae/fsimg/f2fs-10.image

2. Mount image
$ mkdir /tmp/f2fs
$ sudo mount -o loop f2fs-10.image /tmp/f2fs

3. Compile and run PoC
$ gcc poc.c -o poc
$ sudo ./poc /tmp/f2fs
(System reboots)

[Check]
1. Re-mount the crashed image
$ mkdir /tmp/f2fs
$ sudo mount -o loop f2fs-10.image /tmp/f2fs

2. Check inconsistency
$ stat /tmp/f2fs/foo
-> Access: (0755/drwxr-xr-x)


[Description]
In the base image, 2 directories and 7 files exist.

0: 0755 (mount_point)
+--257: 0755 foo
   +--258: 0755 bar
      +--259: 0644 baz (12 bytes, offset: {})
      +--259: 0644 hln (12 bytes, offset: {})
      +--260: 0644 xattr (0 bytes, offset: {})
      +--261: 0644 acl (0 bytes, offset: {})
      +--262: 0644 æøå (4 bytes, offset: {})
      +--263: 0644 fifo
      +--264: 0777 sln -> mnt/foo/bar/baz

The PoC basically
1. opens directory "foo",
(line 26) fd = syscall(SYS_open, "./foo", O_DIRECTORY, 0);
2. changes the permission of it to 0666,
(line 27) syscall(SYS_chmod, "./foo", 0666);
3. flushes its metadata, and then
(line 28) syscall(SYS_fsync, fd);
4. simulates a crash by rebooting right away without unmounting.
(line 30) system("echo b > /proc/sysrq-trigger");

As we run fsync operation on "foo",
we expect that the metadata regarding
the new permission is successfully flushed to disk,
and when we remount the crashed image,
we will see that "foo"'s mode is changed to 0666.

However, the directory still has its old mode, 0755.
Comment 1 Chao Yu 2019-02-24 03:12:04 UTC
Seulbae, thanks for the report.

I've written a patch for that, could you please check it?

[PATCH] f2fs: fix to dirty inode for i_mode recovery
Comment 2 Seulbae Kim 2019-02-25 19:30:47 UTC
Thank you for quick response Chao Yu!
I tested your patch by applying it on kernel v5.0.0-rc7, and confirmed that the reported bug is fixed.

(In reply to Chao Yu from comment #1)
> Seulbae, thanks for the report.
> 
> I've written a patch for that, could you please check it?
> 
> [PATCH] f2fs: fix to dirty inode for i_mode recovery
Comment 3 Chao Yu 2019-02-26 08:40:00 UTC
Thanks for the quick test. :)

I've also sent a patch to simulate your testcase for fstest suit, you can find it in below link:

https://sourceforge.net/p/linux-f2fs/mailman/linux-f2fs-devel/thread/20190226070302.17756-1-yuchao0%40huawei.com/#msg36597256

Once the kernel fix patch is merged in upstream, I will close this issue.
Comment 4 Chao Yu 2019-03-16 08:03:02 UTC
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ca597bddedd94906cd761d8be6a3ad21292725de

The fixing patch has been merged, tagged this issue as resolved one.

Note You need to log in before you can comment on or make changes to this bug.