From: Bruce Richardson <bruce.richardson@intel.com>
To: dev@dpdk.org
Cc: ciara.power@intel.com, roretzla@linux.microsoft.com,
Bruce Richardson <bruce.richardson@intel.com>
Subject: [PATCH v3 2/5] telemetry: remove variable length array in printf fn
Date: Wed, 5 Apr 2023 17:03:23 +0100 [thread overview]
Message-ID: <20230405160326.186921-3-bruce.richardson@intel.com> (raw)
In-Reply-To: <20230405160326.186921-1-bruce.richardson@intel.com>
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 <bruce.richardson@intel.com>
---
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 <inttypes.h>
#include <stdarg.h>
#include <stdio.h>
+#include <stdlib.h>
#include <rte_common.h>
#include <rte_telemetry.h>
@@ -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
next prev parent reply other threads:[~2023-04-05 16:05 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-10 18:18 [PATCH] telemetry: fix autotest failures on Alpine Bruce Richardson
2023-03-10 19:08 ` Stephen Hemminger
2023-03-13 9:38 ` Bruce Richardson
2023-04-05 15:44 ` [PATCH v2 0/5] telemetry: remove variable length arrays Bruce Richardson
2023-04-05 15:44 ` [PATCH v2 1/5] telemetry: fix autotest failures on Alpine Bruce Richardson
2023-04-07 19:21 ` Tyler Retzlaff
2023-04-11 8:43 ` Bruce Richardson
2023-04-05 15:44 ` [PATCH v2 2/5] telemetry: remove variable length array in printf fn Bruce Richardson
2023-04-05 15:44 ` [PATCH v2 3/5] telemetry: split out body of json string format fn Bruce Richardson
2023-04-05 15:44 ` [PATCH v2 4/5] telemetry: rename local variables Bruce Richardson
2023-04-05 15:44 ` [PATCH v2 5/5] telemetry: remove VLA in json string format function Bruce Richardson
2023-04-05 16:03 ` [PATCH v3 0/5] telemetry: remove variable length arrays Bruce Richardson
2023-04-05 16:03 ` [PATCH v3 1/5] telemetry: fix autotest failures on Alpine Bruce Richardson
2023-04-07 19:22 ` Tyler Retzlaff
2023-04-05 16:03 ` Bruce Richardson [this message]
2023-04-07 19:25 ` [PATCH v3 2/5] telemetry: remove variable length array in printf fn Tyler Retzlaff
2023-04-05 16:03 ` [PATCH v3 3/5] telemetry: split out body of json string format fn Bruce Richardson
2023-04-07 19:28 ` Tyler Retzlaff
2023-04-05 16:03 ` [PATCH v3 4/5] telemetry: rename local variables Bruce Richardson
2023-04-07 19:50 ` Tyler Retzlaff
2023-04-11 8:58 ` Bruce Richardson
2023-04-05 16:03 ` [PATCH v3 5/5] telemetry: remove VLA in json string format function Bruce Richardson
2023-04-07 19:54 ` Tyler Retzlaff
2023-05-25 7:12 ` David Marchand
2023-05-24 20:47 ` [PATCH v3 0/5] telemetry: remove variable length arrays Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230405160326.186921-3-bruce.richardson@intel.com \
--to=bruce.richardson@intel.com \
--cc=ciara.power@intel.com \
--cc=dev@dpdk.org \
--cc=roretzla@linux.microsoft.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).