Re: [LTP] [PATCH v2 5/5] Add test for CVE 2023-31248

Petr Vorel
2023-11-15T08:23:35Z

> Hi Martin, Petr.

> 2023年11月15日(水) 0:39 Petr Vorel <pvorel@suse.cz>:

> > Hi Martin,

> > ...
> > > +++ b/testcases/network/iptables/nft02.c
> > ...
> > > +/* Chain creation and deletion config */
> > > +static const struct tst_netlink_attr_list newchain_config[] = {
> > > + {NFTA_TABLE_NAME, TABNAME, strlen(TABNAME) + 1, NULL},
> > > + {NFTA_CHAIN_NAME, CHAINNAME, strlen(CHAINNAME) + 1, NULL},
> > > + {NFTA_CHAIN_ID, &chain_id, sizeof(chain_id), NULL},

> > Unfortunately the current oldest distros (Leap 42 and Ubuntu Bionic) are too old
> > for NFTA_CHAIN_ID, NFTA_RULE_CHAIN_ID, could you please add them to LAPI?

> It was the same on RHEL8 (and/or other clone OS).

> And, I would like to see this test added to runtest/cve and .gitignore.

Good point, thanks. Maybe it could be also in runtest/net.tcp_cmds.

> > + if (tst_taint_check()) {
> > + tst_res(TFAIL, "Kernel is vulnerable");
> > + return;
> > + }

> Not a problem with the test itself but, If TAINT_W was already set,
> the test report TPASS by misstake.
> Below patch masks TAINT_W if that taint flag was set before test started.

> commit de9b5c2636204ab06dbb3c7dc57296d73ac572ac
> Author: Jan Stancek <jstancek@redhat.com>
> Date: Tue Apr 21 14:15:48 2020 +0200

> lib: tst_taint: Ignore WARN taint flag if already set

> This commit changes the library so that it ignores the taint warn flag
> if it was set prior to the test run. It turns out that the warn taint
> flag is not well defined and could be easily set on a freshly booted
> kernel for example when buggy BIOS is detected.

> Other recent example is disabling ip forward on kvm guests:
> https://github.com/containers/libpod/issues/5815
> https://lore.kernel.org/netdev/a47b6a3b-c064-2f53-7cf6-d0d0720e9d99@redhat.com/

> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> Cc: Chang Yin <cyin@redhat.com>
> Reviewed-by: Li Wang <liwang@redhat.com>

> diff --git a/lib/tst_taint.c b/lib/tst_taint.c
> index a5dbf77d2..49146aacb 100644
> --- a/lib/tst_taint.c
> +++ b/lib/tst_taint.c
> @@ -82,9 +82,14 @@ void tst_taint_init(unsigned int mask)
> tst_res(TCONF, "Kernel is too old for requested mask");

> taint_mask = mask;
> -
> taint = tst_taint_read();
> - if ((taint & mask) != 0)
> +
> + if (taint & TST_TAINT_W) {
> + tst_res(TCONF, "Ignoring already set kernel warning taint");
> + taint_mask &= ~TST_TAINT_W;
> + }
> +
> + if ((taint & taint_mask) != 0)
> tst_brk(TBROK, "Kernel is already tainted: %u", taint);
> }

> Other tests rely on TAINT_W to decide the result also report TPASS by
> misstake if TAINT_W was already set.
> In my patch v1 about CVE-2023-31248 check error == ENOENT. In this
> case, test reports correct result.
> It may be difficult to implement test like patch v1 using the internal
> tst_netlink API, but we should be aware of this issue.

Cc Jan. But if I read it correctly if the kernel is already tainted with
TAINT_W, this flag is indeed skipped, but it can still be detected via
TST_TAINT_D (oops).

Kind regards,
Petr

> Best regards.
> Souta Kawahara <souta.kawahara@miraclelinux.com>

> > Otherwise the patchset looks ok.

> > Kind regards,
> > Petr

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