Re: [LTP] [PATCH v1] fsconfig: New case cover CVE-2022-0185

Wei Gao via ltp
2023-02-08T09:02:06Z

On Mon, Feb 06, 2023 at 05:19:53PM +0100, Petr Vorel wrote:
> Hi Wei,
>
> ...
> > > Hm, there is a kernel fix from 5.17 [1]. But test fails when I run it on 6.2.0-rc5:
>
> > > tst_supported_fs_types.c:165: TINFO: Skipping FUSE based ntfs as requested by the test
> > > tst_supported_fs_types.c:157: TINFO: Skipping tmpfs as requested by the test
> > > tst_test.c:1634: TINFO: === Testing on ext3 ===
> > > tst_test.c:1093: TINFO: Formatting /dev/loop0 with ext3 opts='' extra opts=''
> > > mke2fs 1.46.5 (30-Dec-2021)
> > > fsconfig03.c:44: TFAIL: fsconfig(FSCONFIG_SET_STRING) failed: EINVAL (22)
>
> > > Isn't it the opposite: we expect to fail, thus TST_EXP_FAIL() should here be
> > > used?
>
> > I have not test on 6.2.0 kernel, i need reproduce this firstly.
>
> FYI 6.0.6 is also broken, works on 5.10.46.
>

After long investigation i finally get what's happen now since i have
never touch kernel fs code before :)

The root caused is cebe85d570cf8 which make udpate initialize of
ext3_fs_type.

Each file system will initialize a struct file_system_type and ext3 initialize
in fs/ext4/super.c(maybe ext3 much same as ext4 so they put in same file).
This patch add new memeber .init_fs_context in ext3 file_system_type struct and
this new member will lead pase function which called by fsconfig change
from legacy_parse_param to ext4_parse_param(this function will check
parameter and not allow 0x00)

===key change part of cebe85d570cf8===
static struct file_system_type ext3_fs_type = {
- .owner = THIS_MODULE,
- .name = "ext3",
- .mount = ext4_mount,
- .kill_sb = kill_block_super,
- .fs_flags = FS_REQUIRES_DEV,
+ .owner = THIS_MODULE,
+ .name = "ext3",
+ .init_fs_context = ext4_init_fs_context, // in this patch init_fs_context start set ext4_init_fs_context
+ .parameters = ext4_param_specs,
+ .kill_sb = kill_block_super,
+ .fs_flags = FS_REQUIRES_DEV,
};
===key change part of cebe85d570cf8===

Following logic will decide whether use legacy_init_fs_context base on
exist of init_fs_context, obviously before patch we have no
init_fs_context but after patch we have it

==function alloc_fs_context==
.....
init_fs_context = fc->fs_type->init_fs_context;
if (!init_fs_context)
init_fs_context = legacy_init_fs_context; //before patch cebe85d570cf8, legacy_init_fs_context will be set.
ret = init_fs_context(fc);

==function alloc_fs_context==

====code example for set parse function used by fsconfig===
const struct fs_context_operations legacy_fs_context_ops = {
.free = legacy_fs_context_free,
.dup = legacy_fs_context_dup,
.parse_param = legacy_parse_param,
.parse_monolithic = legacy_parse_monolithic,
.get_tree = legacy_get_tree,
.reconfigure = legacy_reconfigure,
};

/*
* Initialise a legacy context for a filesystem that doesn't support
* fs_context.
*/
static int legacy_init_fs_context(struct fs_context *fc)
{
fc->fs_private = kzalloc(sizeof(struct legacy_fs_context), GFP_KERNEL_ACCOUNT);
if (!fc->fs_private)
return -ENOMEM;
fc->ops = &legacy_fs_context_ops;
return 0;
}
====code for set parse function used by fsconfig===

====final call parse function within fsconfig logic==
vfs_parse_fs_param
145 if (fc->ops->parse_param) {
146 ret = fc->ops->parse_param(fc, param); //this will call legacy_parse_param or ext4_parse_param
147 if (ret != -ENOPARAM)
148 return ret;
149 }
====final call parse function within fsconfig logic==

Just FYI the fs_type real data show in GDB(init_fs_context= 0 in kernel5.x but in kernel 6.x init_fs_context=ext4_parse_param):

(gdb) p *fs_type
$4 = {name = 0xffffffff822278e1 "ext3", fs_flags = 1, init_fs_context = 0x0 <fixed_percpu_data>, parameters = 0x0 <fixed_percpu_data>,
mount = 0xffffffff812ec510 <ext4_mount>, kill_sb = 0xffffffff811f7220 <kill_block_super>, owner = 0x0 <fixed_percpu_data>,
next = 0xffffffff82564600 <ext2_fs_type>, fs_supers = {first = 0x0 <fixed_percpu_data>}, s_lock_key = {<No data fields>},
s_umount_key = {<No data fields>}, s_vfs_rename_key = {<No data fields>}, s_writers_key = 0xffffffff825645e8, i_lock_key = {<No data fields>},
i_mutex_key = {<No data fields>}, invalidate_lock_key = {<No data fields>}, i_mutex_dir_key = {<No data fields>}}

> Kind regards,
> Petr