197 lines
6.9 KiB
Diff
197 lines
6.9 KiB
Diff
From f790fae387e151437057cc8a223b1fba282aecfa Mon Sep 17 00:00:00 2001
|
|
From: Kamal Heib <kheib@redhat.com>
|
|
Date: Mon, 20 Apr 2026 15:08:58 -0400
|
|
Subject: [PATCH] net/mlx5: fw_tracer, Validate format string parameters
|
|
|
|
JIRA: https://redhat.atlassian.net/browse/RHEL-169055
|
|
|
|
commit b35966042d20b14e2d83330049f77deec5229749
|
|
Author: Shay Drory <shayd@nvidia.com>
|
|
Date: Tue Dec 9 14:56:11 2025 +0200
|
|
|
|
net/mlx5: fw_tracer, Validate format string parameters
|
|
|
|
Add validation for format string parameters in the firmware tracer to
|
|
prevent potential security vulnerabilities and crashes from malformed
|
|
format strings received from firmware.
|
|
|
|
The firmware tracer receives format strings from the device firmware and
|
|
uses them to format trace messages. Without proper validation, bad
|
|
firmware could provide format strings with invalid format specifiers
|
|
(e.g., %s, %p, %n) that could lead to crashes, or other undefined
|
|
behavior.
|
|
|
|
Add mlx5_tracer_validate_params() to validate that all format specifiers
|
|
in trace strings are limited to safe integer/hex formats (%x, %d, %i,
|
|
%u, %llx, %lx, etc.). Reject strings containing other format types that
|
|
could be used to access arbitrary memory or cause crashes.
|
|
Invalid format strings are added to the trace output for visibility with
|
|
"BAD_FORMAT: " prefix.
|
|
|
|
Fixes: 70dd6fdb8987 ("net/mlx5: FW tracer, parse traces and kernel tracing support")
|
|
Signed-off-by: Shay Drory <shayd@nvidia.com>
|
|
Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
|
|
Reported-by: Breno Leitao <leitao@debian.org>
|
|
Closes: https://lore.kernel.org/netdev/hanz6rzrb2bqbplryjrakvkbmv4y5jlmtthnvi3thg5slqvelp@t3s3erottr6s/
|
|
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
|
|
Link: https://patch.msgid.link/1765284977-1363052-4-git-send-email-tariqt@nvidia.com
|
|
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
|
|
Signed-off-by: Kamal Heib <kheib@redhat.com>
|
|
|
|
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c b/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c
|
|
index 7bcf822a89f9..b415dfe5de45 100644
|
|
--- a/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c
|
|
+++ b/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c
|
|
@@ -33,6 +33,7 @@
|
|
#include "lib/eq.h"
|
|
#include "fw_tracer.h"
|
|
#include "fw_tracer_tracepoint.h"
|
|
+#include <linux/ctype.h>
|
|
|
|
static int mlx5_query_mtrc_caps(struct mlx5_fw_tracer *tracer)
|
|
{
|
|
@@ -358,6 +359,43 @@ static const char *VAL_PARM = "%llx";
|
|
static const char *REPLACE_64_VAL_PARM = "%x%x";
|
|
static const char *PARAM_CHAR = "%";
|
|
|
|
+static bool mlx5_is_valid_spec(const char *str)
|
|
+{
|
|
+ /* Parse format specifiers to find the actual type.
|
|
+ * Structure: %[flags][width][.precision][length]type
|
|
+ * Skip flags, width, precision & length.
|
|
+ */
|
|
+ while (isdigit(*str) || *str == '#' || *str == '.' || *str == 'l')
|
|
+ str++;
|
|
+
|
|
+ /* Check if it's a valid integer/hex specifier:
|
|
+ * Valid formats: %x, %d, %i, %u, etc.
|
|
+ */
|
|
+ if (*str != 'x' && *str != 'X' && *str != 'd' && *str != 'i' &&
|
|
+ *str != 'u' && *str != 'c')
|
|
+ return false;
|
|
+
|
|
+ return true;
|
|
+}
|
|
+
|
|
+static bool mlx5_tracer_validate_params(const char *str)
|
|
+{
|
|
+ const char *substr = str;
|
|
+
|
|
+ if (!str)
|
|
+ return false;
|
|
+
|
|
+ substr = strstr(substr, PARAM_CHAR);
|
|
+ while (substr) {
|
|
+ if (!mlx5_is_valid_spec(substr + 1))
|
|
+ return false;
|
|
+
|
|
+ substr = strstr(substr + 1, PARAM_CHAR);
|
|
+ }
|
|
+
|
|
+ return true;
|
|
+}
|
|
+
|
|
static int mlx5_tracer_message_hash(u32 message_id)
|
|
{
|
|
return jhash_1word(message_id, 0) & (MESSAGE_HASH_SIZE - 1);
|
|
@@ -419,6 +457,10 @@ static int mlx5_tracer_get_num_of_params(char *str)
|
|
char *substr, *pstr = str;
|
|
int num_of_params = 0;
|
|
|
|
+ /* Validate that all parameters are valid before processing */
|
|
+ if (!mlx5_tracer_validate_params(str))
|
|
+ return -EINVAL;
|
|
+
|
|
/* replace %llx with %x%x */
|
|
substr = strstr(pstr, VAL_PARM);
|
|
while (substr) {
|
|
@@ -570,14 +612,17 @@ void mlx5_tracer_print_trace(struct tracer_string_format *str_frmt,
|
|
{
|
|
char tmp[512];
|
|
|
|
- snprintf(tmp, sizeof(tmp), str_frmt->string,
|
|
- str_frmt->params[0],
|
|
- str_frmt->params[1],
|
|
- str_frmt->params[2],
|
|
- str_frmt->params[3],
|
|
- str_frmt->params[4],
|
|
- str_frmt->params[5],
|
|
- str_frmt->params[6]);
|
|
+ if (str_frmt->invalid_string)
|
|
+ snprintf(tmp, sizeof(tmp), "BAD_FORMAT: %s", str_frmt->string);
|
|
+ else
|
|
+ snprintf(tmp, sizeof(tmp), str_frmt->string,
|
|
+ str_frmt->params[0],
|
|
+ str_frmt->params[1],
|
|
+ str_frmt->params[2],
|
|
+ str_frmt->params[3],
|
|
+ str_frmt->params[4],
|
|
+ str_frmt->params[5],
|
|
+ str_frmt->params[6]);
|
|
|
|
trace_mlx5_fw(dev->tracer, trace_timestamp, str_frmt->lost,
|
|
str_frmt->event_id, tmp);
|
|
@@ -609,6 +654,13 @@ static int mlx5_tracer_handle_raw_string(struct mlx5_fw_tracer *tracer,
|
|
return 0;
|
|
}
|
|
|
|
+static void mlx5_tracer_handle_bad_format_string(struct mlx5_fw_tracer *tracer,
|
|
+ struct tracer_string_format *cur_string)
|
|
+{
|
|
+ cur_string->invalid_string = true;
|
|
+ list_add_tail(&cur_string->list, &tracer->ready_strings_list);
|
|
+}
|
|
+
|
|
static int mlx5_tracer_handle_string_trace(struct mlx5_fw_tracer *tracer,
|
|
struct tracer_event *tracer_event)
|
|
{
|
|
@@ -619,12 +671,18 @@ static int mlx5_tracer_handle_string_trace(struct mlx5_fw_tracer *tracer,
|
|
if (!cur_string)
|
|
return mlx5_tracer_handle_raw_string(tracer, tracer_event);
|
|
|
|
- cur_string->num_of_params = mlx5_tracer_get_num_of_params(cur_string->string);
|
|
- cur_string->last_param_num = 0;
|
|
cur_string->event_id = tracer_event->event_id;
|
|
cur_string->tmsn = tracer_event->string_event.tmsn;
|
|
cur_string->timestamp = tracer_event->string_event.timestamp;
|
|
cur_string->lost = tracer_event->lost_event;
|
|
+ cur_string->last_param_num = 0;
|
|
+ cur_string->num_of_params = mlx5_tracer_get_num_of_params(cur_string->string);
|
|
+ if (cur_string->num_of_params < 0) {
|
|
+ pr_debug("%s Invalid format string parameters\n",
|
|
+ __func__);
|
|
+ mlx5_tracer_handle_bad_format_string(tracer, cur_string);
|
|
+ return 0;
|
|
+ }
|
|
if (cur_string->num_of_params == 0) /* trace with no params */
|
|
list_add_tail(&cur_string->list, &tracer->ready_strings_list);
|
|
} else {
|
|
@@ -634,6 +692,11 @@ static int mlx5_tracer_handle_string_trace(struct mlx5_fw_tracer *tracer,
|
|
__func__, tracer_event->string_event.tmsn);
|
|
return mlx5_tracer_handle_raw_string(tracer, tracer_event);
|
|
}
|
|
+ if (cur_string->num_of_params < 0) {
|
|
+ pr_debug("%s string parameter of invalid string, dumping\n",
|
|
+ __func__);
|
|
+ return 0;
|
|
+ }
|
|
cur_string->last_param_num += 1;
|
|
if (cur_string->last_param_num > TRACER_MAX_PARAMS) {
|
|
pr_debug("%s Number of params exceeds the max (%d)\n",
|
|
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.h b/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.h
|
|
index 5c548bb74f07..30d0bcba8847 100644
|
|
--- a/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.h
|
|
+++ b/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.h
|
|
@@ -125,6 +125,7 @@ struct tracer_string_format {
|
|
struct list_head list;
|
|
u32 timestamp;
|
|
bool lost;
|
|
+ bool invalid_string;
|
|
};
|
|
|
|
enum mlx5_fw_tracer_ownership_state {
|
|
--
|
|
2.50.1 (Apple Git-155)
|
|
|