All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yordan Karadzhov <ykaradzhov@vmware.com>
To: "rostedt@goodmis.org" <rostedt@goodmis.org>
Cc: "linux-trace-devel@vger.kernel.org" <linux-trace-devel@vger.kernel.org>
Subject: [PATCH 05/17] kernel-shark-qt: Protect all calls of tep_read_number_field()
Date: Wed, 28 Nov 2018 15:16:12 +0000	[thread overview]
Message-ID: <20181128151530.21965-6-ykaradzhov@vmware.com> (raw)
In-Reply-To: <20181128151530.21965-1-ykaradzhov@vmware.com>

tep_read_number_field() is being used to retrieve the value of a data
field and this value has being used without checking if the function
succeeded. This is a potential bug because tep_read_number_field() may
fail and in such a case the retrieved field value will be arbitrary.

Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark-qt/src/plugins/sched_events.c | 52 +++++++++++++---------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/kernel-shark-qt/src/plugins/sched_events.c b/kernel-shark-qt/src/plugins/sched_events.c
index 1851569..59ffcfe 100644
--- a/kernel-shark-qt/src/plugins/sched_events.c
+++ b/kernel-shark-qt/src/plugins/sched_events.c
@@ -97,10 +97,12 @@ int plugin_get_next_pid(struct tep_record *record)
 	struct plugin_sched_context *plugin_ctx =
 		plugin_sched_context_handler;
 	unsigned long long val;
+	int ret;
 
-	tep_read_number_field(plugin_ctx->sched_switch_next_field,
-			      record->data, &val);
-	return val;
+	ret = tep_read_number_field(plugin_ctx->sched_switch_next_field,
+				    record->data, &val);
+
+	return ret ? : val;
 }
 
 /**
@@ -113,10 +115,12 @@ int plugin_get_rec_wakeup_pid(struct tep_record *record)
 	struct plugin_sched_context *plugin_ctx =
 		plugin_sched_context_handler;
 	unsigned long long val;
+	int ret;
+
+	ret = tep_read_number_field(plugin_ctx->sched_wakeup_pid_field,
+				    record->data, &val);
 
-	tep_read_number_field(plugin_ctx->sched_wakeup_pid_field,
-			      record->data, &val);
-	return val;
+	return ret ? : val;
 }
 
 static void plugin_register_command(struct kshark_context *kshark_ctx,
@@ -145,11 +149,12 @@ static int plugin_get_rec_wakeup_new_pid(struct tep_record *record)
 	struct plugin_sched_context *plugin_ctx =
 		plugin_sched_context_handler;
 	unsigned long long val;
+	int ret;
 
-	tep_read_number_field(plugin_ctx->sched_wakeup_new_pid_field,
-				 record->data, &val);
+	ret = tep_read_number_field(plugin_ctx->sched_wakeup_new_pid_field,
+				    record->data, &val);
 
-	return val;
+	return ret ? : val;
 }
 
 /**
@@ -170,7 +175,7 @@ bool plugin_wakeup_match_rec_pid(struct kshark_context *kshark_ctx,
 	struct plugin_sched_context *plugin_ctx;
 	struct tep_record *record = NULL;
 	unsigned long long val;
-	int wakeup_pid = -1;
+	int ret, wakeup_pid = -1;
 
 	plugin_ctx = plugin_sched_context_handler;
 	if (!plugin_ctx)
@@ -181,10 +186,10 @@ bool plugin_wakeup_match_rec_pid(struct kshark_context *kshark_ctx,
 		record = kshark_read_at(kshark_ctx, e->offset);
 
 		/* We only want those that actually woke up the task. */
-		tep_read_number_field(plugin_ctx->sched_wakeup_success_field,
-				      record->data, &val);
+		ret = tep_read_number_field(plugin_ctx->sched_wakeup_success_field,
+					    record->data, &val);
 
-		if (val)
+		if (ret == 0 && val)
 			wakeup_pid = plugin_get_rec_wakeup_pid(record);
 	}
 
@@ -193,10 +198,10 @@ bool plugin_wakeup_match_rec_pid(struct kshark_context *kshark_ctx,
 		record = kshark_read_at(kshark_ctx, e->offset);
 
 		/* We only want those that actually woke up the task. */
-		tep_read_number_field(plugin_ctx->sched_wakeup_new_success_field,
-				      record->data, &val);
+		ret = tep_read_number_field(plugin_ctx->sched_wakeup_new_success_field,
+					    record->data, &val);
 
-		if (val)
+		if (ret == 0 && val)
 			wakeup_pid = plugin_get_rec_wakeup_new_pid(record);
 	}
 
