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 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 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 --- 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 */ } -- 2.37.2