linux-kernel - [PATCH] tools/thermal: tmon: fix for segfault

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <0ef0f303-de13-326c-efa4-82fa40847099@objecthunter.net>
Date:   Mon, 12 Mar 2018 19:57:06 +0100
From:   Frank Asseg <frank.asseg@...ecthunter.net>
To:     trivial@...nel.org
Cc:     linux-kernel@...r.kernel.org
Subject: [PATCH] tools/thermal: tmon: fix for segfault

Fixes a segfault occurring when e.g. <TAB> is pressed multiple times in
the ncurses tmon application. The segfault is caused by incrementing
cur_thermal_record in the main function without checking if it's value
reached NR_THERMAL_RECORD immediately. Since the boundary check only
occurred in update_thermal_data a race condition existed, which lead to
an attempted read beyond the last element of the trec array.
The fix was implemented by moving the cur_thermal_record incrementation
to the update_thermal_data function using a temporary variable
on which the boundary condition is checked before updating
cur_thread_record, so that the variable is never incremented beyond the
trec array's boundary.
It seems the segfault does not occur on every machine: On a HP EliteBook
G4 the segfault happens, while it does not happen on a Thinkpad T540p.

Signed-off-by: Frank Asseg <frank.asseg@...ecthunter.net>
---
 tools/thermal/tmon/sysfs.c | 12 +++++++-----
 tools/thermal/tmon/tmon.c  |  1 -
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/thermal/tmon/sysfs.c b/tools/thermal/tmon/sysfs.c
index 1c12536f2081..18f523557983 100644
--- a/tools/thermal/tmon/sysfs.c
+++ b/tools/thermal/tmon/sysfs.c
@@ -486,6 +486,7 @@ int zone_instance_to_index(int zone_inst)
 int update_thermal_data()
 {
 	int i;
+	int next_thermal_record = cur_thermal_record + 1;
 	char tz_name[256];
 	static unsigned long samples;

@@ -495,9 +496,9 @@ int update_thermal_data()
 	}

 	/* circular buffer for keeping historic data */
-	if (cur_thermal_record >= NR_THERMAL_RECORDS)
-		cur_thermal_record = 0;
-	gettimeofday(&trec[cur_thermal_record].tv, NULL);
+	if (next_thermal_record >= NR_THERMAL_RECORDS)
+		next_thermal_record = 0;
+	gettimeofday(&trec[next_thermal_record].tv, NULL);
 	if (tmon_log) {
 		fprintf(tmon_log, "%lu ", ++samples);
 		fprintf(tmon_log, "%3.1f ", p_param.t_target);
@@ -507,11 +508,12 @@ int update_thermal_data()
 		snprintf(tz_name, 256, "%s/%s%d", THERMAL_SYSFS, TZONE,
 			ptdata.tzi[i].instance);
 		sysfs_get_ulong(tz_name, "temp",
-				&trec[cur_thermal_record].temp[i]);
+				&trec[next_thermal_record].temp[i]);
 		if (tmon_log)
 			fprintf(tmon_log, "%lu ",
-				trec[cur_thermal_record].temp[i]/1000);
+				trec[next_thermal_record].temp[i] / 1000);
 	}
+	cur_thermal_record = next_thermal_record;
 	for (i = 0; i < ptdata.nr_cooling_dev; i++) {
 		char cdev_name[256];
 		unsigned long val;
diff --git a/tools/thermal/tmon/tmon.c b/tools/thermal/tmon/tmon.c
index 9aa19652e8e8..b43138f8b862 100644
--- a/tools/thermal/tmon/tmon.c
+++ b/tools/thermal/tmon/tmon.c
@@ -336,7 +336,6 @@ int main(int argc, char **argv)
 			show_data_w();
 			show_cooling_device();
 		}
-		cur_thermal_record++;
 		time_elapsed += ticktime;
 		controller_handler(trec[0].temp[target_tz_index] / 1000,
 				&yk);
-- 
2.16.2