From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 Content-Transfer-Encoding: 8bit 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 --- 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 */ } -- 2.37.2