linux-kernel - Re: [PATCH v1 00/17] selftests/nolibc: allow run with minimal kernel config

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 24 Jun 2023 16:54:03 +0800
From:   Zhangjin Wu <falcon@...ylab.org>
To:     thomas@...ch.de
Cc:     falcon@...ylab.org, arnd@...db.de, linux-kernel@...r.kernel.org,
        linux-kselftest@...r.kernel.org, w@....eu
Subject: Re: [PATCH v1 00/17] selftests/nolibc: allow run with minimal kernel config

Hi, Thomas

> > Hi Zhangjin,
> > 
> > some general comments for the whole series.
> > 
> > On 2023-06-21 20:52:30+0800, Zhangjin Wu wrote:
> > > Hi, Willy
> > > 
> > > This patchset mainly allows speed up the nolibc test with a minimal
> > > kernel config.
> > > 
> (snip)
> > > 
> > > * selftests/nolibc: fix up kernel parameters support
> > > 
> > >   kernel cmdline allows pass two types of parameters, one is without
> > >   '=', another is with '=', the first one is passed as init arguments,
> > >   the sencond one is passed as init environment variables.
> > > 
> > >   Our nolibc-test prefer arguments to environment variables, this not
> > >   work when users add such parameters in the kernel cmdline:
> > > 
> > >     noapic NOLIBC_TEST=syscall
> > > 
> > >   So, this patch will verify the setting from arguments at first, if it
> > >   is no valid, will try the environment variables instead.
> > 
> > This would be much simpler as:
> > 
> > test = getenv("NOLIBC_TEST");
> > if (!test)
> >         test = argv[1];
> >
> > It changes the semantics a bit, but it doesn't seem to be an issue.
> > (Maybe gated behind getpid() == 1).
> 
> Cool suggestion, it looks really better:
> 
> 	if (getpid() == 1) {
> 		prepare();
> 		
> 		/* kernel cmdline may pass: "noapic NOLIBC_TEST=syscall",
>                  * to init program:
> 		 *
> 		 *   "noapic" as arguments,
> 		 *   "NOLIBC_TEST=syscall" as environment variables,
>                  *
> 		 * to avoid getting null test in this case, parsing
> 		 * environment variables at first.
> 		 */
> 		test = getenv("NOLIBC_TEST");
> 		if (!test)
> 			test = argv[1];
> 	} else {
> 		/* for normal nolibc-test program, prefer arguments */
> 		test = argv[1];
> 		if (!test)
> 			test = getenv("NOLIBC_TEST");
> 	}
> 

Test shows, when no NOLIBC_TEST environment variable passed to kernel cmdline,
it will still branch to this code:

    test = argv[1]; /* nopaic ... */

And therefore report the whole test is ignored and no test will be run:

    Ignoring unknown test name 'noapic'

So, we may still need to verify it like my originally proposed method, but
let's further verify the one from NOLIBC_TEST=, we should tune the code a
litle.

Thanks,
Zhangjin