From e439d37e580e2d66e98f2b987f8ba0c295231439 Mon Sep 17 00:00:00 2001 From: Timo Beckers Date: Fri, 20 Dec 2024 01:58:41 +0100 Subject: [PATCH] prog: fix shadowing log size variable in verifier log retry loop When ProgramOptions.LogSize was removed in 0.16, the tests weren't updated to exercise the retry loop, since a minimum log size was chosen that was larger than what the test program could generate. With the addition of LogSizeStart, this notoriously fragile code broke when logSize was again tracked as a separate variable, while being accidentally shadowed within the scope of the for loop. This resulted in an endless loop on kernels without the LogTrueSize field. Remove the shadowing and fix the tests. Signed-off-by: Timo Beckers --- prog.go | 2 +- prog_test.go | 21 +++++++++++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/prog.go b/prog.go index 3767f2873..4f3ce43bf 100644 --- a/prog.go +++ b/prog.go @@ -451,7 +451,7 @@ func newProgramWithOptions(spec *ProgramSpec, opts ProgramOptions) (*Program, er // Make an educated guess how large the buffer should be by multiplying. // Ensure the size doesn't overflow. const factor = 2 - logSize := internal.Between(logSize, minVerifierLogSize, maxVerifierLogSize/factor) + logSize = internal.Between(logSize, minVerifierLogSize, maxVerifierLogSize/factor) logSize *= factor if attr.LogTrueSize != 0 { diff --git a/prog_test.go b/prog_test.go index 5fcddc065..3b17fd273 100644 --- a/prog_test.go +++ b/prog_test.go @@ -469,12 +469,16 @@ func TestProgramVerifierLog(t *testing.T) { var ve *internal.VerifierError qt.Assert(t, qt.ErrorAs(err, &ve)) + + loglen := len(fmt.Sprintf("%+v", ve)) + qt.Assert(t, qt.IsTrue(loglen > minVerifierLogSize), + qt.Commentf("Log buffer didn't grow past minimum, got %d bytes", loglen)) } // Generate a base program of sufficient size whose verifier log does not fit - // a 128-byte buffer. This should always result in ENOSPC. + // in the minimum buffer size. Stay under 4096 insn limit of older kernels. var base asm.Instructions - for i := 0; i < 32; i++ { + for i := 0; i < 4093; i++ { base = append(base, asm.Mov.Reg(asm.R0, asm.R1)) } @@ -493,8 +497,7 @@ func TestProgramVerifierLog(t *testing.T) { Instructions: invalid, } - // Set an undersized log buffer without explicitly requesting a verifier log - // for an invalid program. + // Don't explicitly request a verifier log for an invalid program. _, err := NewProgramWithOptions(spec, ProgramOptions{}) check(t, err) @@ -528,6 +531,16 @@ func TestProgramVerifierLog(t *testing.T) { LogLevel: LogLevelInstruction, }) qt.Assert(t, qt.IsNil(err)) + qt.Assert(t, qt.IsTrue(len(prog.VerifierLog) > minVerifierLogSize)) + prog.Close() + + // Repeat the previous test with a larger starting buffer size. + prog, err = NewProgramWithOptions(spec, ProgramOptions{ + LogLevel: LogLevelInstruction, + LogSizeStart: minVerifierLogSize * 2, + }) + qt.Assert(t, qt.IsNil(err)) + qt.Assert(t, qt.IsTrue(len(prog.VerifierLog) > minVerifierLogSize)) prog.Close() }