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

[Wei Gao via ltp <ltp@lists.linux.it>]

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

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