From 5e8ac358946b8898e30bb3d9642a1ce5e5b15a5a Mon Sep 17 00:00:00 2001
From: Hrutvik Kanabar <hrutvik@google.com>
Date: Fri, 30 Sep 2022 10:52:11 +0000
Subject: pkg/report: improve guilty file identification

Previously we would pick the first non-ignored file. Now instead,
continue searching the stack trace for more specific files.

A "more specific" file wrt the first non-ignored file has:
  - the same directory prefix
  - a deeper directory nesting
E.g. `fs/ntfs3/*.c` is "more specific" than "fs/*.c".
We search for the most specific file (i.e. the deepest nesting), and
take the first most specific if there are multiple files with the same
nesting.

This commit also adds three tests for this behaviour, taken from
recent `syzbot` bugs which identified the wrong file. Now the desired
file is identified.

Only one existing test shows different output with the new behaviour.

Updates #3393.
---
 pkg/report/linux.go | 44 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 8 deletions(-)

(limited to 'pkg/report/linux.go')

diff --git a/pkg/report/linux.go b/pkg/report/linux.go
index 984ad8627..57d2ad10a 100644
--- a/pkg/report/linux.go
+++ b/pkg/report/linux.go
@@ -703,27 +703,55 @@ func (ctx *linux) extractGuiltyFile(rep *Report) string {
 }
 
 func (ctx *linux) extractGuiltyFileImpl(report []byte) string {
-	first := ""
-	for s := bufio.NewScanner(bytes.NewReader(report)); s.Scan(); {
-		match := filenameRe.FindSubmatch(s.Bytes())
+	scanner := bufio.NewScanner(bytes.NewReader(report))
+
+	// Extract the first possible guilty file.
+	guilty := ""
+	for scanner.Scan() {
+		match := filenameRe.FindSubmatch(scanner.Bytes())
 		if match == nil {
 			continue
 		}
 		file := match[1]
-		if first == "" {
+		if guilty == "" {
 			// Avoid producing no guilty file at all, otherwise we mail the report to nobody.
 			// It's unclear if it's better to return the first one or the last one.
 			// So far the only test we have has only one file anyway.
-			first = string(file)
+			guilty = string(file)
 		}
 
-		if matchesAny(file, ctx.guiltyFileIgnores) || ctx.guiltyLineIgnore.Match(s.Bytes()) {
+		if matchesAny(file, ctx.guiltyFileIgnores) || ctx.guiltyLineIgnore.Match(scanner.Bytes()) {
 			continue
 		}
-		first = string(file)
+		guilty = string(file)
 		break
 	}
-	return filepath.Clean(first)
+	guilty = filepath.Clean(guilty)
+
+	// Search for deeper filepaths in the stack trace below the first possible guilty file.
+	deepestPath := filepath.Dir(guilty)
+	for scanner.Scan() {
+		match := filenameRe.FindSubmatch(scanner.Bytes())
+		if match == nil {
+			continue
+		}
+		file := match[1]
+		if matchesAny(file, ctx.guiltyFileIgnores) || ctx.guiltyLineIgnore.Match(scanner.Bytes()) {
+			continue
+		}
+		clean := filepath.Clean(string(file))
+
+		// Check if the new path has *both* the same directory prefix *and* a deeper suffix.
+		if strings.HasPrefix(clean, deepestPath) {
+			suffix := strings.TrimPrefix(clean, deepestPath)
+			if deeperPathRe.Match([]byte(suffix)) {
+				guilty = clean
+				deepestPath = filepath.Dir(guilty)
+			}
+		}
+	}
+
+	return guilty
 }
 
 func (ctx *linux) getMaintainers(file string) (vcs.Recipients, error) {
-- 
cgit mrf-deployment