Skip to content

Commit 7c1b377

Browse files
author
Ethan Graham
committed
prog: fix syz_kfuzztest_run allocation strategy
Previously, the generated KFuzzTest programs were reusing the address of the top-level input struct. A problem could arise when the encoded blob is large and overflows into another allocated region - this certainly happens in the case where the input struct points to some large char buffer, for example. While this wasn't directly a problem, it could lead to racy behavior when running KFuzzTest targets concurrently. To fix this, we now introduce an additional buffer parameter into syz_kfuzztest_run that is as big as the maximum accepted input size in the KFuzzTest kernel code. When this buffer is allocated, we ensure that we have some allocated space in the program that can hold the entire encoded input. This works in practice, but has not been tested with concurrent KFuzzTest executions yet.
1 parent cd97285 commit 7c1b377

File tree

5 files changed

+46
-26
lines changed

5 files changed

+46
-26
lines changed

executor/common_linux.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5865,7 +5865,7 @@ static long syz_pidfd_open(volatile long pid, volatile long flags)
58655865
#include <unistd.h>
58665866

58675867
static long syz_kfuzztest_run(volatile long test_name_ptr, volatile long input_data,
5868-
volatile long input_data_size)
5868+
volatile long input_data_size, volatile long buffer)
58695869
{
58705870
const char* test_name = (const char*)test_name_ptr;
58715871
if (!test_name) {
@@ -5876,6 +5876,10 @@ static long syz_kfuzztest_run(volatile long test_name_ptr, volatile long input_d
58765876
debug("syz_kfuzztest_run: input data was NULL\n");
58775877
return -1;
58785878
}
5879+
if (!buffer) {
5880+
debug("syz_kfuzztest_run: buffer was NULL\n");
5881+
return -1;
5882+
}
58795883

58805884
char buf[256];
58815885
int ret = snprintf(buf, sizeof(buf), "/sys/kernel/debug/kfuzztest/%s/input", test_name);
@@ -5890,7 +5894,7 @@ static long syz_kfuzztest_run(volatile long test_name_ptr, volatile long input_d
58905894
return -1;
58915895
}
58925896

5893-
ssize_t bytes_written = write(fd, (void*)input_data, (size_t)input_data_size);
5897+
ssize_t bytes_written = write(fd, (void*)buffer, (size_t)input_data_size);
58945898
if (bytes_written != input_data_size) {
58955899
debug("syz_kfuzztest_run: failed to write to %s, reason: %s\n", buf, strerror(errno));
58965900
close(fd);

pkg/kfuzztest/builder.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,8 @@ func syzFuncToSyzlang(s SyzFunc) string {
199199
fmt.Fprintf(&builder, "syz_kfuzztest_run$%s(", s.Name)
200200
fmt.Fprintf(&builder, "name ptr[in, string[\"%s\"]], ", s.Name)
201201
fmt.Fprintf(&builder, "data ptr[in, %s], ", typeName)
202-
builder.WriteString("len bytesize[data])")
202+
builder.WriteString("len bytesize[data], ")
203+
builder.WriteString("buf ptr[in, array[int8, 65536]]) ")
203204
// TODO:(ethangraham) The only other way I can think of getting this name
204205
// would involve using the "reflect" package and matching against the
205206
// KFuzzTest name, which isn't much better than hardcoding this.

prog/encodingexec.go

Lines changed: 34 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,9 @@ func (p *Prog) SerializeForExec() ([]byte, error) {
7575
w.write(uint64(len(p.Calls)))
7676
for _, c := range p.Calls {
7777
w.csumMap, w.csumUses = calcChecksumsCall(c)
78-
w.serializeCall(c)
78+
// TODO: if we propagate this error, something breaks and no coverage
79+
// is displayed to the dashboard or the logs.
80+
_ = w.serializeCall(c)
7981
}
8082
w.write(execInstrEOF)
8183
if len(w.buf) > ExecBufferSize {
@@ -87,13 +89,12 @@ func (p *Prog) SerializeForExec() ([]byte, error) {
8789
return w.buf, nil
8890
}
8991

90-
func (w *execContext) serializeCall(c *Call) {
92+
func (w *execContext) serializeCall(c *Call) error {
9193
// We introduce special serialization logic for kfuzztest targets, which
9294
// require special handling due to their use of relocation tables to copy
9395
// entire blobs of data into the kenrel.
9496
if c.Meta.Attrs.KFuzzTest {
95-
w.serializeKFuzzTestCall(c)
96-
return
97+
return w.serializeKFuzzTestCall(c)
9798
}
9899

99100
// Calculate arg offsets within structs.
@@ -125,18 +126,35 @@ func (w *execContext) serializeCall(c *Call) {
125126

126127
// Generate copyout instructions that persist interesting return values.
127128
w.writeCopyout(c)
129+
return nil
128130
}
129131

130132
// KFuzzTest targets require special handling due to their use of relocation
131133
// tables for serializing all data (including pointed-to data) into a
132134
// continuous blob that can be passed into the kernel.
133-
func (w *execContext) serializeKFuzzTestCall(c *Call) {
135+
func (w *execContext) serializeKFuzzTestCall(c *Call) error {
134136
if !c.Meta.Attrs.KFuzzTest {
135137
// This is a specialized function that shouldn't be called on anything
136138
// other than an instance of a syz_kfuzztest_run$* syscall
137139
panic("serializeKFuzzTestCall called on an invalid syscall")
138140
}
139141

142+
// Generate the final syscall instruction with the update arguments.
143+
kFuzzTestRunID, err := w.target.KFuzzTestRunID()
144+
if err != nil {
145+
panic(err)
146+
}
147+
// Ensures that we copy some arguments into the executor so that it doesn't
148+
// receive an incomplete program on failure.
149+
defer func() {
150+
w.write(uint64(kFuzzTestRunID))
151+
w.write(ExecNoCopyout)
152+
w.write(uint64(len(c.Args)))
153+
for _, arg := range c.Args {
154+
w.writeArg(arg)
155+
}
156+
}()
157+
140158
// Write the initial string argument (test name) normally.
141159
w.writeCopyin(&Call{Meta: c.Meta, Args: []Arg{c.Args[0]}})
142160

@@ -145,12 +163,17 @@ func (w *execContext) serializeKFuzzTestCall(c *Call) {
145163
// to the fuzzing driver with a relocation table.
146164
dataArg := c.Args[1].(*PointerArg)
147165
finalBlob := MarshallKFuzztestArg(dataArg.Res)
166+
if len(finalBlob) > int(KFuzzTestMaxInputSize) {
167+
return fmt.Errorf("encoded blob was too large")
168+
}
148169

149-
// Reuse the memory address that was pre-allocated for the original struct
150-
// argument. This avoids needing to hook into the memory allocation which
151-
// is done at a higher level than the serialization. This relies on the
152-
// original buffer being large enough
153-
blobAddress := w.target.PhysicalAddr(dataArg) - w.target.DataOffset
170+
// Use the buffer argument as data offset - this represents a buffer of
171+
// size 64KiB - the maximum input size that the KFuzzTest module accepts.
172+
bufferArg := c.Args[3].(*PointerArg)
173+
if bufferArg.Res == nil {
174+
return fmt.Errorf("buffer was nil")
175+
}
176+
blobAddress := w.target.PhysicalAddr(bufferArg) - w.target.DataOffset
154177

155178
// Write the entire marshalled blob as a raw byte array.
156179
w.write(execInstrCopyin)
@@ -164,18 +187,7 @@ func (w *execContext) serializeKFuzzTestCall(c *Call) {
164187
// of the struct argument passed into the pseudo-syscall.
165188
lenArg := c.Args[2].(*ConstArg)
166189
lenArg.Val = uint64(len(finalBlob))
167-
168-
// Generate the final syscall instruction with the update arguments.
169-
kFuzzTestRunID, err := w.target.KFuzzTestRunID()
170-
if err != nil {
171-
panic(err)
172-
}
173-
w.write(uint64(kFuzzTestRunID))
174-
w.write(ExecNoCopyout)
175-
w.write(uint64(len(c.Args)))
176-
for _, arg := range c.Args {
177-
w.writeArg(arg)
178-
}
190+
return nil
179191
}
180192

181193
type execContext struct {

prog/kfuzztest.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ const (
2222
// x86_64, so we hardcode it for now. A more robust solution would involve
2323
// reading this from the debugfs entry at boot before fuzzing begins.
2424
kFuzzTestMinalign uint64 = 8
25+
26+
// Maximum input size accepted by the KFuzzTest kernel module.
27+
KFuzzTestMaxInputSize uint64 = 64 << 10
2528
)
2629

2730
func kFuzzTestWritePrefix(buf *bytes.Buffer) {

sys/linux/kfuzztest.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
# Copyright 2025 syzkaller project authors. All rights reserved.
22
# Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file.
33

4-
syz_kfuzztest_run(name ptr[in, string], data ptr[in, array[int8]], len bytesize[data]) (kfuzz_test, no_generate)
4+
syz_kfuzztest_run(name ptr[in, string], data ptr[in, array[int8]], len bytesize[data], buf ptr[in, array[int8, 65536]]) (kfuzz_test, no_generate)

0 commit comments

Comments
 (0)