Re: [LTP] [PATCH] cve: add CVE-2025-38236 test

[Petr Vorel <pvorel@suse.cz>]

Hi Andrea,

> +++ b/testcases/cve/cve-2025-38236.c
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2025 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +/*\
> + * Test for CVE-2025-38236 fixed in kernel v6.16-rc4:
> + * af_unix: Don't leave consecutive consumed OOB skbs.
> + *
> + * The bug is triggered by sending multiple out-of-band data to a socket and
> + * reading it back from it. According to the MSG_OOB implementation, this
> + * shouldn't be possible. When system is affected by CVE-2025-38236, instead,
> + * skb queue holds MSG_OOB data, breaking recv() and causing a use-after-free
> + * condition.
> + *
> + * Even if MSG_OOB is mostly used inside Oracle's product, it is enabled by
> + * default in linux kernel via CONFIG_AF_UNIX_OOB. This is accessible via
> + * Chrome's renderer sandbox, which might cause an attacker to escalate and to
> + * obtain privileges in the system.

Maybe mention it's based on the reproducer from
https://project-zero.issues.chromium.org/issues/423023990

(That adds both background info + kind of credit of the author of the patch who
was the author of the initial reproducer).

> + */
> +
> +#include "tst_test.h"
> +
> +static const struct timeval sock_timeout = {
> +	.tv_sec = 1,
> +};
> +
> +static char dummy;
> +static int sock[2];
> +
> +static void run(void)
> +{
> +	int ret;
> +
> +	dummy = '\0';
> +
> +	tst_res(TINFO, "#1 send and receive out-of-band data");
> +	SAFE_SEND(0, sock[1], "A", 1, MSG_OOB);
> +	SAFE_RECV(0, sock[0], &dummy, 1, MSG_OOB);
> +
> +	tst_res(TINFO, "#2 send and receive out-of-band data");
> +	SAFE_SEND(0, sock[1], "A", 1, MSG_OOB);
> +	SAFE_RECV(0, sock[0], &dummy, 1, MSG_OOB);
> +
> +	tst_res(TINFO, "Send out-of-band data");
> +	SAFE_SEND(0, sock[1], "A", 1, MSG_OOB);
> +
> +	tst_res(TINFO, "Receive data from normal stream");
> +
> +	ret = recv(sock[0], &dummy, 1, 0);

+1 the core part from the original verifier.

> +	if (ret == -1) {
> +		if (errno == EWOULDBLOCK)
> +			tst_res(TPASS, "Can't read out-of-band data from normal stream");
> +		else
> +			tst_brk(TBROK | TERRNO, "recv error");
> +	} else {
very nit: using 'if' with return is more readable than short 'if' part and long
'else' part:

if (...) {
	...
	return;
}

const char *msg = "We are able to read out-of-band data from normal stream";
...


> +		const char *msg = "We are able to read out-of-band data from normal stream";
> +
> +		if (dummy == 'A') {
> +			tst_res(TFAIL, "%s", msg);
> +		} else {
> +			tst_res(TFAIL, "%s, but data doesn't match: '%c' != 'A'",
> +				msg, dummy);
> +		}

> +
> +		SAFE_RECV(0, sock[0], &dummy, 1, MSG_OOB);

FYI: on vulnerable kernel with SELinux I get (nothing to be fixed):
cve-2025-38236.c:48: TINFO: Receive data from normal stream
cve-2025-38236.c:60: TFAIL: We are able to read out-of-band data from normal stream
cve-2025-38236.c:66: TBROK: recv(3, 0x4391d8, 1, 1) failed: EFAULT (14)
> +
> +		tst_res(TFAIL, "We are able to access data from skb queue (use-after-free)");
> +	}
> +}
> +
> +static void setup(void)
> +{
> +	SAFE_SOCKETPAIR(AF_UNIX, SOCK_STREAM, 0, sock);
> +	SAFE_SETSOCKOPT(sock[0], SOL_SOCKET, SO_RCVTIMEO,
> +		 &sock_timeout, sizeof(struct timeval));
> +}

Why is struct timeval needed? I haven't found that in
https://project-zero.issues.chromium.org/issues/423023990
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=32ca245464e1

and test works as a verifier without it. If really not needed please remove it
before merge.

> +
> +static void cleanup(void)
> +{
> +	if (sock[0] != -1)
> +		SAFE_CLOSE(sock[0]);
> +
> +	if (sock[1] != -1)
> +		SAFE_CLOSE(sock[1]);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_kconfigs = (const char *[]) {
> +		"CONFIG_AF_UNIX_OOB=y",
Although this is true I wonder if we should limit the reproducer to this.
If one day config silently renames / is removed (but OOB kept) the reproducer
will be lost.

LGTM, thanks for a quickly porting the reproducer!

With added entry to runtest/cve you may add:
Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

> +		NULL
> +	},
> +	.tags = (const struct tst_tag[]) {
> +		{"linux-git", "32ca245464e1"},
> +		{"CVE", "2025-38236"},
> +		{}
> +	}
> +};

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