@@ -224,7 +229,7 @@ bool plugin_switch_match_rec_pid(struct kshark_context *kshark_ctx,
 {
 	struct plugin_sched_context *plugin_ctx;
 	unsigned long long val;
-	int switch_pid = -1;
+	int ret, switch_pid = -1;
 
 	plugin_ctx = plugin_sched_context_handler;
 
@@ -233,10 +238,10 @@ bool plugin_switch_match_rec_pid(struct kshark_context *kshark_ctx,
 		struct tep_record *record;
 
 		record = kshark_read_at(kshark_ctx, e->offset);
-		tep_read_number_field(plugin_ctx->sched_switch_prev_state_field,
-				      record->data, &val);
+		ret = tep_read_number_field(plugin_ctx->sched_switch_prev_state_field,
+					    record->data, &val);
 
-		if (!(val & 0x7f))
+		if (ret == 0 && !(val & 0x7f))
 			switch_pid = tep_data_pid(plugin_ctx->pevent, record);
 
 		free_record(record);
@@ -278,8 +283,11 @@ static void plugin_sched_action(struct kshark_context *kshark_ctx,
 				struct tep_record *rec,
 				struct kshark_entry *entry)
 {
-	entry->pid = plugin_get_next_pid(rec);
-	plugin_register_command(kshark_ctx, rec, entry->pid);
+	int pid = plugin_get_next_pid(rec);
+	if (pid >= 0) {
+		entry->pid = pid;
+		plugin_register_command(kshark_ctx, rec, entry->pid);
+	}
 }
 
 static int plugin_sched_init(struct kshark_context *kshark_ctx)
-- 
2.17.1

  parent reply	other threads:[~2018-11-29  2:18 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-28 15:16 [PATCH 00/17] More modifications and bug fixes toward KS 1.0 Yordan Karadzhov
2018-11-28 15:16 ` [PATCH 01/17] kernel-shark-qt: Updata Event filter mask when applaing filters to Graph Yordan Karadzhov
2018-11-28 15:16 ` [PATCH 02/17] kernel-shark-qt: Reprocess all CPU collections when the filtering changes Yordan Karadzhov
2018-11-28 15:16 ` [PATCH 03/17] kernel-shark-qt: Fix a byg in unset_event_filter_flag() Yordan Karadzhov
2018-11-28 15:16 ` [PATCH 04/17] kernel-shark qt: No error when Record authentication dialog is closed Yordan Karadzhov
2018-11-28 15:16 ` Yordan Karadzhov [this message]
2018-11-28 15:16 ` [PATCH 06/17] kernel-shark-qt: Ignore sched_wakeup events if the task is running Yordan Karadzhov
2018-11-28 15:16 ` [PATCH 07/17] kernel-shark-qt: Fix the documentation of libkshark-model Yordan Karadzhov
2018-11-28 15:16 ` [PATCH 08/17] kernel-shark-qt: Add a method for easy retrieve of all Ids of a filter Yordan Karadzhov
2018-11-28 15:16 ` [PATCH 09/17] kernel-shark-qt: Add centralized context menu for View and Graph widgets Yordan Karadzhov
2018-11-28 15:16 ` [PATCH 10/17] kernel-shark-qt: Add keyboard shortcuts for deselecting the marker Yordan Karadzhov
2018-11-28 15:16 ` [PATCH 11/17] kernel-shark-qt: Sort all graphs before plotting Yordan Karadzhov
2018-11-28 17:12   ` Steven Rostedt
2018-11-29 11:29     ` Yordan Karadzhov
2018-11-28 15:16 ` [PATCH 12/17] kernel-shark-qt: Add CPU-based filtering to the C API Yordan Karadzhov
2018-11-28 15:16 ` [PATCH 13/17] kernel-shark-qt: Add CPU-based filtering to KsDataStore Yordan Karadzhov
2018-11-28 15:16 ` [PATCH 14/17] kernel-shark-qt: Add Hide CPU action to the Quick Context Menu Yordan Karadzhov
2018-11-28 15:16 ` [PATCH 15/17] kernel-shark-qt: Add the CPU filters to the Json config I/O Yordan Karadzhov
2018-11-28 15:16 ` [PATCH 16/17] kernel-shark-qt: Add "Hide CPU" checkbox dialog to the Main window menu Yordan Karadzhov
2018-11-28 17:16   ` Steven Rostedt
2018-11-28 15:16 ` [PATCH 17/17] kernel-shark-qt: Add the user filter mask to the Json config I/O Yordan Karadzhov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181128151530.21965-6-ykaradzhov@vmware.com \
    --to=ykaradzhov@vmware.com \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.