From 80d43738f1e4c648ccfc4599e17dc8ba455fe1ea Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Sat, 14 Mar 2020 16:42:00 +0100 Subject: prog: rename target.SanitizeCall to Neutralize We will need a wrapper for target.SanitizeCall that will do more than just calling the target-provided function. To avoid confusion and potential mistakes, give the target function and prog function different names. Prog package will continue to call this "sanitize", which will include target's "neutralize" + more. Also refactor API a bit: we need a helper function that sanitizes the whole program because that's needed most of the time. Fixes #477 Fixes #502 --- prog/encoding.go | 4 ++-- prog/generation.go | 1 + prog/hints.go | 7 ++++++- prog/minimization.go | 4 +--- prog/mutation.go | 4 +--- prog/prog.go | 15 +++++++++++++++ prog/prog_test.go | 4 +--- prog/rand.go | 6 +----- prog/target.go | 14 ++++++++++---- sys/akaros/init.go | 4 ++-- sys/freebsd/init.go | 6 +++--- sys/linux/init.go | 18 +++++++++--------- sys/linux/init_iptables.go | 2 +- sys/linux/init_test.go | 2 +- sys/netbsd/init.go | 6 +++--- sys/openbsd/init.go | 10 +++++----- sys/openbsd/init_test.go | 2 +- sys/targets/common.go | 8 ++++---- sys/trusty/init.go | 8 -------- 19 files changed, 67 insertions(+), 58 deletions(-) diff --git a/prog/encoding.go b/prog/encoding.go index 6bded49d5..9a06c6d4b 100644 --- a/prog/encoding.go +++ b/prog/encoding.go @@ -227,8 +227,8 @@ func (target *Target) Deserialize(data []byte, mode DeserializeMode) (*Prog, err if p.autos != nil { p.fixupAutos(prog) } - for _, c := range prog.Calls { - target.SanitizeCall(c) + if err := prog.sanitize(mode == NonStrict); err != nil { + return nil, err } return prog, nil } diff --git a/prog/generation.go b/prog/generation.go index 1ceda4820..037801b75 100644 --- a/prog/generation.go +++ b/prog/generation.go @@ -29,6 +29,7 @@ func (target *Target) Generate(rs rand.Source, ncalls int, ct *ChoiceTable) *Pro for len(p.Calls) > ncalls { p.removeCall(ncalls - 1) } + p.sanitizeFix() p.debugValidate() return p } diff --git a/prog/hints.go b/prog/hints.go index f7f9dc487..9a5675b1b 100644 --- a/prog/hints.go +++ b/prog/hints.go @@ -66,7 +66,12 @@ func (p *Prog) MutateWithHints(callIndex int, comps CompMap, exec func(p *Prog)) p = p.Clone() c := p.Calls[callIndex] execValidate := func() { - p.Target.SanitizeCall(c) + // Don't try to fix the candidate program. + // Assuming the original call was sanitized, we've got a bad call + // as the result of hint substitution, so just throw it away. + if p.Target.sanitize(c, false) != nil { + return + } p.debugValidate() exec(p) } diff --git a/prog/minimization.go b/prog/minimization.go index 9a71dd067..93a986556 100644 --- a/prog/minimization.go +++ b/prog/minimization.go @@ -13,9 +13,7 @@ import ( // the simplification attempt is committed and the process continues. func Minimize(p0 *Prog, callIndex0 int, crash bool, pred0 func(*Prog, int) bool) (*Prog, int) { pred := func(p *Prog, callIndex int) bool { - for _, call := range p.Calls { - p.Target.SanitizeCall(call) - } + p.sanitizeFix() p.debugValidate() return pred0(p, callIndex) } diff --git a/prog/mutation.go b/prog/mutation.go index 62acba586..5a1819569 100644 --- a/prog/mutation.go +++ b/prog/mutation.go @@ -49,9 +49,7 @@ func (p *Prog) Mutate(rs rand.Source, ncalls int, ct *ChoiceTable, corpus []*Pro ok = ctx.removeCall() } } - for _, c := range p.Calls { - p.Target.SanitizeCall(c) - } + p.sanitizeFix() p.debugValidate() if got := len(p.Calls); got < 1 || got > ncalls { panic(fmt.Sprintf("bad number of calls after mutation: %v, want [1, %v]", got, ncalls)) diff --git a/prog/prog.go b/prog/prog.go index 575f0da9b..1600c0a28 100644 --- a/prog/prog.go +++ b/prog/prog.go @@ -387,3 +387,18 @@ func (p *Prog) removeCall(idx int) { copy(p.Calls[idx:], p.Calls[idx+1:]) p.Calls = p.Calls[:len(p.Calls)-1] } + +func (p *Prog) sanitizeFix() { + if err := p.sanitize(true); err != nil { + panic(err) + } +} + +func (p *Prog) sanitize(fix bool) error { + for _, c := range p.Calls { + if err := p.Target.sanitize(c, fix); err != nil { + return err + } + } + return nil +} diff --git a/prog/prog_test.go b/prog/prog_test.go index 3a3577328..16a54e2d6 100644 --- a/prog/prog_test.go +++ b/prog/prog_test.go @@ -441,9 +441,7 @@ func TestSanitizeRandom(t *testing.T) { for i := 0; i < iters; i++ { p := target.Generate(rs, 10, nil) s0 := string(p.Serialize()) - for _, c := range p.Calls { - target.SanitizeCall(c) - } + p.sanitizeFix() s1 := string(p.Serialize()) if s0 != s1 { t.Fatalf("non-sanitized program or non-idempotent sanitize\nwas: %v\ngot: %v", s0, s1) diff --git a/prog/rand.go b/prog/rand.go index bf6d66e9a..8583fbdcb 100644 --- a/prog/rand.go +++ b/prog/rand.go @@ -561,11 +561,7 @@ func (r *randGen) generateParticularCall(s *state, meta *Syscall) (calls []*Call } c.Args, calls = r.generateArgs(s, meta.Args) r.target.assignSizesCall(c) - calls = append(calls, c) - for _, c1 := range calls { - r.target.SanitizeCall(c1) - } - return calls + return append(calls, c) } // GenerateAllSyzProg generates a program that contains all pseudo syz_ calls for testing. diff --git a/prog/target.go b/prog/target.go index 69398a54d..b19645ea2 100644 --- a/prog/target.go +++ b/prog/target.go @@ -28,8 +28,9 @@ type Target struct { // MakeMmap creates call that maps [addr, addr+size) memory range. MakeMmap func(addr, size uint64) *Call - // SanitizeCall neutralizes harmful calls. - SanitizeCall func(c *Call) + // Neutralize neutralizes harmful calls by transforming them into non-harmful ones + // (e.g. an ioctl that turns off console output is turned into ioctl that turns on output). + Neutralize func(c *Call) // AnnotateCall annotates a syscall invocation in C reproducers. // The returned string will be placed inside a comment except for the @@ -113,7 +114,7 @@ func AllTargets() []*Target { } func (target *Target) lazyInit() { - target.SanitizeCall = func(c *Call) {} + target.Neutralize = func(c *Call) {} target.AnnotateCall = func(c ExecCall) string { return "" } target.initTarget() target.initArch(target) @@ -165,6 +166,11 @@ func (target *Target) GetConst(name string) uint64 { return v } +func (target *Target) sanitize(c *Call, fix bool) error { + target.Neutralize(c) + return nil +} + func RestoreLinks(syscalls []*Syscall, resources []*ResourceDesc, structs []*KeyedStruct) { restoreLinks(syscalls, resources, structs) } @@ -277,7 +283,7 @@ func MakeProgGen(target *Target) *Builder { func (pg *Builder) Append(c *Call) error { pg.target.assignSizesCall(c) - pg.target.SanitizeCall(c) + pg.target.sanitize(c, true) pg.p.Calls = append(pg.p.Calls, c) return nil } diff --git a/sys/akaros/init.go b/sys/akaros/init.go index ede55dfe9..5458f1462 100644 --- a/sys/akaros/init.go +++ b/sys/akaros/init.go @@ -17,10 +17,10 @@ func InitTarget(target *prog.Target) { MAP_FIXED: target.GetConst("MAP_FIXED"), } target.MakeMmap = targets.MakePosixMmap(target) - target.SanitizeCall = arch.sanitizeCall + target.Neutralize = arch.Neutralize } -func (arch *arch) sanitizeCall(c *prog.Call) { +func (arch *arch) Neutralize(c *prog.Call) { switch c.Meta.CallName { case "mmap": c.Args[3].(*prog.ConstArg).Val |= arch.MAP_FIXED diff --git a/sys/freebsd/init.go b/sys/freebsd/init.go index ab2224715..ecb976f91 100644 --- a/sys/freebsd/init.go +++ b/sys/freebsd/init.go @@ -10,13 +10,13 @@ import ( func InitTarget(target *prog.Target) { arch := &arch{ - unix: targets.MakeUnixSanitizer(target), + unix: targets.MakeUnixNeutralizer(target), } target.MakeMmap = targets.MakePosixMmap(target) - target.SanitizeCall = arch.unix.SanitizeCall + target.Neutralize = arch.unix.Neutralize } type arch struct { - unix *targets.UnixSanitizer + unix *targets.UnixNeutralizer } diff --git a/sys/linux/init.go b/sys/linux/init.go index cf54ffcd0..e1474995b 100644 --- a/sys/linux/init.go +++ b/sys/linux/init.go @@ -12,7 +12,7 @@ import ( func InitTarget(target *prog.Target) { arch := &arch{ - unix: targets.MakeUnixSanitizer(target), + unix: targets.MakeUnixNeutralizer(target), clockGettimeSyscall: target.SyscallMap["clock_gettime"], MREMAP_MAYMOVE: target.GetConst("MREMAP_MAYMOVE"), MREMAP_FIXED: target.GetConst("MREMAP_FIXED"), @@ -49,7 +49,7 @@ func InitTarget(target *prog.Target) { } target.MakeMmap = targets.MakePosixMmap(target) - target.SanitizeCall = arch.sanitizeCall + target.Neutralize = arch.neutralize target.SpecialTypes = map[string]func(g *prog.Gen, typ prog.Type, old prog.Arg) ( prog.Arg, []*prog.Call){ "timespec": arch.generateTimespec, @@ -118,7 +118,7 @@ var ( ) type arch struct { - unix *targets.UnixSanitizer + unix *targets.UnixNeutralizer clockGettimeSyscall *prog.Syscall @@ -155,8 +155,8 @@ type arch struct { TIOCGSERIAL uint64 } -func (arch *arch) sanitizeCall(c *prog.Call) { - arch.unix.SanitizeCall(c) +func (arch *arch) neutralize(c *prog.Call) { + arch.unix.Neutralize(c) switch c.Meta.CallName { case "mremap": // Add MREMAP_FIXED flag, otherwise it produces non-deterministic results. @@ -175,7 +175,7 @@ func (arch *arch) sanitizeCall(c *prog.Call) { cmd.Val = arch.SYSLOG_ACTION_SIZE_UNREAD } case "ioctl": - arch.sanitizeIoctl(c) + arch.neutralizeIoctl(c) case "fanotify_mark": // FAN_*_PERM require the program to reply to open requests. // If that does not happen, the program will hang in an unkillable state forever. @@ -222,7 +222,7 @@ func (arch *arch) sanitizeCall(c *prog.Call) { switch c.Meta.Name { case "setsockopt$EBT_SO_SET_ENTRIES": - arch.sanitizeEbtables(c) + arch.neutralizeEbtables(c) } } @@ -241,7 +241,7 @@ func enforceIntArg(a prog.Arg) { } } -func (arch *arch) sanitizeIoctl(c *prog.Call) { +func (arch *arch) neutralizeIoctl(c *prog.Call) { cmd := c.Args[1].(*prog.ConstArg) switch uint64(uint32(cmd.Val)) { case arch.FIFREEZE: @@ -270,7 +270,7 @@ func (arch *arch) sanitizeIoctl(c *prog.Call) { // https://groups.google.com/g/syzkaller-bugs/c/1rVENJf9P4U/m/QtGpapRxAgAJ // https://syzkaller.appspot.com/bug?extid=f4f1e871965064ae689e // TODO: TIOCSSERIAL does some other things that are not dangerous - // and would be nice to test, if/when we can sanitize based on sandbox value + // and would be nice to test, if/when we can neutralize based on sandbox value // we could prohibit it only under sandbox=none. cmd.Val = arch.TIOCGSERIAL } diff --git a/sys/linux/init_iptables.go b/sys/linux/init_iptables.go index 2a49bffe4..a1adf3fb0 100644 --- a/sys/linux/init_iptables.go +++ b/sys/linux/init_iptables.go @@ -164,7 +164,7 @@ func (arch *arch) generateEbtables(g *prog.Gen, typ prog.Type, old prog.Arg) ( return } -func (arch *arch) sanitizeEbtables(c *prog.Call) { +func (arch *arch) neutralizeEbtables(c *prog.Call) { // This is very hacky... just as netfilter interfaces. // setsockopt's len argument must be equal to size of ebt_replace + entries size. lenArg := c.Args[4].(*prog.ConstArg) diff --git a/sys/linux/init_test.go b/sys/linux/init_test.go index ae1a49698..fb3068295 100644 --- a/sys/linux/init_test.go +++ b/sys/linux/init_test.go @@ -10,7 +10,7 @@ import ( _ "github.com/google/syzkaller/sys/linux/gen" ) -func TestSanitize(t *testing.T) { +func TestNeutralize(t *testing.T) { prog.TestDeserializeHelper(t, "linux", "amd64", nil, []prog.DeserializeTest{ { In: `syslog(0x10000000006, 0x0, 0x0)`, diff --git a/sys/netbsd/init.go b/sys/netbsd/init.go index c039f4d43..6591e67b7 100644 --- a/sys/netbsd/init.go +++ b/sys/netbsd/init.go @@ -10,13 +10,13 @@ import ( func InitTarget(target *prog.Target) { arch := &arch{ - unix: targets.MakeUnixSanitizer(target), + unix: targets.MakeUnixNeutralizer(target), } target.MakeMmap = targets.MakePosixMmap(target) - target.SanitizeCall = arch.unix.SanitizeCall + target.Neutralize = arch.unix.Neutralize } type arch struct { - unix *targets.UnixSanitizer + unix *targets.UnixNeutralizer } diff --git a/sys/openbsd/init.go b/sys/openbsd/init.go index e7c5d08f6..e27ba8d60 100644 --- a/sys/openbsd/init.go +++ b/sys/openbsd/init.go @@ -13,19 +13,19 @@ import ( func InitTarget(target *prog.Target) { arch := &arch{ - unix: targets.MakeUnixSanitizer(target), + unix: targets.MakeUnixNeutralizer(target), DIOCKILLSTATES: target.GetConst("DIOCKILLSTATES"), S_IFMT: target.GetConst("S_IFMT"), S_IFCHR: target.GetConst("S_IFCHR"), } target.MakeMmap = targets.MakePosixMmap(target) - target.SanitizeCall = arch.SanitizeCall + target.Neutralize = arch.neutralize target.AnnotateCall = arch.annotateCall } type arch struct { - unix *targets.UnixSanitizer + unix *targets.UnixNeutralizer DIOCKILLSTATES uint64 S_IFMT uint64 S_IFCHR uint64 @@ -72,7 +72,7 @@ func isKcovFd(dev uint64) bool { return major == devFdMajor && minor >= kcovFdMinorMin && minor < kcovFdMinorMax } -func (arch *arch) SanitizeCall(c *prog.Call) { +func (arch *arch) neutralize(c *prog.Call) { argStart := 1 switch c.Meta.CallName { case "chflagsat": @@ -169,7 +169,7 @@ func (arch *arch) SanitizeCall(c *prog.Call) { } } default: - arch.unix.SanitizeCall(c) + arch.unix.Neutralize(c) } } diff --git a/sys/openbsd/init_test.go b/sys/openbsd/init_test.go index 6895ce3c9..30937bade 100644 --- a/sys/openbsd/init_test.go +++ b/sys/openbsd/init_test.go @@ -10,7 +10,7 @@ import ( _ "github.com/google/syzkaller/sys/openbsd/gen" ) -func TestSanitizeCall(t *testing.T) { +func TestNeutralize(t *testing.T) { prog.TestDeserializeHelper(t, "openbsd", "amd64", nil, []prog.DeserializeTest{ { In: `chflagsat(0x0, 0x0, 0x60004, 0x0)`, diff --git a/sys/targets/common.go b/sys/targets/common.go index 0096bcf75..a5a4838c2 100644 --- a/sys/targets/common.go +++ b/sys/targets/common.go @@ -51,7 +51,7 @@ func MakeSyzMmap(target *prog.Target) func(addr, size uint64) *prog.Call { } } -type UnixSanitizer struct { +type UnixNeutralizer struct { MAP_FIXED uint64 S_IFREG uint64 S_IFCHR uint64 @@ -60,8 +60,8 @@ type UnixSanitizer struct { S_IFSOCK uint64 } -func MakeUnixSanitizer(target *prog.Target) *UnixSanitizer { - return &UnixSanitizer{ +func MakeUnixNeutralizer(target *prog.Target) *UnixNeutralizer { + return &UnixNeutralizer{ MAP_FIXED: target.GetConst("MAP_FIXED"), S_IFREG: target.GetConst("S_IFREG"), S_IFCHR: target.GetConst("S_IFCHR"), @@ -71,7 +71,7 @@ func MakeUnixSanitizer(target *prog.Target) *UnixSanitizer { } } -func (arch *UnixSanitizer) SanitizeCall(c *prog.Call) { +func (arch *UnixNeutralizer) Neutralize(c *prog.Call) { switch c.Meta.CallName { case "mmap": // Add MAP_FIXED flag, otherwise it produces non-deterministic results. diff --git a/sys/trusty/init.go b/sys/trusty/init.go index 5f5335566..d437b5d3f 100644 --- a/sys/trusty/init.go +++ b/sys/trusty/init.go @@ -8,14 +8,6 @@ import ( "github.com/google/syzkaller/sys/targets" ) -type arch struct { -} - func InitTarget(target *prog.Target) { - arch := &arch{} target.MakeMmap = targets.MakeSyzMmap(target) - target.SanitizeCall = arch.sanitizeCall -} - -func (arch *arch) sanitizeCall(c *prog.Call) { } -- cgit mrf-deployment