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

[Petr Vorel <pvorel@suse.cz>]

> 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.
What exactly happen in cebe85d570cf ("ext4: switch to the new mount api")
from v5.17-rc1? This enables fsconfig, thus test *should* be working on >=
v5.17-rc1, right? But it does not.

BTW xfs also uses new mount API 73e5fff98b64 ("xfs: switch to use the new
mount-api") in v5.5-rc1.  Obviously legacy ext2 driver does not use it.

I'd expect fsopen_supported_by_kernel() is enough and no filesystem needs to be
filtered out (at least not real linux filesystems like ext2 or xfs).

> 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).
Yes, ext3 and ext4 share the same code (ext4 driver). This code also servers
ext2 filesystem if CONFIG_EXT4_USE_FOR_EXT2=y (mostly the default), otherwise
there is ext2 driver: CONFIG_EXT2_FS).

Kind regards,
Petr

> 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

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp