From fc067f05bce8156101e90f93fe87e702114b863f Mon Sep 17 00:00:00 2001
From: Aleksandr Nogikh <nogikh@google.com>
Date: Tue, 28 Mar 2023 15:03:50 +0200
Subject: pkg/subsystem: disambiguate subsystems by reproducers

There are some minor subsystems (e.g. PAGE CACHE in Linux) that are
parts of several big subsystems. At the same time, a reproducer can
clearly disambiguate such case.

If subsystems from reproducers and subsystems from guilty files
intersect, only proceed with the results of the intersection.
---
 pkg/subsystem/extractor.go        | 27 +++++++++++++++++++++++----
 pkg/subsystem/extractor_test.go   | 30 +++++++++++++++++++++++++++---
 pkg/subsystem/lists/linux_test.go | 19 +++++++++++++++++++
 3 files changed, 69 insertions(+), 7 deletions(-)

(limited to 'pkg/subsystem')

diff --git a/pkg/subsystem/extractor.go b/pkg/subsystem/extractor.go
index 1f635c7b3..338b7a4fc 100644
--- a/pkg/subsystem/extractor.go
+++ b/pkg/subsystem/extractor.go
@@ -39,19 +39,38 @@ func (e *Extractor) Extract(crashes []*Crash) []*Subsystem {
 	}
 
 	// If all reproducers hint at the same subsystem, take it as well.
-	reproSubsystems := map[*Subsystem]int{}
+	reproCounts := map[*Subsystem]int{}
+	fromRepro := []*Subsystem{}
 	for _, crash := range crashes {
 		if len(crash.SyzRepro) == 0 {
 			continue
 		}
 		for _, subsystem := range e.raw.FromProg(crash.SyzRepro) {
-			reproSubsystems[subsystem]++
-			if reproSubsystems[subsystem] == reproCount {
-				subsystems = append(subsystems, subsystem)
+			reproCounts[subsystem]++
+			if reproCounts[subsystem] == reproCount {
+				fromRepro = append(fromRepro, subsystem)
 			}
 		}
 	}
 
+	// It can be the case that guilty paths point to several subsystems, but the reproducer
+	// can clearly point to one of them.
+	if len(fromRepro) > 0 {
+		newSubsystems := []*Subsystem{}
+		for _, reproSubsystem := range fromRepro {
+			parents := reproSubsystem.ReachableParents()
+			for _, subsystem := range subsystems {
+				if _, ok := parents[subsystem]; ok {
+					newSubsystems = append(newSubsystems, reproSubsystem)
+					break
+				}
+			}
+		}
+		if len(newSubsystems) > 0 {
+			subsystems = newSubsystems
+		}
+	}
+
 	// If there are both parents and children, remove parents.
 	ignore := make(map[*Subsystem]struct{})
 	for _, entry := range subsystems {
diff --git a/pkg/subsystem/extractor_test.go b/pkg/subsystem/extractor_test.go
index a78509e0f..ec725a556 100644
--- a/pkg/subsystem/extractor_test.go
+++ b/pkg/subsystem/extractor_test.go
@@ -12,11 +12,13 @@ import (
 
 func TestExtractor(t *testing.T) {
 	// Objects used in tests.
-	fsPath := "fs/"
+	fsPath, mmFsPath := "fs/", "mm/fs.c"
 	extProg, nfsProg, extNfsProg := []byte("ext"), []byte("nfs"), []byte("ext nfs")
-	fs := &Subsystem{Name: "fs"}
+	all := &Subsystem{Name: "fs"}
+	fs := &Subsystem{Name: "fs", Parents: []*Subsystem{all}}
 	ext := &Subsystem{Name: "ext", Parents: []*Subsystem{fs}}
 	nfs := &Subsystem{Name: "nfs", Parents: []*Subsystem{fs}}
+	mm := &Subsystem{Name: "mm", Parents: []*Subsystem{all}}
 	// Tests themselves.
 	tests := []struct {
 		name    string
@@ -79,11 +81,33 @@ func TestExtractor(t *testing.T) {
 			},
 			want: []*Subsystem{ext},
 		},
+		{
+			name: `Reproducer supporting one of guilty paths`,
+			crashes: []*Crash{
+				// The guilty paths correspond both to mm and fs.
+				{
+					GuiltyPath: mmFsPath,
+				},
+				{
+					GuiltyPath: mmFsPath,
+				},
+				{
+					GuiltyPath: mmFsPath,
+				},
+				{
+					// But one reproducer points clearly to fs.
+					GuiltyPath: mmFsPath,
+					SyzRepro:   extProg,
+				},
+			},
+			want: []*Subsystem{ext},
+		},
 	}
 	extractor := &Extractor{
 		raw: &testRawExtractor{
 			perPath: map[string][]*Subsystem{
-				fsPath: {fs},
+				fsPath:   {fs},
+				mmFsPath: {mm, fs},
 			},
 			perProg: []progSubsystems{
 				{extProg, []*Subsystem{ext}},
diff --git a/pkg/subsystem/lists/linux_test.go b/pkg/subsystem/lists/linux_test.go
index 4f53948fd..8a3893f55 100644
--- a/pkg/subsystem/lists/linux_test.go
+++ b/pkg/subsystem/lists/linux_test.go
@@ -54,6 +54,25 @@ syz_mount_image$xfs(&(0x7f0000000100), &(0x7f0000009640)='./file2\x00', 0x200800
 syz_mount_image$ntfs3(&(0x7f000001f740), &(0x7f000001f780)='./file0\x00', 0x0, &(0x7f0000000200)=ANY=[@ANYBLOB="64697363==")
 mkdirat(0xffffffffffffff9c, &(0x7f0000000600)='./file0aaaaaaaaaaaaaaaaa\x00', 0x0)
 symlinkat(&(0x7f00000004c0)='./file0aaaaaaaaaa/file0\x00', 0xffffffffffffff9c, &(0x7f0000000280)='./file0aaaaaa/file0\x00')
+`),
+				},
+			},
+			expect: []string{"ntfs3"},
+		},
+		{
+			name: `a bug with page cache in guilty path`,
+			crashes: []*subsystem.Crash{
+				{
+					GuiltyPath: `mm/filemap.c`,
+				},
+				{
+					GuiltyPath: `mm/filemap.c`,
+					SyzRepro: []byte(`# https://syzkaller.appspot.com/bug?id=cdaf5ed409125df023889aefe50b4cc4a41c0973
+# See https://goo.gl/kgGztJ for information about syzkaller reproducers.
+#{"threaded":true,"repeat":true,"procs":6,"slowdown":1,"sandbox":"","sandbox_arg":0,"close_fds":false,"ieee802154":true,"sysctl":true,"tmpdir":true,"segv":true}
+syz_mount_image$ntfs3(&(0x7f000001f740), &(0x7f000001f780)='./file0\x00', 0x0, &(0x7f0000000200)=ANY=[@ANYBLOB="64697363==")
+mkdirat(0xffffffffffffff9c, &(0x7f0000000600)='./file0aaaaaaaaaaaaaaaaa\x00', 0x0)
+symlinkat(&(0x7f00000004c0)='./file0aaaaaaaaaa/file0\x00', 0xffffffffffffff9c, &(0x7f0000000280)='./file0aaaaaa/file0\x00')
 `),
 				},
 			},
-- 
cgit mrf-deployment