From 18e8a98274ea4160a7f7d4a0b207f31a8726a791 Mon Sep 17 00:00:00 2001
From: Aleksandr Nogikh <nogikh@google.com>
Date: Tue, 4 Apr 2023 11:39:00 +0200
Subject: pkg/subsystem: update the bug subsystem extraction logic

There was a small bug and, as a result, subsystems from reproducers
always superceded all other ones. That was not the desired side-effect.

Fix the logic and add a test to linux_test.go.
---
 pkg/subsystem/extractor.go        | 36 +++++++++++++++++++++++-------------
 pkg/subsystem/lists/linux_test.go | 19 +++++++++++++++++++
 2 files changed, 42 insertions(+), 13 deletions(-)

(limited to 'pkg')

diff --git a/pkg/subsystem/extractor.go b/pkg/subsystem/extractor.go
index 338b7a4fc..996560a31 100644
--- a/pkg/subsystem/extractor.go
+++ b/pkg/subsystem/extractor.go
@@ -56,10 +56,13 @@ func (e *Extractor) Extract(crashes []*Crash) []*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 {
+		// If we do not exclude parents, there'll always be a root subsystem in there
+		// and, as a result, subsystems reproducers will always replace all other ones.
+		withoutParents := removeParents(subsystems)
 		newSubsystems := []*Subsystem{}
 		for _, reproSubsystem := range fromRepro {
 			parents := reproSubsystem.ReachableParents()
-			for _, subsystem := range subsystems {
+			for _, subsystem := range withoutParents {
 				if _, ok := parents[subsystem]; ok {
 					newSubsystems = append(newSubsystems, reproSubsystem)
 					break
@@ -71,21 +74,10 @@ func (e *Extractor) Extract(crashes []*Crash) []*Subsystem {
 		}
 	}
 
-	// If there are both parents and children, remove parents.
-	ignore := make(map[*Subsystem]struct{})
-	for _, entry := range subsystems {
-		for p := range entry.ReachableParents() {
-			ignore[p] = struct{}{}
-		}
-	}
-
 	// And calculate counts.
 	counts := make(map[*Subsystem]int)
 	maxCount := 0
-	for _, entry := range subsystems {
-		if _, ok := ignore[entry]; ok {
-			continue
-		}
+	for _, entry := range removeParents(append(subsystems, fromRepro...)) {
 		counts[entry]++
 		if counts[entry] > maxCount {
 			maxCount = counts[entry]
@@ -102,3 +94,21 @@ func (e *Extractor) Extract(crashes []*Crash) []*Subsystem {
 	}
 	return ret
 }
+
+func removeParents(subsystems []*Subsystem) []*Subsystem {
+	// If there are both parents and children, remove parents.
+	ignore := make(map[*Subsystem]struct{})
+	for _, entry := range subsystems {
+		for p := range entry.ReachableParents() {
+			ignore[p] = struct{}{}
+		}
+	}
+	var ret []*Subsystem
+	for _, entry := range subsystems {
+		if _, ok := ignore[entry]; ok {
+			continue
+		}
+		ret = append(ret, entry)
+	}
+	return ret
+}
diff --git a/pkg/subsystem/lists/linux_test.go b/pkg/subsystem/lists/linux_test.go
index 8a3893f55..6107ff68f 100644
--- a/pkg/subsystem/lists/linux_test.go
+++ b/pkg/subsystem/lists/linux_test.go
@@ -76,8 +76,27 @@ symlinkat(&(0x7f00000004c0)='./file0aaaaaaaaaa/file0\x00', 0xffffffffffffff9c, &
 `),
 				},
 			},
+			// There should be no mm in the subsystems.
 			expect: []string{"ntfs3"},
 		},
+		{
+			name: "dri bug with usb call",
+			crashes: []*subsystem.Crash{
+				{
+					GuiltyPath: `drivers/gpu/drm/udl/udl_drv.c`,
+				},
+				{
+					GuiltyPath: `drivers/gpu/drm/udl/udl_drv.c`,
+					SyzRepro: []byte(`
+# https://syzkaller.appspot.com/bug?id=56e9aec9bc3b5378c9b231a3f4b3329cf9f80990
+# See https://goo.gl/kgGztJ for information about syzkaller reproducers.
+#{"procs":1,"slowdown":1,"sandbox":"none","sandbox_arg":0,"tun":true,"netdev":true,"close_fds":true}
+syz_usb_connect(0x0, 0x24, &(0x7f0000000140)=ANY=[@ANYBLOB="12010000abbe6740e9174e8b089c000000010902120001000000000904000800ff"], 0x0)
+`),
+				},
+			},
+			expect: []string{"dri"},
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-- 
cgit mrf-deployment