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

[Jan Stancek]

On Wed, Nov 15, 2023 at 9:23 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> > 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).

If we mask already set TAINT_W, you can get PASS by mistake,
if we don't mask it, you can get FAIL by mistake - which one is better?

I believe at the time we saw masking as better alternative, since many
CI systems also look at dmesg/journal and would find WARNING messages
even when test reports PASS by mistake. So the failure wouldn't go unnoticed.