From patchwork Wed Nov 21 15:14:18 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Yordan Karadzhov X-Patchwork-Id: 10759837 Return-Path: Received: from mail-eopbgr790053.outbound.protection.outlook.com ([40.107.79.53]:64832 "EHLO NAM03-CO1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1730512AbeKVBtL (ORCPT ); Wed, 21 Nov 2018 20:49:11 -0500 From: Yordan Karadzhov To: "rostedt@goodmis.org" CC: "linux-trace-devel@vger.kernel.org" Subject: [PATCH 01/11] kernel-shark-qt: protect all calls of tep_read_number_field() Date: Wed, 21 Nov 2018 15:14:18 +0000 Message-ID: <20181121151356.16901-2-ykaradzhov@vmware.com> References: <20181121151356.16901-1-ykaradzhov@vmware.com> In-Reply-To: <20181121151356.16901-1-ykaradzhov@vmware.com> Content-Language: en-US Content-ID: MIME-Version: 1.0 Sender: linux-trace-devel-owner@vger.kernel.org List-ID: Content-Length: 6489 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 fаil and in such a case the retrieved field value will be arbitrary. Signed-off-by: Yordan Karadzhov --- 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..c22e198 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 == 0) ? val : ret; } /** @@ -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 == 0) ? val : ret; } 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 == 0) ? val : ret; } /** @@ -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)