From patchwork Wed Apr 5 16:03:22 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bruce Richardson X-Patchwork-Id: 125819 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 706CD428D4; Wed, 5 Apr 2023 18:04:44 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 608C842B8C; Wed, 5 Apr 2023 18:04:44 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mails.dpdk.org (Postfix) with ESMTP id 45463427E9; Wed, 5 Apr 2023 18:04:42 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1680710682; x=1712246682; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=vyG6oyrJvzCQDUTMUwfJrdN9vsdcCdtrQddmeHIlMYc=; b=a83PFlhZj2aEoR7Yq5W/3T6qwd2QiaYhpEHIsZMQbC8pqa102kMB6l3/ p7CRENd5lo6a0o2IjEtK5xkhL9gkLXyYyzDrgxq8Kef1tZKt+TgSZlv7u TDVCCI9L3p+NIu+wERVon/NZg38cgR976QnzARaRnNNJ0DBbUwKKw+y33 CFzbbFm9LwjTEVDQcq96W5HoQcZxFN6SBO005DJo1sTfEMNQCrJBUZUB+ H/j1/aKPKISotkdmhrKEbIG9sjmtgSL5gW1n1mi0YpTYKME0pO1BfZWlU 1jF6GyjRIn0W9lVW6HuWPTghTWjXTrPyX0YAawrndsIMFHk/gBo9a3hdz w==; X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="341218587" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="341218587" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Apr 2023 09:04:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="830405799" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="830405799" Received: from silpixa00401385.ir.intel.com ([10.237.214.40]) by fmsmga001.fm.intel.com with ESMTP; 05 Apr 2023 09:04:16 -0700 From: Bruce Richardson To: dev@dpdk.org Cc: ciara.power@intel.com, roretzla@linux.microsoft.com, Bruce Richardson , stable@dpdk.org Subject: [PATCH v3 1/5] telemetry: fix autotest failures on Alpine Date: Wed, 5 Apr 2023 17:03:22 +0100 Message-Id: <20230405160326.186921-2-bruce.richardson@intel.com> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20230405160326.186921-1-bruce.richardson@intel.com> References: <20230310181836.162336-1-bruce.richardson@intel.com> <20230405160326.186921-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 16:03:23 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bruce Richardson X-Patchwork-Id: 125821 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 7B2B4428D4; Wed, 5 Apr 2023 18:05:02 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 212CC42D2D; Wed, 5 Apr 2023 18:04:48 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mails.dpdk.org (Postfix) with ESMTP id D551842D2C for ; Wed, 5 Apr 2023 18:04:45 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1680710686; x=1712246686; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=npRz5H/BKu267RzOeanxaLE8mYLL2/vjfd6CkcJEg44=; b=k5FfuajgmFkIYmAk7dBQBC1TPShAWYi8kqP9zaoRY5a1Ite3RBXj9jOp xEctLnG5wY8JP4wCZzReSH0kJ0eDwOMa5QwRP3F1YvAbf5ddiwW/8Pmgx icWRwtu5Tne/LD+9UV3UBWn2jMkNOfgxhKlTYbioiwKvpZ983jNFdOB4f xymkCtWekcNWsHzON8K4ECsdIfoB+5ClhYPKomKOoINLLvRTjN/NnM/Mu Rat4sULfuROLWSnWICQVVWvCMHsYLuyqKVnZu39HcGo2AjWh0mM0jf0Ey fQycv/XzpcXqUz6CE79Uq1XXKuRl6DG9Ji0OWvhkBFhnImKoXGb3dOXlD w==; X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="341218596" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="341218596" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Apr 2023 09:04:19 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="830405808" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="830405808" Received: from silpixa00401385.ir.intel.com ([10.237.214.40]) by fmsmga001.fm.intel.com with ESMTP; 05 Apr 2023 09:04:18 -0700 From: Bruce Richardson To: dev@dpdk.org Cc: ciara.power@intel.com, roretzla@linux.microsoft.com, Bruce Richardson Subject: [PATCH v3 2/5] telemetry: remove variable length array in printf fn Date: Wed, 5 Apr 2023 17:03:23 +0100 Message-Id: <20230405160326.186921-3-bruce.richardson@intel.com> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20230405160326.186921-1-bruce.richardson@intel.com> References: <20230310181836.162336-1-bruce.richardson@intel.com> <20230405160326.186921-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 Acked-by: Tyler Retzlaff --- v3: remove use of non-standard vasprintf --- lib/telemetry/telemetry_json.h | 36 ++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/lib/telemetry/telemetry_json.h b/lib/telemetry/telemetry_json.h index 744bbfe053..1bddd124f9 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,44 @@ __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 + */ + newbuf = malloc(len); + if (newbuf == NULL) + return 0; + va_start(ap, format); - ret = vsnprintf(tmp, sizeof(tmp), format, ap); + ret = vsnprintf(newbuf, len, 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 16:03:24 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bruce Richardson X-Patchwork-Id: 125820 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 C4B23428D4; Wed, 5 Apr 2023 18:04:54 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 23B2042D17; Wed, 5 Apr 2023 18:04:47 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mails.dpdk.org (Postfix) with ESMTP id 0310D42C76 for ; Wed, 5 Apr 2023 18:04:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1680710685; x=1712246685; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=UPZTfqHBNQu8Jg6cu1IrchoURK1D6gcL/1fIHkBd+8E=; b=hDKWtXuWZixeCTG1tJI045Sfpko4cPUMYT4PL0wEPwWzF/FcCvVpGG0y 4bALnw1w5qQMn/KhwSMKkTkdH2t58npkn4y+DiBUeU7lIfrF5nqdcE0pF cs+N8OyxrK4hsnMa8fI+UjXqsPuy+1xulipPkRshTgQVjiHi7a3QKxc71 RQBbAjZM6G+7GcHZOvrS5CkBaxw4vJdDHt6RGdhhvCv6rzigfa1KyuWSb d+VWFqpBYWvX8MXaoR+Iz5dPzjb+F6odkqn1DQJ16+v4IV8ZH3vBxboTZ MohccySA5WsGM/r95uO4O1j9feiMA3gyRXy0NBxxlm56s67G4AabC350K g==; X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="341218605" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="341218605" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Apr 2023 09:04:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="830405816" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="830405816" Received: from silpixa00401385.ir.intel.com ([10.237.214.40]) by fmsmga001.fm.intel.com with ESMTP; 05 Apr 2023 09:04:19 -0700 From: Bruce Richardson To: dev@dpdk.org Cc: ciara.power@intel.com, roretzla@linux.microsoft.com, Bruce Richardson Subject: [PATCH v3 3/5] telemetry: split out body of json string format fn Date: Wed, 5 Apr 2023 17:03:24 +0100 Message-Id: <20230405160326.186921-4-bruce.richardson@intel.com> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20230405160326.186921-1-bruce.richardson@intel.com> References: <20230310181836.162336-1-bruce.richardson@intel.com> <20230405160326.186921-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 Acked-by: Tyler Retzlaff --- 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 1bddd124f9..aada523a27 100644 --- a/lib/telemetry/telemetry_json.h +++ b/lib/telemetry/telemetry_json.h @@ -80,15 +80,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) @@ -123,11 +121,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 16:03:25 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bruce Richardson X-Patchwork-Id: 125822 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 3D9AF428D4; Wed, 5 Apr 2023 18:05:10 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 41EF942D3A; Wed, 5 Apr 2023 18:04:49 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mails.dpdk.org (Postfix) with ESMTP id 9506042C24 for ; Wed, 5 Apr 2023 18:04:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1680710686; x=1712246686; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=kmwz0JZLMz/PYB9q+460ZarpveznP3XHVPXKHnvqjbk=; b=NAhzmwV87OL+o/3xLXeBUUesHUhFaOH8zVEmyqQ8FbHnnlT9YpR1xDWN EaFlTLcP+x664KejE5HudjEbjX2nvQFNq5CLwr4dwDQL9nslwws/w8IOD oFYh/GNuHJYmjkRtYNitWCSgZkuWd9yfwn578DHAMApoU6vvWCm9as+lQ qE2JiTyNwytutP5IQrqRrMsDS2c/lgY9TzJ2+n2X+1e21BZwDdr2rtutr ExBsic2AzMhohZErbB2RGJghVrCWCnIyzrB5auJLCYZoQSBghJRGUXlfV kMa0khW42aiPYA+CYhf9C8VFKoXiZRmmeh1N8Y6Ink4YcYcmlYeSZmnOQ Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="341218622" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="341218622" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Apr 2023 09:04:21 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="830405825" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="830405825" Received: from silpixa00401385.ir.intel.com ([10.237.214.40]) by fmsmga001.fm.intel.com with ESMTP; 05 Apr 2023 09:04:20 -0700 From: Bruce Richardson To: dev@dpdk.org Cc: ciara.power@intel.com, roretzla@linux.microsoft.com, Bruce Richardson Subject: [PATCH v3 4/5] telemetry: rename local variables Date: Wed, 5 Apr 2023 17:03:25 +0100 Message-Id: <20230405160326.186921-5-bruce.richardson@intel.com> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20230405160326.186921-1-bruce.richardson@intel.com> References: <20230310181836.162336-1-bruce.richardson@intel.com> <20230405160326.186921-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 Acked-by: Tyler Retzlaff --- 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 aada523a27..c087b833eb 100644 --- a/lib/telemetry/telemetry_json.h +++ b/lib/telemetry/telemetry_json.h @@ -84,44 +84,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 16:03:26 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bruce Richardson X-Patchwork-Id: 125823 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 8B8E1428D4; Wed, 5 Apr 2023 18:05:18 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5364242D3D; Wed, 5 Apr 2023 18:04:52 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mails.dpdk.org (Postfix) with ESMTP id 7AED542D3F for ; Wed, 5 Apr 2023 18:04:50 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1680710690; x=1712246690; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=lD4rRLY1FW6QSObnxnPfchXl6ww0aUf8D51zJ97VRdU=; b=Nr+DBkwxuNKt9za5pvu3mCjkli3lAVOU/4RTS17bKA9yDyVV2GSf5Mxk dSbdn5B/+N6uO7GVvNaHoDIMalXxLbj3VySNzMFTawRp3mtO6WRS7fYgL jqCUP3sj3T6m943+dK8RU0D0k53OYmXfOmxzgokfa/AyVrsAnCcAPjrTy YIZtXMFJZdnKbpj1TvlWJkJCWlpeWhXI45iE+lCx/YAJL11R9lcJsZrB7 cSth0jmzC/+8lwP5b8Vjx/VEqUVyDcpzsaCXw8vC0Hg5Qw1PIhqLNU+2B Qgt/0blC+2iwnE2a8AfiKQ1rO94R0RlJwkvQwCIvZRZ8e3tut4oxaWMhD A==; X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="341218633" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="341218633" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Apr 2023 09:04:23 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10671"; a="830405832" X-IronPort-AV: E=Sophos;i="5.98,321,1673942400"; d="scan'208";a="830405832" Received: from silpixa00401385.ir.intel.com ([10.237.214.40]) by fmsmga001.fm.intel.com with ESMTP; 05 Apr 2023 09:04:22 -0700 From: Bruce Richardson To: dev@dpdk.org Cc: ciara.power@intel.com, roretzla@linux.microsoft.com, Bruce Richardson Subject: [PATCH v3 5/5] telemetry: remove VLA in json string format function Date: Wed, 5 Apr 2023 17:03:26 +0100 Message-Id: <20230405160326.186921-6-bruce.richardson@intel.com> X-Mailer: git-send-email 2.37.2 In-Reply-To: <20230405160326.186921-1-bruce.richardson@intel.com> References: <20230310181836.162336-1-bruce.richardson@intel.com> <20230405160326.186921-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 insecure 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 Acked-by: Tyler Retzlaff --- 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 c087b833eb..7999535848 100644 --- a/lib/telemetry/telemetry_json.h +++ b/lib/telemetry/telemetry_json.h @@ -134,13 +134,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; }