From patchwork Wed Apr 5 15:44:10 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bruce Richardson X-Patchwork-Id: 125813 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5D81E428D4; Wed, 5 Apr 2023 17:44:39 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 691AC42B7E; Wed, 5 Apr 2023 17:44:35 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mails.dpdk.org (Postfix) with ESMTP id 09E1641133; Wed, 5 Apr 2023 17:44:33 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1680709474; x=1712245474; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=vyG6oyrJvzCQDUTMUwfJrdN9vsdcCdtrQddmeHIlMYc=; b=WhUMs7/V4qq/n8UWE/+deUGoDqq9uSUTneH26YOaa+MT+JYOzJnBeZoV rfRoRzznTYrCLGkoTfFIV17vhRxBcIdHREI3Ff2xWZqQr7ZCfp3UiZmDD l52zFlHD/AEwDYJyGOXdmltuHxrzIfWkXEJFBE9E+8kCKWRuuhvlWIwra wxvQEG7yERnUSrTP81fWMsJNUnCPpXmI9ZvS9DgR+obWrtTZuX0RULltb eWCoJYPTjRzTS67E5DBsZuxHzRIuIzHnQskOLByi2swOTFVJHeW1hOsi3 vsvDUWyJTicV+dJPI+5NYymIK7xVf9o6IO8E9RVUn/V3PTvGqs2Hzr8Bb Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="339980353" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="339980353" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Apr 2023 08:44:30 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="686790254" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="686790254" Received: from silpixa00401385.ir.intel.com ([10.237.214.40]) by orsmga002.jf.intel.com with ESMTP; 05 Apr 2023 08:44:28 -0700 From: Bruce Richardson To: dev@dpdk.org Cc: ciara.power@intel.com, roretzla@linux.microsoft.com, Bruce Richardson , stable@dpdk.org Subject: [PATCH v2 1/5] telemetry: fix autotest failures on Alpine Date: Wed, 5 Apr 2023 16:44:10 +0100 Message-Id: <20230405154414.183915-2-bruce.richardson@intel.com> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20230405154414.183915-1-bruce.richardson@intel.com> References: <20230310181836.162336-1-bruce.richardson@intel.com> <20230405154414.183915-1-bruce.richardson@intel.com> MIME-Version: 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Alpine linux, the telemetry_data_autotest was failing for the test where we had dictionaries embedded in other dictionaries up to three levels deep. Indications are that this issue is due to excess data being stored on the stack, so replace stack-allocated buffer data with dynamically allocated data in the case where we are doing recursive processing of telemetry data structures into json. Bugzilla ID: 1177 Fixes: c933bb5177ca ("telemetry: support array values in data object") Fixes: d2671e642a8e ("telemetry: support dict of dicts") Cc: stable@dpdk.org Signed-off-by: Bruce Richardson Acked-by: Tyler Retzlaff --- V2: set '\0' in newly malloc'ed buffer to ensure it always has valid string data. --- lib/telemetry/telemetry.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c index 7bceadcee7..728a0bffd4 100644 --- a/lib/telemetry/telemetry.c +++ b/lib/telemetry/telemetry.c @@ -208,7 +208,11 @@ container_to_json(const struct rte_tel_data *d, char *out_buf, size_t buf_len) break; case RTE_TEL_CONTAINER: { - char temp[buf_len]; + char *temp = malloc(buf_len); + if (temp == NULL) + break; + *temp = '\0'; /* ensure valid string */ + const struct container *cont = &v->value.container; if (container_to_json(cont->data, @@ -219,6 +223,7 @@ container_to_json(const struct rte_tel_data *d, char *out_buf, size_t buf_len) v->name, temp); if (!cont->keep) rte_tel_data_free(cont->data); + free(temp); break; } } @@ -275,7 +280,11 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s) break; case RTE_TEL_CONTAINER: { - char temp[buf_len]; + char *temp = malloc(buf_len); + if (temp == NULL) + break; + *temp = '\0'; /* ensure valid string */ + const struct container *cont = &v->value.container; if (container_to_json(cont->data, @@ -286,6 +295,7 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s) v->name, temp); if (!cont->keep) rte_tel_data_free(cont->data); + free(temp); } } } @@ -311,7 +321,11 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s) buf_len, used, d->data.array[i].uval); else if (d->type == TEL_ARRAY_CONTAINER) { - char temp[buf_len]; + char *temp = malloc(buf_len); + if (temp == NULL) + break; + *temp = '\0'; /* ensure valid string */ + const struct container *rec_data = &d->data.array[i].container; if (container_to_json(rec_data->data, @@ -321,6 +335,7 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s) buf_len, used, temp); if (!rec_data->keep) rte_tel_data_free(rec_data->data); + free(temp); } break; } From patchwork Wed Apr 5 15:44:11 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bruce Richardson X-Patchwork-Id: 125814 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 9D6DC428D4; Wed, 5 Apr 2023 17:44:46 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id BD21842C54; Wed, 5 Apr 2023 17:44:36 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mails.dpdk.org (Postfix) with ESMTP id 46C0241156 for ; Wed, 5 Apr 2023 17:44:34 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1680709474; x=1712245474; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=ogeTmaVhDWFYJkgcf4GTeduVwTLiQZUnh6LH7/evuyQ=; b=RECaucDdm9q1TXgCNlcZVFAmntq5MrsvnYWObqGWjFFkI9r7DHu9kCRW pB0msBzPQBVFtZqbhDrFKQ5gibThpB/40ww216/g2d6wJMfFz/T6+VL6H Zue4QhLWkNmD1yU87/u4UdH56p5igoTcRLCbc9uEt4ip4oZjB+xfDiEOd rE+xX//Mzzdpd+Yl4RPqEIy4FzE20aTYFWRXJsqpYbIFRnN3CB+Hl7J6I SAAy53c08r0VriKJhKWob2/CSLAiKiuITbKhpQXzUm77k3jTuexLa6GTc sQKT4NSApkqIsGaQwGOr1KyMXbxlE7vapVjDFbKtFckPv4v0JODAGKbTR Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="339980369" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="339980369" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Apr 2023 08:44:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="686790275" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="686790275" Received: from silpixa00401385.ir.intel.com ([10.237.214.40]) by orsmga002.jf.intel.com with ESMTP; 05 Apr 2023 08:44:31 -0700 From: Bruce Richardson To: dev@dpdk.org Cc: ciara.power@intel.com, roretzla@linux.microsoft.com, Bruce Richardson Subject: [PATCH v2 2/5] telemetry: remove variable length array in printf fn Date: Wed, 5 Apr 2023 16:44:11 +0100 Message-Id: <20230405154414.183915-3-bruce.richardson@intel.com> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20230405154414.183915-1-bruce.richardson@intel.com> References: <20230310181836.162336-1-bruce.richardson@intel.com> <20230405154414.183915-1-bruce.richardson@intel.com> MIME-Version: 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org The json_snprintf function, used to add json characters on to a buffer, leaving the buffer unmodified in case of error, used a variable length array to store the data temporarily while checking for overflow. VLAs can be unsafe, and are unsupported by some compilers, so remove use of the VLA. For the normal case where there is only a small amount of existing text in the buffer (<4 chars) to be preserved, save that off temporarily to a local array, and restore on error. To handle cases where there is more than a few characters in the buffer, we use the existing logic of doing the print to a temporary buffer initially and then copying. In this case, though we use malloc-allocated buffer rather than VLA. Within the unit tests, the "telemetry_data_autotests" test cases - which mimic real telemetry use - all exercise the first path. The telemetry_json_autotest cases work directly with generating json, and use uninitialized buffers so also test the second, malloc-allocated buffer, cases. Signed-off-by: Bruce Richardson --- lib/telemetry/telemetry_json.h | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/lib/telemetry/telemetry_json.h b/lib/telemetry/telemetry_json.h index 744bbfe053..cb18453449 100644 --- a/lib/telemetry/telemetry_json.h +++ b/lib/telemetry/telemetry_json.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -30,17 +31,40 @@ __rte_format_printf(3, 4) static inline int __json_snprintf(char *buf, const int len, const char *format, ...) { - char tmp[len]; va_list ap; + char tmp[4]; + char *newbuf; int ret; + if (len == 0) + return 0; + + /* to ensure unmodified if we overflow, we save off any values currently in buf + * before we printf, if they are short enough. We restore them on error. + */ + if (strnlen(buf, sizeof(tmp)) < sizeof(tmp)) { + strcpy(tmp, buf); /* strcpy is safe as we know the length */ + va_start(ap, format); + ret = vsnprintf(buf, len, format, ap); + va_end(ap); + if (ret > 0 && ret < len) + return ret; + strcpy(buf, tmp); /* restore on error */ + return 0; + } + + /* in normal operations should never hit this, but can do if buffer is + * incorrectly initialized e.g. in unit test cases + */ va_start(ap, format); - ret = vsnprintf(tmp, sizeof(tmp), format, ap); + ret = vasprintf(&newbuf, format, ap); va_end(ap); - if (ret > 0 && ret < (int)sizeof(tmp) && ret < len) { - strcpy(buf, tmp); + if (ret > 0 && ret < len) { + strcpy(buf, newbuf); + free(newbuf); return ret; } + free(newbuf); return 0; /* nothing written or modified */ } From patchwork Wed Apr 5 15:44:12 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bruce Richardson X-Patchwork-Id: 125815 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6EE31428D4; Wed, 5 Apr 2023 17:44:55 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 50D7642D1A; Wed, 5 Apr 2023 17:44:38 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mails.dpdk.org (Postfix) with ESMTP id 642D342BAC for ; Wed, 5 Apr 2023 17:44:36 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1680709476; x=1712245476; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=aMqGXe4qQ2TIKAIHehX1IKjUBXVjPvqniY4tJ8Tfm9w=; b=kCHHYYWYJkkHufeidY8n3eHUrUZrswFXQ+e7RNAH0wpmQR9kjy+gs3GH wRFKHZcDfM01pHVFBbd0slMgTv2+M5ejykKhnUWGI0DMaiKQVoAO1mJHe Jr/oVMSZwsnAkU5tKDEjx7vHcrtdEbKNifwPcgHio3GvmyLY3wE/fovIg uS4lnqCTI8hf+s5RVpR7/TT5QgnvYvGBSdazfT1d8B0K/bwVhmhHL9UDk ZC891pnl71qCPLQQrPMriGgJjIHpLivm2ymd2MBw6QV7G++TuZ7IwNC65 SmxsP1H8A0qfnmInmva27fZfWn/P6OP6G+kzX0B1J4aXNa4sFN9auJEU0 A==; X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="339980381" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="339980381" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Apr 2023 08:44:35 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="686790287" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="686790287" Received: from silpixa00401385.ir.intel.com ([10.237.214.40]) by orsmga002.jf.intel.com with ESMTP; 05 Apr 2023 08:44:33 -0700 From: Bruce Richardson To: dev@dpdk.org Cc: ciara.power@intel.com, roretzla@linux.microsoft.com, Bruce Richardson Subject: [PATCH v2 3/5] telemetry: split out body of json string format fn Date: Wed, 5 Apr 2023 16:44:12 +0100 Message-Id: <20230405154414.183915-4-bruce.richardson@intel.com> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20230405154414.183915-1-bruce.richardson@intel.com> References: <20230310181836.162336-1-bruce.richardson@intel.com> <20230405154414.183915-1-bruce.richardson@intel.com> MIME-Version: 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org To enable further rework to (efficiently) avoid using variable-length arrays, we first separate out the body of the __json_format_str function. This means that the actual VLA buffer is in the wrapper function, and means we can reuse the actual writing code in multiple code paths without duplication. Signed-off-by: Bruce Richardson --- lib/telemetry/telemetry_json.h | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/lib/telemetry/telemetry_json.h b/lib/telemetry/telemetry_json.h index cb18453449..4c1941ad15 100644 --- a/lib/telemetry/telemetry_json.h +++ b/lib/telemetry/telemetry_json.h @@ -76,15 +76,13 @@ static const char control_chars[0x20] = { /** * @internal - * This function acts the same as __json_snprintf(buf, len, "%s%s%s", prefix, str, suffix) - * except that it does proper escaping of "str" as necessary. Prefix and suffix should be compile- - * time constants, or values not needing escaping. - * Drops any invalid characters we don't support + * Function that does the actual printing, used by __json_format_str. Modifies buffer + * directly, but returns 0 on overflow. Otherwise returns number of chars written to buffer. */ static inline int -__json_format_str(char *buf, const int len, const char *prefix, const char *str, const char *suffix) +__json_format_str_to_buf(char *tmp, const int len, + const char *prefix, const char *str, const char *suffix) { - char tmp[len]; int tmpidx = 0; while (*prefix != '\0' && tmpidx < len) @@ -119,11 +117,29 @@ __json_format_str(char *buf, const int len, const char *prefix, const char *str, return 0; tmp[tmpidx] = '\0'; - - strcpy(buf, tmp); return tmpidx; } +/** + * @internal + * This function acts the same as __json_snprintf(buf, len, "%s%s%s", prefix, str, suffix) + * except that it does proper escaping of "str" as necessary. Prefix and suffix should be compile- + * time constants, or values not needing escaping. + * Drops any invalid characters we don't support + */ +static inline int +__json_format_str(char *buf, const int len, const char *prefix, const char *str, const char *suffix) +{ + char tmp[len]; + int ret; + + ret = __json_format_str_to_buf(tmp, len, prefix, str, suffix); + if (ret > 0) + strcpy(buf, tmp); + + return ret; +} + /* Copies an empty array into the provided buffer. */ static inline int rte_tel_json_empty_array(char *buf, const int len, const int used) From patchwork Wed Apr 5 15:44:13 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bruce Richardson X-Patchwork-Id: 125816 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id F24C4428D4; Wed, 5 Apr 2023 17:45:01 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 8602742D2D; Wed, 5 Apr 2023 17:44:39 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mails.dpdk.org (Postfix) with ESMTP id 34D6E42D13 for ; Wed, 5 Apr 2023 17:44:38 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1680709478; x=1712245478; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=fNu55pEca/MaDdDy6qY7lvnjZ3nahCffEj5thK9tnxc=; b=DdPGScRXsD1kG22AirNwtPygB60n2pX8U0XenyttPXAw77qXD+RR4Y9B pJSIOqhNawrMmZxeM10WPUiNaOOU01zaoknVWDbvP2fI3q62aRCzyWjB8 KwLENfR5HAHX0oqMqc4Cnp8KB7xzHmb/6V5ci0kGTDXadsiCsYc2sqItm ia8rn9miNqcoUp16eKIqcR9v73X/DWUZww4tUnGGJQqfr6hG9pLPkyAJL 6qw8A5ObeUX0Drdf0i8Kydun/kBVIKiQJEOIYisLs5xRcTcfN9N7jy6sG Lsbs+8d5n7M3Ip1kLhnoFZtm/mwf1oRSxJ1ryABzzVs+xOS4Ux4ESVxyx Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="339980393" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="339980393" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Apr 2023 08:44:37 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="686790302" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="686790302" Received: from silpixa00401385.ir.intel.com ([10.237.214.40]) by orsmga002.jf.intel.com with ESMTP; 05 Apr 2023 08:44:36 -0700 From: Bruce Richardson To: dev@dpdk.org Cc: ciara.power@intel.com, roretzla@linux.microsoft.com, Bruce Richardson Subject: [PATCH v2 4/5] telemetry: rename local variables Date: Wed, 5 Apr 2023 16:44:13 +0100 Message-Id: <20230405154414.183915-5-bruce.richardson@intel.com> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20230405154414.183915-1-bruce.richardson@intel.com> References: <20230310181836.162336-1-bruce.richardson@intel.com> <20230405154414.183915-1-bruce.richardson@intel.com> MIME-Version: 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org In the newly separated out function, rename "tmp" to "buf" to have more meaningful variable names. Signed-off-by: Bruce Richardson --- When committing, this patch can be merged with the previous. I've kept them separate for now, as it makes reviewing a lot easier. --- lib/telemetry/telemetry_json.h | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/telemetry/telemetry_json.h b/lib/telemetry/telemetry_json.h index 4c1941ad15..4d725d938b 100644 --- a/lib/telemetry/telemetry_json.h +++ b/lib/telemetry/telemetry_json.h @@ -80,44 +80,44 @@ static const char control_chars[0x20] = { * directly, but returns 0 on overflow. Otherwise returns number of chars written to buffer. */ static inline int -__json_format_str_to_buf(char *tmp, const int len, +__json_format_str_to_buf(char *buf, const int len, const char *prefix, const char *str, const char *suffix) { - int tmpidx = 0; + int bufidx = 0; - while (*prefix != '\0' && tmpidx < len) - tmp[tmpidx++] = *prefix++; - if (tmpidx >= len) + while (*prefix != '\0' && bufidx < len) + buf[bufidx++] = *prefix++; + if (bufidx >= len) return 0; while (*str != '\0') { if (*str < (int)RTE_DIM(control_chars)) { int idx = *str; /* compilers don't like char type as index */ if (control_chars[idx] != 0) { - tmp[tmpidx++] = '\\'; - tmp[tmpidx++] = control_chars[idx]; + buf[bufidx++] = '\\'; + buf[bufidx++] = control_chars[idx]; } } else if (*str == '"' || *str == '\\') { - tmp[tmpidx++] = '\\'; - tmp[tmpidx++] = *str; + buf[bufidx++] = '\\'; + buf[bufidx++] = *str; } else - tmp[tmpidx++] = *str; + buf[bufidx++] = *str; /* we always need space for (at minimum) closing quote and null character. * Ensuring at least two free characters also means we can always take an * escaped character like "\n" without overflowing */ - if (tmpidx > len - 2) + if (bufidx > len - 2) return 0; str++; } - while (*suffix != '\0' && tmpidx < len) - tmp[tmpidx++] = *suffix++; - if (tmpidx >= len) + while (*suffix != '\0' && bufidx < len) + buf[bufidx++] = *suffix++; + if (bufidx >= len) return 0; - tmp[tmpidx] = '\0'; - return tmpidx; + buf[bufidx] = '\0'; + return bufidx; } /** From patchwork Wed Apr 5 15:44:14 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bruce Richardson X-Patchwork-Id: 125817 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id BD615428D4; Wed, 5 Apr 2023 17:45:08 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C28BA42D0C; Wed, 5 Apr 2023 17:44:41 +0200 (CEST) Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mails.dpdk.org (Postfix) with ESMTP id 7DE3A42D0C for ; Wed, 5 Apr 2023 17:44:40 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1680709480; x=1712245480; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=3AkT5t0n+8KM11Lio4V7yr97BYKxeiFJwttdaTNR43I=; b=NRAG5+9wghQj09hpgNe4TmlAXT9o9BGCjv36KM+l8BmaL0V3ak5HTf+G cc6DZ6F7qukGyIm9/S8EQAgpHLZbQpD9YsAzv7BmiZHcF3Y23dGtL2BGR iH+D/CES1xUpMbiQJpiiWipqmO297b3aQN1UTR1xmT0Vl72+P+fYLidpc flchVcwdTKqGpR5lIVj5lq2xjfGZt+AeAtgl5cpPoopUHhsDKF1KDbp29 bxbmtWoSWM4y5IuCtSxtE78iGs9uvL5The1/iw2oZYJ5xvKZjsR7JvoRC yHHcZRURTxKM3zrht/b+3sjhvHq0aSCXVzM06aEBNsOSOozXhbhv7L3w+ A==; X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="339980405" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="339980405" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Apr 2023 08:44:40 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="686790315" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="686790315" Received: from silpixa00401385.ir.intel.com ([10.237.214.40]) by orsmga002.jf.intel.com with ESMTP; 05 Apr 2023 08:44:38 -0700 From: Bruce Richardson To: dev@dpdk.org Cc: ciara.power@intel.com, roretzla@linux.microsoft.com, Bruce Richardson Subject: [PATCH v2 5/5] telemetry: remove VLA in json string format function Date: Wed, 5 Apr 2023 16:44:14 +0100 Message-Id: <20230405154414.183915-6-bruce.richardson@intel.com> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20230405154414.183915-1-bruce.richardson@intel.com> References: <20230310181836.162336-1-bruce.richardson@intel.com> <20230405154414.183915-1-bruce.richardson@intel.com> MIME-Version: 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Since variable length arrays (VLAs) are potentially unsecure and unsupported by some compilers, rework the code to remove their use. As with previous changes to remove VLAs in the telemetry code, this function uses two methods to avoid modifying the buffer when adding to it fails: * if there are only a few characters in the buffer, save them off to restore on failure, then use the buffer as-is, * otherwise use malloc rather than a VLA to allocate a temporary buffer and copy from that on success only. Signed-off-by: Bruce Richardson --- app/test/test_telemetry_json.c | 2 +- lib/telemetry/telemetry_json.h | 19 +++++++++++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/app/test/test_telemetry_json.c b/app/test/test_telemetry_json.c index e81e3a8a98..5617eac540 100644 --- a/app/test/test_telemetry_json.c +++ b/app/test/test_telemetry_json.c @@ -129,7 +129,7 @@ test_string_char_escaping(void) { static const char str[] = "A string across\ntwo lines and \"with quotes\"!"; const char *expected = "\"A string across\\ntwo lines and \\\"with quotes\\\"!\""; - char buf[sizeof(str) + 10]; + char buf[sizeof(str) + 10] = ""; int used = 0; used = rte_tel_json_str(buf, sizeof(buf), used, str); diff --git a/lib/telemetry/telemetry_json.h b/lib/telemetry/telemetry_json.h index 4d725d938b..fceff91842 100644 --- a/lib/telemetry/telemetry_json.h +++ b/lib/telemetry/telemetry_json.h @@ -130,13 +130,28 @@ __json_format_str_to_buf(char *buf, const int len, static inline int __json_format_str(char *buf, const int len, const char *prefix, const char *str, const char *suffix) { - char tmp[len]; int ret; + char saved[4] = ""; + char *tmp; + + if (strnlen(buf, sizeof(saved)) < sizeof(saved)) { + /* we have only a few bytes in buffer, so save them off to restore on error*/ + strcpy(saved, buf); + ret = __json_format_str_to_buf(buf, len, prefix, str, suffix); + if (ret == 0) + strcpy(buf, saved); /* restore */ + return ret; + } + + tmp = malloc(len); + if (tmp == NULL) + return 0; ret = __json_format_str_to_buf(tmp, len, prefix, str, suffix); if (ret > 0) - strcpy(buf, tmp); + strcpy(buf, saved); + free(tmp); return ret; }