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

[Petr Vorel <pvorel@suse.cz>]

Hi Wei,

...
> +++ b/include/lapi/fsmount.h
> @@ -11,12 +11,15 @@
>  #include "config.h"
>  #include <sys/syscall.h>
>  #include <sys/types.h>
> -#include <sys/mount.h>

>  #ifndef HAVE_FSOPEN
>  # ifdef HAVE_LINUX_MOUNT_H
>  #  include <linux/mount.h>
> +# else
> +#  include <sys/mount.h>
>  # endif
> +#else
> +# include <sys/mount.h>
>  #endif
Does <linux/mount.h> conflicts with <sys/mount.h>? Or why is this needed?

>  #include "lapi/fcntl.h"
> diff --git a/runtest/syscalls b/runtest/syscalls
> index ae37a1192..b4cde8071 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -383,6 +383,7 @@ fremovexattr02 fremovexattr02

>  fsconfig01 fsconfig01
>  fsconfig02 fsconfig02
> +fsconfig03 fsconfig03

NOTE: you also need to add a new record in testcases/kernel/syscalls/fsconfig/.gitignore.

> diff --git a/testcases/kernel/syscalls/fsconfig/fsconfig03.c b/testcases/kernel/syscalls/fsconfig/fsconfig03.c
> new file mode 100644
> index 000000000..e076c2f09
> --- /dev/null
> +++ b/testcases/kernel/syscalls/fsconfig/fsconfig03.c
> @@ -0,0 +1,58 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2023 Wei Gao <wegao@suse.com>
> + */
> +
> +/*\
NOTE, there should be docparse label:
 * [Description]
> + * Test add some coverage to CVE-2022-0185.
> + * Try to trigger a crash.
> + * References links:
> + * https://www.openwall.com/lists/oss-security/2022/01/25/14
> + * https://github.com/Crusaders-of-Rust/CVE-2022-0185
> + */
> +
> +#include "tst_test.h"
> +#include "lapi/fsmount.h"
> +
> +#define MNTPOINT	"mntpoint"
> +
> +static int fd = -1;
> +
> +static void setup(void)
> +{
> +	fsopen_supported_by_kernel();
> +
> +	TEST(fd = fsopen(tst_device->fs_type, 0));
> +	if (fd == -1)
> +		tst_brk(TBROK | TTERRNO, "fsopen() failed");
Sooner or later we should add SAFE_FSOPEN(), but that can wait.

> +
> +}
> +
> +static void cleanup(void)
> +{
> +	if (fd != -1)
> +		SAFE_CLOSE(fd);
> +}
> +
> +static void run(void)
> +{
> +	char *val = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
> +
> +	for (unsigned int i = 0; i < 2; i++) {
> +		TEST(fsconfig(fd, FSCONFIG_SET_STRING, "\x00", val, 0));
> +		if (TST_RET == -1)
> +			tst_brk(TFAIL | TTERRNO, "fsconfig(FSCONFIG_SET_STRING) failed");
TST_EXP_PASS() or other could here be used (it should be changes also in fsconfig01.c).

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?

> +	}
> +	tst_res(TPASS, "Try fsconfig overflow on %s done!", tst_device->fs_type);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_root = 1,
> +	.format_device = 1,
> +	.mntpoint = MNTPOINT,
> +	.all_filesystems = 1,
> +	.skip_filesystems = (const char *const []){"fuse", "ext2", "xfs", "tmpfs", NULL},

I wonder why this is should not be run on XFS and ext2.

Also, while we have CVE and kernel fix [1], it should be marked in struct tst_test:

	.tags = (const struct tst_tag[]) {
		{"linux-git", "722d94847de2"},
		{"CVE", "2020-29373"},
		{"CVE", "2022-0185"},
		{}
	}

Kind regards,
Petr

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=722d94847de2


> +};

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