linux-kernel - Re: [PATCH] kunit: tool: Fix bug in parsing test plan

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]

Message-ID: <CA+GJov5kKD+QX+kdEG3LQun=zo_aPwbA7=1NUw0dTLSGg-h=mw@mail.gmail.com>
Date: Thu, 6 Mar 2025 11:02:13 -0500
From: Rae Moar <rmoar@...gle.com>
To: Brendan Jackman <jackmanb@...gle.com>
Cc: David Gow <davidgow@...gle.com>, shuah@...nel.org, linux-kselftest@...r.kernel.org, 
	kunit-dev@...glegroups.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kunit: tool: Fix bug in parsing test plan

On Thu, Mar 6, 2025 at 7:26 AM Brendan Jackman <jackmanb@...gle.com> wrote:
>
> On Thu, 6 Mar 2025 at 10:00, David Gow <davidgow@...gle.com> wrote:
> >
> > On Thu, 6 Mar 2025 at 08:29, Rae Moar <rmoar@...gle.com> wrote:
> > >
> > > A bug was identified where the KTAP below caused an infinite loop:
> > >
> > >  TAP version 13
> > >  ok 4 test_case
> > >  1..4
> > >
> > > The infinite loop was caused by the parser not parsing a test plan
> > > if following a test result line.
> > >
> > > Fix bug to correctly parse test plan and add error if test plan is
> > > missing.
> > >
> > > Signed-off-by: Rae Moar <rmoar@...gle.com>
>
> Thanks for taking a look at this Rae! I tried to take a look myself
> but I could not really get a grip on the parsing logic in the time I
> had.
>
> > Thanks for looking into this: I don't think we want to unconditionally
> > error if there's no test plan, though. Pretty much no parameterised
> > tests include one -- it's not always possible to know how many tests
> > there'll be in advance -- so this triggers all of the time.
> >
> > Maybe we can only include an error if we find a test plan line after
> > an existing result, or something?
>
> Since I reported this bug, I discovered that the example above is in
> fact valid TAP:
>
> > The plan [...] must appear once, whether at the beginning or end of the output.
>
> From https://testanything.org/tap-version-13-specification.html

Hi!
This brings up an interesting question because the parser has been
mainly geared towards parsing KTAP
(https://docs.kernel.org/dev-tools/ktap.html) rather than TAP.
(Although we do try to have backwards compatibility with TAP v14
"Subtest" lines)

For example,

TAP version 13
1..1
  TAP version 13
  1..1
  ok 1 test_case
ok 1 test_suite

This would be accepted by the parser without error because it is valid
KTAP even though it is not valid TAP v13.

The scenario above that caused the infinite loop would be incorrect
KTAP (which requires the test plan to follow a version line) but
correct TAP v13. So do we accept it without error? Ideally, we would
parse based on the version given in the version line.

Just an interesting thought. Either way, I will remove the error for
now as our parameterized tests don't properly produce a test plan,
which causes errors.

Thanks!
-Rae