On 4/12/22 15:29, Dumitru Ceara wrote:
When compiled with '-fsanitize=undefined' running 'ovsdb-client
--timestamp monitor Open_vSwitch' in a sandbox triggers the following
undefined behavior (flagged by UBSan):

   lib/dynamic-string.c:207:38: runtime error: applying zero offset to null 
pointer
       #0 0x196c05b in ds_put_strftime_msec lib/dynamic-string.c:207:38
       #1 0x196c2aa in xastrftime_msec lib/dynamic-string.c:225:5
       #2 0x1935f33 in pmd_info_show_perf lib/dpif-netdev.c:799:17
       #3 0x1935f33 in dpif_netdev_pmd_info lib/dpif-netdev.c:1487:13
       #4 0x1938155 in pmd_perf_show_cmd lib/dpif-netdev.c:1537:5
       #5 0x1d7634b in process_command lib/unixctl.c:310:13
       #6 0x1d7634b in run_connection lib/unixctl.c:344:17
       #7 0x1d7634b in unixctl_server_run lib/unixctl.c:395:21
       #8 0x168b5bd in main vswitchd/ovs-vswitchd.c:130:9
       #9 0x7f6fa2fd8492 in __libc_start_main (/lib64/libc.so.6+0x23492)
       #10 0xe6911d in _start (vswitchd/ovs-vswitchd+0xe6911d)

Reported-by: Ilya Maximets <i.maxim...@ovn.org>
Signed-off-by: Dumitru Ceara <dce...@redhat.com>
---
  lib/dynamic-string.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/dynamic-string.c b/lib/dynamic-string.c
index fd0127ed1740..3d415af4f4e0 100644
--- a/lib/dynamic-string.c
+++ b/lib/dynamic-string.c
@@ -200,8 +200,8 @@ ds_put_strftime_msec(struct ds *ds, const char *template, 
long long int when,
for (;;) {
          size_t avail = ds->string ? ds->allocated - ds->length + 1 : 0;
-        size_t used = strftime_msec(&ds->string[ds->length], avail, template,
-                                    &tm);
+        char *dest = ds->string ? &ds->string[ds->length] : NULL;
+        size_t used = strftime_msec(dest, avail, template, &tm);
          if (used) {
              ds->length += used;
              return;

The code looks OK to me. However, calling strftime_msec(NULL, 0,...) once if the dynamic string is empty seems to me a bit obfuscated.

I would personally just add:

if (!ds->string) {
     ds_reserve(ds, 64);
}

before the loop to mentally get rid of that corner case.

In any case:

Acked-by: Adrian Moreno <amore...@redhat.com>

--
Adrián Moreno

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to