[LTP] [PATCH] Add setsockopt10 TLS ULP UAF CVE-2023-0461

[Cyril Hrubis]

Hi!
> > Should we set .needs_drivers for the test so that it only attempts to
> > run either if TLS is compiled in or could be modprobed as a module?
> 
> Yes if it worked :-D, although I would still want to keep the above check. I
> tried it and noticed a number of problems.
> 
> On NixOS:
> 
> $ ./setsockopt10
> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
> tst_kernel.c:110: TWARN: expected file /lib/modules/6.5.5/modules.dep does not exist or not a file
> tst_kernel.c:110: TWARN: expected file /lib/modules/6.5.5/modules.builtin does not exist or not a file

Can you strace modprobe to see what is different on the system, these
files have to be installed somewhere in order for modprobe to actually
work...

> tst_test.c:1195: TCONF: tls driver not available
> $ lsmod | grep tls
> tls                   151552  0
> 
> We have a special switch for Android which disables the check, but it is
> not just Android. Granted NixOS is really wierd by desktop standards,
> but we have ALP and embedded systems to think about.
>
> AFAICT the test library does not do a modprobe if the driver is
> missing. Some tests (e.g. zram, CAN) do that and then claim to require
> modprobe. This is not good, those tests do not need modprobe if the
> module is loaded or compiled in.

That all should be handled in the library when .need_drivers is set.

The problem is that without the modules.buildin file we cannot easily
tell if things have been compiled in. If we manage to find the location
of the files used by modprobe we should do something as:

1) Is foo in modules.buildin -> return to the test
2) Is foo in /proc/modules -> return to the text
3) Is foo in mdules.dep -> try modprobe foo

> Perhaps if the check in tst_kernel fails for any reason we could just do
> a modprobe (or use the configured kernel usermode helper if it is
> set). If that fails we just carry on, like for Android. That's a
> seperate patch though IMO. Once we have a solution for that, this test
> can have needs_drivers added.

I suppose that just calling modprobe and check the return value would
work as well, however that would make it hard dependency.

> >> +	else if (TST_RET == -1)
> >> +		tst_brk(TBROK | TTERRNO, "parent: setsockopt failed");
> >> +
> >> +	SAFE_SETSOCKOPT(tcp1_sk, SOL_TLS, TLS_TX, &opts, sizeof(opts));
> >> +	TST_CHECKPOINT_WAKE(1);
> >> +
> >> +	tst_res(TINFO, "parent: Disconnect by setting unspec address");
> >> +	SAFE_CONNECT(tcp1_sk, &unspec_addr, sizeof(unspec_addr));
> >> +	SAFE_BIND(tcp1_sk, (struct sockaddr *)&tcp1_addr, sizeof(tcp1_addr));
> >> +
> >> +	TEST(listen(tcp1_sk, 1));
> >> +
> >> +	if (TST_RET == -1) {
> >> +		if (TST_ERR == EINVAL)
> >> +			tst_res(TPASS | TTERRNO, "parent: Can't listen on disconnected TLS socket");
> >> +		else
> >> + tst_res(TCONF | TTERRNO, "parent: Can't listen on disconnected TLS
> >> socket, but the errno is not EINVAL as expected");
> >> +
> >> +		TST_CHECKPOINT_WAKE(2);
> >> +		goto out;
> >> +	}
> >> +
> >> +	tst_res(TINFO, "parent: Can listen on disconnected TLS socket");
> >> +	TST_CHECKPOINT_WAKE(2);
> >> +
> >> +	tcp2_sk = SAFE_ACCEPT(tcp1_sk, NULL, 0);
> >> +	SAFE_CLOSE(tcp2_sk);
> >> +
> >> + tst_res(TINFO, "parent: Attempting double free, because we set
> >> cipher options this should result in an crash");
> >> +	SAFE_CLOSE(tcp1_sk);
> >> +
> >> +	TST_CHECKPOINT_WAKE(3);
> >> +	usleep(0);
> >
> > Did you forget this here?
> 
> It's supposed to give the kernel chance to propagate the taint or
> panic. Which stops it from printing there was no kernel taint in
> parallel with the kernel splat. It appears to work on my test system.

Right, so it's really just sched_yield(), maybe explicit sched_yield()
would be more descriptive.

> >> +	if (tst_taint_check())
> >> +		tst_res(TFAIL, "Kernel is tainted");
> >> +	else
> >> +		tst_res(TCONF, "No kernel taint or crash, maybe the kernel can clone the TLS-ULP context now?");
> >
> > If you set up .taint_check this is going to be redundant since we print
> > TFAIL in the test library in that case.
> 
> In that case we can unconditionally do TCONF and it'll be overridden by
> fail and the resulting panic. Or it will be prevented by the panic.

I guess that we should make the message "No kernel crash, maybe ..."
instead in that case.

> >> +out:
> >> +	tst_reap_children();
> >> +}
> >> +
> >> +static struct tst_test test = {
> >> +	.setup = setup,
> >> +	.cleanup = cleanup,
> >> +	.test_all = run,
> >> +	.forks_child = 1,
> >> +	.needs_checkpoints = 1,
> >> +	.taint_check = TST_TAINT_W | TST_TAINT_D,
> >> +	.needs_kconfigs = (const char *[]) {
> >> +		"CONFIG_TLS",
> >> +		NULL
> >> +	},
> >> +	.tags = (const struct tst_tag[]) {
> >> +		{"linux-git", "2c02d41d71f90"},
> >> +		{"CVE", "2023-0461"},
> >> +		{}
> >> +	}
> >> +};
> >> +
> >> +#else
> >> +
> >> +TST_TEST_TCONF("linux/tls.h missing, we assume your system is too old");
> >> +
> >> +#endif
> >> -- 
> >> 2.40.1
> >> 
> >> 
> >> -- 
> >> Mailing list info: https://lists.linux.it/listinfo/ltp
> 
> 
> -- 
> Thank you,
> Richard.

-- 
Cyril Hrubis
chrubis@suse.cz