Skip to content

Commit

Permalink
prog: fix selection of args eligible for squashing
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dvyukov committed Apr 15, 2024
1 parent 932a0a0 commit b9af7e6
Show file tree
Hide file tree
Showing 10 changed files with 218 additions and 115 deletions.
54 changes: 51 additions & 3 deletions pkg/compiler/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
20 changes: 5 additions & 15 deletions prog/any.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
63 changes: 49 additions & 14 deletions prog/any_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -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) {
Expand Down
9 changes: 6 additions & 3 deletions prog/encoding.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions prog/encoding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
},
})
}

Expand Down
5 changes: 1 addition & 4 deletions prog/size_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}})",
Expand Down
Loading

0 comments on commit b9af7e6

Please sign in to comment.