aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDmitry Vyukov <dvyukov@google.com>2024-04-15 10:45:34 +0200
committerDmitry Vyukov <dvyukov@google.com>2024-04-15 11:53:27 +0000
commitb9af7e61fec27039b365f0ce8ec03fbccd2c6d6a (patch)
tree77c11460683ff0a9798cd47aff7e4de107713f8c
parent932a0a0dc74e7f3f5486b27216f3698a74084673 (diff)
prog: fix selection of args eligible for squashing
This fixes 3 issues: 1. We intended to squash only 'in' pointer elems, but we looked at the pointer direction rather than elem direction. Since pointers themselves are always 'in' we squashed a number of types we didn't want to squash. 2. We can squash filenames, which can lead to generation of escaping filenames, e.g. fuzzer managed to create "/" filename for blockdev_filename as: mount(&(0x7f0000000000)=ANY=[@ANYBLOB='/'], ...) Don't squash filenames. 3. We analyzed a concrete arg to see if it contains something we don't want to squash (e.g. pointers). But the whole type can still contain unsupported things in inactive union options, or in 0-sized arrays. E.g. this happened in the mount case above. Analyze the whole type to check for unsupported things. This also moves most of the analysis to the compiler, so mutation will be a bit faster. This removes the following linux types from squashing. 1. These are not 'in': btrfs_ioctl_search_args_v2 btrfs_ioctl_space_args ethtool_cmd_u fscrypt_add_key_arg fscrypt_get_policy_ex_arg fsverity_digest hiddev_ioctl_string_arg hidraw_report_descriptor ifreq_dev_t[devnames, ptr[inout, ethtool_cmd_u]] ifreq_dev_t[ipv4_tunnel_names, ptr[inout, ip_tunnel_parm]] ifreq_dev_t["sit0", ptr[inout, ip_tunnel_prl]] io_uring_probe ip_tunnel_parm ip_tunnel_prl poll_cq_resp query_port_cmd query_qp_resp resize_cq_resp scsi_ioctl_probe_host_out_buffer sctp_assoc_ids sctp_authchunks sctp_getaddrs sctp_getaddrs_old 2. These contain pointers: binder_objects iovec[in, netlink_msg_route_sched] iovec[in, netlink_msg_route_sched_retired] msghdr_netlink[netlink_msg_route_sched] msghdr_netlink[netlink_msg_route_sched_retired] nvme_of_msg 3. These contain filenames: binfmt_script blockdev_filename netlink_msg_route_sched netlink_msg_route_sched_retired selinux_create_req
-rw-r--r--pkg/compiler/types.go54
-rw-r--r--prog/any.go20
-rw-r--r--prog/any_test.go63
-rw-r--r--prog/encoding.go9
-rw-r--r--prog/encoding_test.go5
-rw-r--r--prog/size_test.go5
-rw-r--r--prog/types.go153
-rw-r--r--sys/linux/init_test.go5
-rw-r--r--sys/test/any.txt9
-rw-r--r--sys/test/test.txt10
10 files changed, 218 insertions, 115 deletions
diff --git a/pkg/compiler/types.go b/pkg/compiler/types.go
index e77f80a3d..70a0bd565 100644
--- a/pkg/compiler/types.go
+++ b/pkg/compiler/types.go
@@ -210,14 +210,62 @@ var typePtr = &typeDesc{
base.TypeSize = 8
}
base.TypeAlign = getIntAlignment(comp, base)
+ elem := comp.genType(args[1], 0)
+ elemDir := genDir(args[0])
return &prog.PtrType{
- TypeCommon: base.TypeCommon,
- Elem: comp.genType(args[1], 0),
- ElemDir: genDir(args[0]),
+ TypeCommon: base.TypeCommon,
+ Elem: elem,
+ ElemDir: elemDir,
+ SquashableElem: isSquashableElem(elem, elemDir),
}
},
}
+func isSquashableElem(elem prog.Type, dir prog.Dir) bool {
+ if dir != prog.DirIn {
+ return false
+ }
+ // Check if the pointer element contains something that can be complex, and does not contain
+ // anything unsupported we don't want to sqaush. Prog package later checks at runtime
+ // if a concrete arg actually contains something complex. But we look at the whole type
+ // to understand if it contains anything unsupported b/c a union may contain e.g. a complex struct
+ // and a filename we don't want to squash, or an array may contain something unsupported,
+ // but has 0 size in a concrete argument.
+ complex, unsupported := false, false
+ prog.ForeachArgType(elem, func(t prog.Type, ctx *prog.TypeCtx) {
+ switch typ := t.(type) {
+ case *prog.StructType:
+ if typ.Varlen() {
+ complex = true
+ }
+ if typ.OverlayField != 0 {
+ // Squashing of structs with out_overlay is not supported.
+ // If we do it, we need to be careful to either squash out part as well,
+ // or remove any resources in the out part from the prog.
+ unsupported = true
+ }
+ case *prog.UnionType:
+ if typ.Varlen() && len(typ.Fields) > 5 {
+ complex = true
+ }
+ case *prog.PtrType:
+ // Squashing of pointers is not supported b/c if we do it
+ // we will pass random garbage as pointers.
+ unsupported = true
+ case *prog.BufferType:
+ switch typ.Kind {
+ case prog.BufferFilename, prog.BufferGlob, prog.BufferCompressed:
+ // Squashing file names may lead to unwanted escaping paths (e.g. "/"),
+ // squashing compressed buffers is not useful since we uncompress them ourselves
+ // (not the kernel).
+ unsupported = true
+ }
+ }
+ ctx.Stop = unsupported
+ })
+ return complex && !unsupported
+}
+
var typeVoid = &typeDesc{
Names: []string{"void"},
CantBeOpt: true,
diff --git a/prog/any.go b/prog/any.go
index 0a0fdddf5..ee898446b 100644
--- a/prog/any.go
+++ b/prog/any.go
@@ -80,38 +80,28 @@ func (p *Prog) complexPtrs() (res []complexPtr) {
}
func (target *Target) isComplexPtr(arg *PointerArg) bool {
- if arg.Res == nil || arg.Dir() != DirIn {
+ if arg.Res == nil || !arg.Type().(*PtrType).SquashableElem {
return false
}
if target.isAnyPtr(arg.Type()) {
return true
}
- complex, unsupported := false, false
+ complex := false
ForeachSubArg(arg.Res, func(a1 Arg, ctx *ArgCtx) {
switch typ := a1.Type().(type) {
case *StructType:
- if typ.OverlayField != 0 {
- // Squashing of structs with out_overlay is not supported.
- // If we do it, we need to be careful to either squash out part as well,
- // or remove any resources in the out part from the prog.
- unsupported = true
- ctx.Stop = true
- }
if typ.Varlen() {
complex = true
+ ctx.Stop = true
}
case *UnionType:
if typ.Varlen() && len(typ.Fields) > 5 {
complex = true
+ ctx.Stop = true
}
- case *PtrType:
- // Squashing of pointers is not supported b/c if we do it
- // we will pass random garbage as pointers.
- unsupported = true
- ctx.Stop = true
}
})
- return complex && !unsupported
+ return complex
}
func (target *Target) isAnyRes(name string) bool {
diff --git a/prog/any_test.go b/prog/any_test.go
index 0e2f5e0e4..333266622 100644
--- a/prog/any_test.go
+++ b/prog/any_test.go
@@ -8,36 +8,61 @@ import (
"sort"
"strings"
"testing"
+
+ "github.com/google/syzkaller/pkg/testutil"
)
func TestIsComplexPtr(t *testing.T) {
target, rs, _ := initRandomTargetTest(t, "linux", "amd64")
- ct := target.DefaultChoiceTable()
- iters := 10
- if testing.Short() {
- iters = 1
+ allComplex := make(map[string]bool)
+ ForeachType(target.Syscalls, func(t Type, ctx *TypeCtx) {
+ if ptr, ok := t.(*PtrType); ok && ptr.SquashableElem {
+ allComplex[ptr.Elem.String()] = true
+ }
+ })
+ var arr []string
+ for id := range allComplex {
+ arr = append(arr, id)
+ }
+ sort.Strings(arr)
+ // Log all complex types for manual inspection.
+ t.Log("complex types:\n" + strings.Join(arr, "\n"))
+ if testing.Short() || testutil.RaceEnabled {
+ return
}
+ // Compare with what we can generate at runtime.
+ // We cannot guarantee that we will generate 100% of complex types
+ // (some are no_generate, and the process is random), but we should
+ // generate at least 90% or there is something wrong.
+ ct := target.DefaultChoiceTable()
r := newRand(target, rs)
- compl := make(map[string]bool)
+ generatedComplex := make(map[string]bool)
for _, meta := range target.Syscalls {
if meta.Attrs.Disabled || meta.Attrs.NoGenerate {
continue
}
- for i := 0; i < iters; i++ {
+ for i := 0; i < 10; i++ {
s := newState(target, ct, nil)
calls := r.generateParticularCall(s, meta)
p := &Prog{Target: target, Calls: calls}
for _, arg := range p.complexPtrs() {
- compl[arg.arg.Res.Type().String()] = true
+ generatedComplex[arg.arg.Res.Type().String()] = true
}
}
}
- var arr []string
- for id := range compl {
- arr = append(arr, id)
+ for id := range generatedComplex {
+ if !allComplex[id] {
+ t.Errorf("generated complex %v that is not statically complex", id)
+ }
+ }
+ for id := range allComplex {
+ if !generatedComplex[id] {
+ t.Logf("did not generate %v", id)
+ }
+ }
+ if len(generatedComplex) < len(allComplex)*9/10 {
+ t.Errorf("generated too few complex types: %v/%v", len(generatedComplex), len(allComplex))
}
- sort.Strings(arr)
- t.Log("complex types:\n" + strings.Join(arr, "\n"))
}
func TestSquash(t *testing.T) {
@@ -48,8 +73,13 @@ func TestSquash(t *testing.T) {
squashed string // leave empty if the arg must not be squashed
}{
{
- `foo$any0(&(0x7f0000000000)={0x11, 0x11223344, 0x2233, 0x1122334455667788, {0x1, 0x7, 0x1, 0x1, 0x1bc, 0x4}, [{@res32=0x0, @i8=0x44, "aabb"}, {@res64=0x1, @i32=0x11223344, "1122334455667788"}, {@res8=0x2, @i8=0x55, "cc"}]})`,
- `foo$any0(&(0x7f0000000000)=ANY=[@ANYBLOB="1100000044332211223300000000000088776655443322117d00bc11", @ANYRES32=0x0, @ANYBLOB="0000000044aabb00", @ANYRES64=0x1, @ANYBLOB="443322111122334455667788", @ANYRES8=0x2, @ANYBLOB="0000000000000055cc0000"])`,
+ `foo$any_in(&(0x7f0000000000)={0x11, 0x11223344, 0x2233, 0x1122334455667788, {0x1, 0x7, 0x1, 0x1, 0x1bc, 0x4}, [{@res32=0x0, @i8=0x44, "aabb"}, {@res64=0x1, @i32=0x11223344, "1122334455667788"}, {@res8=0x2, @i8=0x55, "cc"}]})`,
+ `foo$any_in(&(0x7f0000000000)=ANY=[@ANYBLOB="1100000044332211223300000000000088776655443322117d00bc11", @ANYRES32=0x0, @ANYBLOB="0000000044aabb00", @ANYRES64=0x1, @ANYBLOB="443322111122334455667788", @ANYRES8=0x2, @ANYBLOB="0000000000000055cc0000"])`,
+ },
+ {
+ // Inout pointers must not be squashed.
+ `foo$any_inout(&(0x7f0000000000)={0x11, 0x11223344, 0x2233, 0x1122334455667788, {0x1, 0x7, 0x1, 0x1, 0x1bc, 0x4}, [{@res32=0x0, @i8=0x44, "aabb"}, {@res64=0x1, @i32=0x11223344, "1122334455667788"}, {@res8=0x2, @i8=0x55, "cc"}]})`,
+ ``,
},
{
// Squashing of structs with out_overlay is not supported yet
@@ -60,6 +90,11 @@ overlay_uses(0x0, 0x0, 0x0, r0)
`,
``,
},
+ {
+ // Unions with filenames must not be squashed.
+ `foo$any_filename(&AUTO)`,
+ ``,
+ },
}
for i, test := range tests {
t.Run(fmt.Sprint(i), func(t *testing.T) {
diff --git a/prog/encoding.go b/prog/encoding.go
index e925e7918..db32484a8 100644
--- a/prog/encoding.go
+++ b/prog/encoding.go
@@ -547,9 +547,10 @@ func (p *parser) parseArgRes(typ Type, dir Dir) (Arg, error) {
func (p *parser) parseArgAddr(typ Type, dir Dir) (Arg, error) {
var elem Type
elemDir := DirInOut
+ squashableElem := false
switch t1 := typ.(type) {
case *PtrType:
- elem, elemDir = t1.Elem, t1.ElemDir
+ elem, elemDir, squashableElem = t1.Elem, t1.ElemDir, t1.SquashableElem
case *VmaType:
default:
p.eatExcessive(true, "wrong addr arg %T", typ)
@@ -582,8 +583,10 @@ func (p *parser) parseArgAddr(typ Type, dir Dir) (Arg, error) {
p.Parse('N')
p.Parse('Y')
p.Parse('=')
- anyPtr := p.target.getAnyPtrType(typ.Size())
- typ, elem, elemDir = anyPtr, anyPtr.Elem, anyPtr.ElemDir
+ if squashableElem {
+ anyPtr := p.target.getAnyPtrType(typ.Size())
+ typ, elem, elemDir = anyPtr, anyPtr.Elem, anyPtr.ElemDir
+ }
}
var err error
inner, err = p.parseArg(elem, elemDir)
diff --git a/prog/encoding_test.go b/prog/encoding_test.go
index dd0dd8bfe..42f16036c 100644
--- a/prog/encoding_test.go
+++ b/prog/encoding_test.go
@@ -378,6 +378,11 @@ func TestSerializeDeserialize(t *testing.T) {
{
In: `serialize3(&(0x7f0000000000)="$eJwqrqzKTszJSS0CBAAA//8TyQPi")`,
},
+ {
+ In: `foo$any_filename(&(0x7f0000000000)=ANY=[@ANYBLOB='/'])`,
+ Out: `foo$any_filename(&(0x7f0000000000)=@complex={0x0, 0x0, 0x0, 0x0, {0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, []})`,
+ StrictErr: `wrong array arg`,
+ },
})
}
diff --git a/prog/size_test.go b/prog/size_test.go
index 7db32442f..5b49c437a 100644
--- a/prog/size_test.go
+++ b/prog/size_test.go
@@ -161,10 +161,7 @@ func TestAssignSize(t *testing.T) {
},
{
// If len target points into squashed argument, value is not updated.
- In: `
-test$length11(&(0x7f0000000000)=ANY=[@ANYBLOB="11"], 0x42)
-test$length30(&(0x7f0000000000)=ANY=[@ANYBLOB="11"], 0x42, &(0x7f0000000000)=0x43, 0x44)
-`,
+ In: `test$length_any(&(0x7f0000000000)=ANY=[@ANYBLOB="11"], 0x42)`,
},
{
In: "test$length32(&(0x7f0000000000)={[0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0], {0x0}, &(0x7f0000000040)={0x0}})",
diff --git a/prog/types.go b/prog/types.go
index 4e9d8455e..1707a0452 100644
--- a/prog/types.go
+++ b/prog/types.go
@@ -671,8 +671,9 @@ func (t *ArrayType) isDefaultArg(arg Arg) bool {
type PtrType struct {
TypeCommon
- Elem Type
- ElemDir Dir
+ Elem Type
+ ElemDir Dir
+ SquashableElem bool
}
func (t *PtrType) String() string {
@@ -775,97 +776,103 @@ type TypeCtx struct {
func ForeachType(syscalls []*Syscall, f func(t Type, ctx *TypeCtx)) {
for _, meta := range syscalls {
- foreachTypeImpl(meta, true, f)
+ foreachCallTypeImpl(meta, true, f)
}
}
func ForeachTypePost(syscalls []*Syscall, f func(t Type, ctx *TypeCtx)) {
for _, meta := range syscalls {
- foreachTypeImpl(meta, false, f)
+ foreachCallTypeImpl(meta, false, f)
}
}
func ForeachCallType(meta *Syscall, f func(t Type, ctx *TypeCtx)) {
- foreachTypeImpl(meta, true, f)
+ foreachCallTypeImpl(meta, true, f)
}
-func foreachTypeImpl(meta *Syscall, preorder bool, f func(t Type, ctx *TypeCtx)) {
- // We need seen to be keyed by the type, the direction, and the optionality
- // bit. Even if the first time we see a type it is optional or DirOut, it
- // could be required or DirIn on another path. So to ensure that the
- // information we report to the caller is correct, we need to visit both
- // occurrences.
- type seenKey struct {
- t Type
- d Dir
- o bool
- }
+// We need seen to be keyed by the type, the direction, and the optionality
+// bit. Even if the first time we see a type it is optional or DirOut, it
+// could be required or DirIn on another path. So to ensure that the
+// information we report to the caller is correct, we need to visit both
+// occurrences.
+type seenKey struct {
+ t Type
+ d Dir
+ o bool
+}
+
+func foreachCallTypeImpl(meta *Syscall, preorder bool, f func(t Type, ctx *TypeCtx)) {
// Note: we specifically don't create seen in ForeachType.
// It would prune recursion more (across syscalls), but lots of users need to
// visit each struct per-syscall (e.g. prio, used resources).
seen := make(map[seenKey]bool)
- var rec func(*Type, Dir, bool)
- rec = func(ptr *Type, dir Dir, optional bool) {
- if _, ref := (*ptr).(Ref); !ref {
- optional = optional || (*ptr).Optional()
+ for i := range meta.Args {
+ foreachTypeRec(f, meta, seen, &meta.Args[i].Type, DirIn, preorder, false)
+ }
+ if meta.Ret != nil {
+ foreachTypeRec(f, meta, seen, &meta.Ret, DirOut, preorder, false)
+ }
+}
+
+func ForeachArgType(typ Type, f func(t Type, ctx *TypeCtx)) {
+ foreachTypeRec(f, nil, make(map[seenKey]bool), &typ, DirIn, true, false)
+}
+
+func foreachTypeRec(cb func(t Type, ctx *TypeCtx), meta *Syscall, seen map[seenKey]bool, ptr *Type,
+ dir Dir, preorder, optional bool) {
+ if _, ref := (*ptr).(Ref); !ref {
+ optional = optional || (*ptr).Optional()
+ }
+ ctx := &TypeCtx{Meta: meta, Dir: dir, Ptr: ptr, Optional: optional}
+ if preorder {
+ cb(*ptr, ctx)
+ if ctx.Stop {
+ return
}
- ctx := &TypeCtx{Meta: meta, Dir: dir, Ptr: ptr, Optional: optional}
- if preorder {
- f(*ptr, ctx)
- if ctx.Stop {
- return
- }
+ }
+ switch a := (*ptr).(type) {
+ case *PtrType:
+ foreachTypeRec(cb, meta, seen, &a.Elem, a.ElemDir, preorder, optional)
+ case *ArrayType:
+ foreachTypeRec(cb, meta, seen, &a.Elem, dir, preorder, optional)
+ case *StructType:
+ key := seenKey{
+ t: a,
+ d: dir,
+ o: optional,
}
- switch a := (*ptr).(type) {
- case *PtrType:
- rec(&a.Elem, a.ElemDir, optional)
- case *ArrayType:
- rec(&a.Elem, dir, optional)
- case *StructType:
- key := seenKey{
- t: a,
- d: dir,
- o: optional,
- }
- if seen[key] {
- break // prune recursion via pointers to structs/unions
- }
- seen[key] = true
- for i, f := range a.Fields {
- rec(&a.Fields[i].Type, f.Dir(dir), optional)
- }
- case *UnionType:
- key := seenKey{
- t: a,
- d: dir,
- o: optional,
- }
- if seen[key] {
- break // prune recursion via pointers to structs/unions
- }
- seen[key] = true
- for i, f := range a.Fields {
- rec(&a.Fields[i].Type, f.Dir(dir), optional)
- }
- case *ResourceType, *BufferType, *VmaType, *LenType, *FlagsType,
- *ConstType, *IntType, *ProcType, *CsumType:
- case Ref:
- // This is only needed for pkg/compiler.
- default:
- panic("unknown type")
+ if seen[key] {
+ break // prune recursion via pointers to structs/unions
}
- if !preorder {
- f(*ptr, ctx)
- if ctx.Stop {
- panic("Stop is set in post-order iteration")
- }
+ seen[key] = true
+ for i, f := range a.Fields {
+ foreachTypeRec(cb, meta, seen, &a.Fields[i].Type, f.Dir(dir), preorder, optional)
}
+ case *UnionType:
+ key := seenKey{
+ t: a,
+ d: dir,
+ o: optional,
+ }
+ if seen[key] {
+ break // prune recursion via pointers to structs/unions
+ }
+ seen[key] = true
+ for i, f := range a.Fields {
+ foreachTypeRec(cb, meta, seen, &a.Fields[i].Type, f.Dir(dir), preorder, optional)
+ }
+ case *ResourceType, *BufferType, *VmaType, *LenType, *FlagsType,
+ *ConstType, *IntType, *ProcType, *CsumType:
+ case Ref:
+ // This is only needed for pkg/compiler.
+ default:
+ panic("unknown type")
}
- for i := range meta.Args {
- rec(&meta.Args[i].Type, DirIn, false)
- }
- if meta.Ret != nil {
- rec(&meta.Ret, DirOut, false)
+ if !preorder {
+ cb(*ptr, ctx)
+ if ctx.Stop {
+ panic("Stop is set in post-order iteration")
+ }
}
}
diff --git a/sys/linux/init_test.go b/sys/linux/init_test.go
index 8c4c07ba4..c273a2519 100644
--- a/sys/linux/init_test.go
+++ b/sys/linux/init_test.go
@@ -149,8 +149,9 @@ syz_open_dev$tty1(0xc, 0x4, 0x1)
Out: `sched_setattr(0x0, 0x0, 0x0)`,
},
{
- In: `sched_setattr(0x0, &(0x7f00000001c0)=ANY=[@ANYBLOB="1234567812345678"], 0x0)`,
- Out: `sched_setattr(0x0, &(0x7f00000001c0)=ANY=[@ANYBLOB='\x00\x00\x00\x00\x00\x00\x00\x00'], 0x0)`,
+ In: `sched_setattr(0x0, &(0x7f00000001c0)=ANY=[@ANYBLOB="1234567812345678"], 0x0)`,
+ Out: `sched_setattr(0x0, &(0x7f00000001c0)={0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, 0x0)`,
+ StrictErr: `wrong array arg`,
},
})
}
diff --git a/sys/test/any.txt b/sys/test/any.txt
index 5e0b0367a..b936c7651 100644
--- a/sys/test/any.txt
+++ b/sys/test/any.txt
@@ -7,7 +7,9 @@ resource anyres64[int64]
foo$anyres(a0 ptr[out, anyres8], a1 ptr[out, anyres32], a2 ptr[out, anyres64])
-foo$any0(a ptr[in, any0])
+foo$any_in(a ptr[in, any0])
+foo$any_inout(a ptr[inout, any0])
+foo$any_filename(a ptr[in, any_filename])
any0 {
f1 int8
@@ -43,3 +45,8 @@ anybitfields {
f5 int16:10
f6 int16:3
}
+
+any_filename [
+ complex any0
+ filename filename
+] [varlen]
diff --git a/sys/test/test.txt b/sys/test/test.txt
index c59b9d3d3..71fa79d93 100644
--- a/sys/test/test.txt
+++ b/sys/test/test.txt
@@ -529,6 +529,16 @@ len_of_cond_struct {
test$length35(a0 ptr[in, len_of_cond_struct])
+test$length_any(a0 ptr[in, syz_length_any_struct], a1 len[a0])
+
+syz_length_any_struct {
+ f0 int32
+ f1 int32
+ f2 int32
+ f3 int32
+ f4 array[int8]
+} [packed]
+
# Big endian
test$end0(a0 ptr[in, syz_end_int_struct])