[v3,5/5] telemetry: remove VLA in json string format function

  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 <bruce.richardson@intel.com>
---
 app/test/test_telemetry_json.c |  2 +-
 lib/telemetry/telemetry_json.h | 19 +++++++++++++++++--
 2 files changed, 18 insertions(+), 3 deletions(-)
  

On Wed, Apr 05, 2023 at 05:03:26PM +0100, Bruce Richardson wrote:
> 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 <bruce.richardson@intel.com>
> ---

Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
  

On Wed, Apr 5, 2023 at 6:05 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> 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 <bruce.richardson@intel.com>
> ---
>  app/test/test_telemetry_json.c |  2 +-
>  lib/telemetry/telemetry_json.h | 19 +++++++++++++++++--
>  2 files changed, 18 insertions(+), 3 deletions(-)

This change triggers a unit test failure ("interestingly" only with
gcc, I can't reproduce with clang).

$ ninja -C build-gcc && ./build-gcc/app/test/dpdk-test --no-huge -m
2048 --iova=va -- telemetry_json_autotest
ninja: Entering directory `build-gcc'
ninja: no work to do.
EAL: Detected CPU lcores: 16
EAL: Detected NUMA nodes: 1
EAL: Detected shared linkage of DPDK
EAL: Multi-process socket /run/user/114840/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'VA'
APP: HPET is not enabled, using TSC as default timer
RTE>>telemetry_json_autotest
test_basic_array: buf = '["meaning of life",42]', expected =
'["meaning of life",42]'
OK
test_basic_obj: buf = '{"weddings":4,"funerals":1}', expected =
'{"weddings":4,"funerals":1}'
OK
test_overflow_array: buf = '', expected = '["Arsenal","Chelsea"]'
ERROR
Test Failed


I guess we need:

diff --git a/lib/telemetry/telemetry_json.h b/lib/telemetry/telemetry_json.h
index 7999535848..7a246deacb 100644
--- a/lib/telemetry/telemetry_json.h
+++ b/lib/telemetry/telemetry_json.h
@@ -153,7 +153,7 @@ __json_format_str(char *buf, const int len, const
char *prefix, const char *str,

        ret = __json_format_str_to_buf(tmp, len, prefix, str, suffix);
        if (ret > 0)
-               strcpy(buf, saved);
+               strcpy(buf, tmp);

        free(tmp);
        return ret;