pkg/bisect: rollback the guilty commit and test

[a-nogikh]

Suggested by Pengfei Xu. It was also mentioned during LPC'23.

After completing a bisection, we can rollback the guilty commit and test the kernel again to double-check the correctness of our conclusions.

For cause bisections (also applies for fix bisections with corresponding changes):

• Kernel still crashed.
  • Don't report the bisection result to the mailing lists.
• Cannot revert the changes. Kernel did not compile/boot after rollback.
  • Report the result, but add a note that syzbot could not verify the commit.
• Kernel did not crash.
  • Report the result with a note that rolling the commit back indeed resolves the issue.

Open question is at what base to revert the commit:

• Take the kernel at the identified revision and revert it.
  • Why it's better: the commit is 100% reversible.
• Take the current HEAD and try to revert it.
  • Why it's better: we get the most accurate results -- it can be that the same bug reappears several times and this would still pass the check above.

[dvyukov]

Take the kernel at the identified revision and revert it.

We already done this check during bisection.

Reverting at HEAD may give some additional info, but not sure if it will be useful. Kernel commits are not reverted often. We also don't know the tree maintainer is interested in, e.g. maintainer may consider reverting on sound tree, but we tested on usb. Or it may not revert cleanly, or trap on another bug, or the automatic revert may introduce a new bug.

[a-nogikh]

We already done this check during bisection.

You mean in detectNoopChange()? We compare kernel signatures, but we don't run the reproducer.

[dvyukov]

I mean the bisection itself. It's literally what bisection is checking: the bug happens on the resulting commit, but not on the previous one (which is effectively reverting the guilty commit).

[a-nogikh]

It's literally what bisection is checking: the bug happens on the resulting commit

Yes, but it's only true assuming that we were able to give a 100% correct answer at every bisection step, which is not always the case.

[dvyukov]

I assumed bisection cannot point to a commit w/o testing on that commit with positive results and testing on the previous one with negative result, regardless of results on all previous steps. And that's effectively testing a revert. In all other cases bisection points to multiple commits, which we don't report.

[a-nogikh]

Hmm, indeed! Thanks.

[a-nogikh]

I assumed bisection cannot point to a commit w/o testing on that commit with positive results and testing on the previous one with negative result, regardless of results on all previous steps.

On second thought, I'm actually not quite sure that it's working that way. Bisections on a DAG are of course more tricky, but say in case of an ordinary bisection on some linear sequence of items, we are assumed to have ensured the verdicts of the first and the last elements before starting the process. The bisection reduces that range on every step, still assuming that the range covers the element after which the verdict changes from true to false (or vice versa).

If we made a mistake on any single step, we may switch to a range of elements that already do not cover the element we're looking for. But the algorithm still thinks it's there, so e.g. if all of the following verdicts are false, then the result would just be the last element of the range. From the algorithm's perspective, it makes no sense to retest whether it's really the guilty commit -- it was the invariant it relied on all along the way.

So re-checking the result may actually make sense with our not 100% reliable verdicts.