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>

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 7bcf822a89f9f1d2593876ec12f62007666fe976..b415dfe5de45fc93df8d584931c768d66c93fef8 100644 (file)
--- 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 5c548bb74f07b60b0fc2b42ffd1dc6420a023e66..30d0bcba88479185e0a4715811bd74633db88686 100644 (file)
--- 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 {