Re: [f2fs-dev] [PATCH] f2fs: add additional sanity check in f2fs_acl_from_disk()
On 08/30/2018 11:41 PM, Chao Yu wrote:
Hi Chengguang,
On 2018/8/30 21:33, Chengguang Xu wrote:
Add additinal sanity check for irregular case(e.g. corruption).
If size of extended attribution is smaller than size of acl header,
then return -EINVAL.
Signed-off-by: Chengguang Xu
---
fs/f2fs/acl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c
index 111824199a88..79e9ea773070 100644
--- a/fs/f2fs/acl.c
+++ b/fs/f2fs/acl.c
@@ -53,6 +53,9 @@ static struct posix_acl *f2fs_acl_from_disk(const char
*value, size_t size)
struct f2fs_acl_entry *entry = (struct f2fs_acl_entry *)(hdr + 1);
const char *end = value + size;
+ if (size < sizeof(f2fs_acl_header))
+ return ERR_PTR(-EINVAL);
I guess below codes have checked that already?
count = f2fs_acl_count(size);
if (count < 0)
return ERR_PTR(-EINVAL);
Hi Chao,
Thanks for prompt reply.
I still think in a rare case, it can pass the check in f2fs_acl_count()
and cause unexpected behavior.
For example, like below code path in f2fs_acl_count().
-> if (s < 0) {
if (size % sizeof(struct f2fs_acl_entry_short))
return -1;
-> return size / sizeof(struct f2fs_acl_entry_short);
}
Thanks,
Chengguang
--
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
Re: [f2fs-dev] [Jfs-discussion] [PATCH v2 4/5] jfs: cache NULL when both default_acl and acl are NULL
On 09/04/2018 04:34 AM, Dave Kleikamp wrote: Are you pushing these as a series, or would you like this patch pushed through the jfs tree? I'd hope maintainers pick individual patch to their tree. Thanks, Chengguang On 8/31/18 9:33 AM, Chengguang Xu wrote: default_acl and acl of newly created inode will be initiated as ACL_NOT_CACHED in vfs function inode_init_always() and later will be updated by calling xxx_init_acl() in specific filesystems. Howerver, when default_acl and acl are NULL then they keep the value of ACL_NOT_CACHED, this patch tries to cache NULL for acl/default_acl in this case. Signed-off-by: Chengguang Xu Acked-by: Dave Kleikamp -- 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
Re: [f2fs-dev] [PATCH v2 2/5] ext4: cache NULL when both default_acl and acl are NULL
On 08/31/2018 10:33 PM, Chengguang Xu wrote:
default_acl and acl of newly created inode will be initiated
as ACL_NOT_CACHED in vfs function inode_init_always() and later
will be updated by calling xxx_init_acl() in specific filesystems.
Howerver, when default_acl and acl are NULL then they keep the value
of ACL_NOT_CACHED, this patch tries to cache NULL for acl/default_acl
in this case.
Signed-off-by: Chengguang Xu
---
v1->v2:
- Coding style change.
fs/ext4/acl.c | 4
1 file changed, 4 insertions(+)
diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c
index fb50f9aa6ead..c1d570ee1d9f 100644
--- a/fs/ext4/acl.c
+++ b/fs/ext4/acl.c
@@ -284,12 +284,16 @@ ext4_init_acl(handle_t *handle, struct inode *inode,
struct inode *dir)
error = __ext4_set_acl(handle, inode, ACL_TYPE_DEFAULT,
default_acl, XATTR_CREATE);
posix_acl_release(default_acl);
+ } else {
+ inode->i_default_acl = NULL;
}
if (acl) {
if (!error)
error = __ext4_set_acl(handle, inode, ACL_TYPE_ACCESS,
acl, XATTR_CREATE);
posix_acl_release(acl);
+ } else {
+ inode->i_acl = NULL;
}
return error;
}
Hi Ted, Andreas
Have you got chance to look at this patch?
Thanks,
Chengguang
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] mount fail with option fault_injection < 20000 && >0
HI guys, I found mount with option fault_injection < 2 && >0 will fail with error -ENOMEM. I understand fault_injection function is explicitly mocking some errors, but isn't it better mocking errors after successful mount? Is it really allocating memory so many times during mount process? Thanks, Chengguang ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: remove default option setting in remount
On 09/18/2018 09:20 PM, Chao Yu wrote:
On 2018/9/18 14:23, Chengguang Xu wrote:
Currently we set default value to options before parsing remount options,
it will cause unexpected change of options which were specified in first
mount.
Can you check below commit? It looks w/o it we may lose default option after
remount.
498c5e9fcd10 ("f2fs: add default mount options to remount")
Hi Chao,
It looks like there was a bug in remount at that time, but I think the
fix above is not correct.
from the patch '498c5e9fcd10', I think it was caused by clearing
sbi->mount_opt.opt before
parsing. I think remount should not change the options which were
specified by user in
previous mount unless user specifies in remount.
Thanks,
Chengguang
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: remove default option setting in remount
On 9/19/18 10:02 PM, Chao Yu wrote:
On 2018/9/18 21:47, cgxu519 wrote:
On 09/18/2018 09:20 PM, Chao Yu wrote:
On 2018/9/18 14:23, Chengguang Xu wrote:
Currently we set default value to options before parsing remount options,
it will cause unexpected change of options which were specified in first
mount.
Can you check below commit? It looks w/o it we may lose default option after
remount.
498c5e9fcd10 ("f2fs: add default mount options to remount")
Hi Chao,
Hi Chengguang,
It looks like there was a bug in remount at that time, but I think the fix above
is not correct.
from the patch '498c5e9fcd10', I think it was caused by clearing
sbi->mount_opt.opt before
parsing. I think remount should not change the options which were specified by
user in
previous mount unless user specifies in remount.
Can you check description in manual of mount.
IIRC, old mount option will be record into /etc/mtab or /proc/mounts (for
adapting namespace feature), with command of "mount -o remount,rw /dir", old
mount options can be loaded from above config file, and merge them with new
specified options.
Even we kill old mount options by call default_options() in ->remount, user's
old mount option can still be set through parameters.
Please let me show you different behaviors before/after this patch.
[root@x201 cgxu]# uname -a
Linux x201 4.19.0-rc2+ #51 SMP Wed Sep 19 22:41:20 CST 2018 x86_64
x86_64 x86_64 GNU/Linux
Before:
[root@x201 cgxu]# mount -o fsync_mode=strict /dev/mapper/test-test1
/mnt/test/test1
[root@x201 cgxu]# mount | grep test1
/dev/mapper/test-test1 on /mnt/test/test1 type f2fs
(rw,relatime,lazytime,background_gc=on,discard,no_heap,user_xattr,inline_xattr,acl,inline_data,inline_dentry,flush_merge,extent_cache,mode=adaptive,active_logs=6,alloc_mode=reuse,fsync_mode=strict)
[root@x201 cgxu]# mount -o remount,background_gc=sync
/dev/mapper/test-test1 /mnt/test/test1
[root@x201 cgxu]# mount | grep test1
/dev/mapper/test-test1 on /mnt/test/test1 type f2fs
(rw,relatime,background_gc=sync,discard,no_heap,user_xattr,inline_xattr,acl,inline_data,inline_dentry,flush_merge,extent_cache,mode=adaptive,active_logs=6,alloc_mode=default,fsync_mode=posix)
I only changed background_gc in ->remount, but fsync_mode is implicitly
set to default value 'posix'.
After:
[root@x201 linus]# mount -o fsync_mode=strict /dev/mapper/test-test1
/mnt/test/test1
[root@x201 linus]# mount | grep test1
/dev/mapper/test-test1 on /mnt/test/test1 type f2fs
(rw,relatime,lazytime,background_gc=on,discard,no_heap,user_xattr,inline_xattr,acl,inline_data,inline_dentry,flush_merge,extent_cache,mode=adaptive,active_logs=6,alloc_mode=reuse,fsync_mode=strict)
[root@x201 linus]# mount -o remount,background_gc=sync
/dev/mapper/test-test1 /mnt/test/test1
[root@x201 linus]# mount | grep test1
/dev/mapper/test-test1 on /mnt/test/test1 type f2fs
(rw,relatime,background_gc=sync,discard,no_heap,user_xattr,inline_xattr,acl,inline_data,inline_dentry,flush_merge,extent_cache,mode=adaptive,active_logs=6,alloc_mode=reuse,fsync_mode=strict)
This time fysnc_mode looks fine.
But the problem here is, some old parameter can be configured via sysfs, if we
reset them in default_options(), we will lose them forever after remount.
If you have no other opinion about this, could you adapt your commit log?
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: remove default option setting in remount
On 09/20/2018 02:29 PM, Chao Yu wrote:
On 2018/9/20 7:54, cgxu519 wrote:
On 9/19/18 10:02 PM, Chao Yu wrote:
On 2018/9/18 21:47, cgxu519 wrote:
On 09/18/2018 09:20 PM, Chao Yu wrote:
On 2018/9/18 14:23, Chengguang Xu wrote:
Currently we set default value to options before parsing remount options,
it will cause unexpected change of options which were specified in first
mount.
Can you check below commit? It looks w/o it we may lose default option after
remount.
498c5e9fcd10 ("f2fs: add default mount options to remount")
Hi Chao,
Hi Chengguang,
It looks like there was a bug in remount at that time, but I think the fix above
is not correct.
from the patch '498c5e9fcd10', I think it was caused by clearing
sbi->mount_opt.opt before
parsing. I think remount should not change the options which were specified by
user in
previous mount unless user specifies in remount.
Can you check description in manual of mount.
IIRC, old mount option will be record into /etc/mtab or /proc/mounts (for
adapting namespace feature), with command of "mount -o remount,rw /dir", old
mount options can be loaded from above config file, and merge them with new
specified options.
Even we kill old mount options by call default_options() in ->remount, user's
old mount option can still be set through parameters.
Please let me show you different behaviors before/after this patch.
[root@x201 cgxu]# uname -a
Linux x201 4.19.0-rc2+ #51 SMP Wed Sep 19 22:41:20 CST 2018 x86_64
x86_64 x86_64 GNU/Linux
Before:
[root@x201 cgxu]# mount -o fsync_mode=strict /dev/mapper/test-test1
/mnt/test/test1
[root@x201 cgxu]# mount | grep test1
/dev/mapper/test-test1 on /mnt/test/test1 type f2fs
(rw,relatime,lazytime,background_gc=on,discard,no_heap,user_xattr,inline_xattr,acl,inline_data,inline_dentry,flush_merge,extent_cache,mode=adaptive,active_logs=6,alloc_mode=reuse,fsync_mode=strict)
[root@x201 cgxu]# mount -o remount,background_gc=sync
/dev/mapper/test-test1 /mnt/test/test1
[root@x201 cgxu]# mount | grep test1
/dev/mapper/test-test1 on /mnt/test/test1 type f2fs
(rw,relatime,background_gc=sync,discard,no_heap,user_xattr,inline_xattr,acl,inline_data,inline_dentry,flush_merge,extent_cache,mode=adaptive,active_logs=6,alloc_mode=default,fsync_mode=posix)
I only changed background_gc in ->remount, but fsync_mode is implicitly
set to default value 'posix'.
IMO, the resul is as expected, let's referenced description in manual of mount:
The remount functionality follows the standard way how the
mount command works with options from fstab. It means the mount command
doesn't read fstab (or mtab) only when a device and
dir are fully specified.
mount -o remount,rw /dev/foo /dir
After this call all old mount options are replaced and
arbitrary stuff from fstab is ignored, except the loop= option which is
internally generated and maintained by the mount command.
mount -o remount,rw /dir
After this call mount reads fstab (or mtab) and merges these
options with options from command line ( -o ).
So if you want to keep old mount option, you should use:
mount -o remount,background_gc=sync /mnt/test/test1
instead of
mount -o remount,background_gc=sync /dev/mapper/test-test1 /mnt/test/test1
I get your point about how mount command organizes options ,
but it seems other filesystem do not initialize mount option in remount.
What do you think for that? and if we keep initialization in f2fs,
shouldn't we set default value to all mount options for remount?
Thanks,
Chengguang
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
