kexec-tools: fix various bugs in get_crash_notes_per_cpu

My colleague Magnus Damm pointed out the following problem

* errno is returned if the sysfs file could not be opened
  and sysfs is mounted (according to the currently bogus stat check).
  But errno may be any value, and is typically > 0, but the callers check
  for < 0. So basically, the callers will think that
  get_crash_notes_per_cpu() has suceeded.  Just return -1 instead, which
  satifies the needs of the callers..

In the course of looking at this I noticed the following problems

* The code testing stat() != 0 for the the non-existence of a file.
  The correc check, notation asside, is !stat() && errno == ENOENT

* fp is leaked if the sysfs file is successfully opened

* if fgets() fails *addr will be invalid

* if fgets() fails *addr the wrong return value is given

Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-Off-By: Simon Horman <horms@verge.net.au>

1 files changed, 27 insertions, 13 deletions

diff --git a/kexec/crashdump.c b/kexec/crashdump.c
index 9e5bf15..21d157c 100644
--- a/kexec/crashdump.c
+++ b/kexec/crashdump.c
@@ -39,27 +39,41 @@ int get_crash_notes_per_cpu(int cpu, uint64_t *addr)
 	struct stat cpu_stat;
 	int count;
 	unsigned long long temp;
+	int fopen_errno;
+	int stat_errno;
+
+	*addr = 0;
 
 	sprintf(crash_notes, "/sys/devices/system/cpu/cpu%d/crash_notes", cpu);
 	fp = fopen(crash_notes, "r");
 	if (!fp) {
-		/* Either sysfs is not mounted or CPU is not present*/
-		if (stat("/sys/devices", &cpu_stat))
-			die("Sysfs is not mounted. Try mounting sysfs\n");
-
+		fopen_errno = errno;
+		if (fopen_errno != ENOENT)
+			die(stderr, "Could not open \"%s\": %s\n",
+				crash_notes, strerror(fopen_errno));
+		if (!stat("/sys/devices", &cpu_stat)) {
+			stat_errno = errno;
+			fprintf(stderr, "Could not open \"%s\": %s\n",
+				crash_notes, strerror(fopen_errno));
+			if (stat_errno == ENOENT)
+				die("\"/sys/devices\" does not exist. "
+				    "Sysfs does not seem to be mounted. "
+				    "Try mounting sysfs.\n");
+			die("Could not open \"/sys/devices\": %s\n",
+			    crash_notes, strerror(stat_errno));
+		}
 		/* CPU is not physically present.*/
-		*addr = 0;
-		return errno;
-	}
-	if (fgets(line, sizeof(line), fp) != 0) {
-		count = sscanf(line, "%Lx", &temp);
-		if (count != 1)
-			die("Cannot parse %s: %s\n", crash_notes,
-						strerror(errno));
-		*addr = (uint64_t) temp;
+		return -1;
 	}
+	if (!fgets(line, sizeof(line), fp))
+		die("Cannot parse %s: %s\n", crash_notes, strerror(errno));
+	count = sscanf(line, "%Lx", &temp);
+	if (count != 1)
+		die("Cannot parse %s: %s\n", crash_notes, strerror(errno));
+	*addr = (uint64_t) temp;
 #if 0
 	printf("crash_notes addr = %Lx\n", *addr);
 #endif
+
 	return 0;
 }