DPDK patches and discussions
 help / color / mirror / Atom feed
* BUG: AddressSanitizer reports a buffer-overflow on rte_hash_lookup
@ 2023-02-05 16:54 Isaac Boukris
  2023-02-05 19:49 ` Stephen Hemminger
  0 siblings, 1 reply; 4+ messages in thread
From: Isaac Boukris @ 2023-02-05 16:54 UTC (permalink / raw)
  To: dev

[-- Attachment #1: Type: text/plain, Size: 2023 bytes --]

Hi,

I managed to reproduce it by modifying the helloworld app (see
attached). The report seem correct, as in case of 10 byte key the code
tries to look at the key as uint32 array and access k[2] which is two
bytes over, see:
https://github.com/DPDK/dpdk/blob/0bf5832222971a0154c9150d4a7a4b82ecbc9ddb/lib/hash/rte_jhash.h#L118

$ sudo build/helloworld --iova-mode=pa
EAL: Detected CPU lcores: 8
EAL: Detected NUMA nodes: 1
EAL: Detected static linkage of DPDK
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'PA'
EAL: VFIO support initialized
EAL: Using IOMMU type 1 (Type 1)
EAL: Ignore mapping IO port bar(3)
EAL: Probe PCI driver: net_vmxnet3 (15ad:7b0) device: 0000:0b:00.0 (socket -1)
=================================================================
==21410==ERROR: AddressSanitizer: global-buffer-overflow on address
0x0000024fe428 at pc 0x000001293b0b bp 0x7fff126ef2d0 sp
0x7fff126ef2c0
READ of size 4 at 0x0000024fe428 thread T0
    #0 0x1293b0a in __rte_jhash_2hashes
(/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x1293b0a)
    #1 0x12953bf in rte_jhash_2hashes
(/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x12953bf)
    #2 0x12954c8 in rte_jhash
(/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x12954c8)
    #3 0x1bd7168 in rte_hash_lookup
(/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x1bd7168)
    #4 0x1295600 in main
(/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x1295600)
    #5 0x7fe8fffbbd84 in __libc_start_main (/lib64/libc.so.6+0x3ad84)
    #6 0x129356d in _start
(/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x129356d)

0x0000024fe42a is located 0 bytes to the right of global variable
'hash_key' defined in 'main.c:34:13' (0x24fe420) of size 10
SUMMARY: AddressSanitizer: global-buffer-overflow
(/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x1293b0a)
in __rte_jhash_2hashes

[-- Attachment #2: Demo-bug-in-rte_hash_lookup.patch --]
[-- Type: text/x-patch, Size: 2451 bytes --]

From 44a74ac537fbee031bedda74fa05099f3fd3f552 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris@gmail.com>
Date: Sun, 5 Feb 2023 11:20:29 +0200
Subject: [PATCH] Demo bug in rte_hash_lookup

---
 examples/helloworld/Makefile |  2 +-
 examples/helloworld/main.c   | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/examples/helloworld/Makefile b/examples/helloworld/Makefile
index 2a6a2f1527..14e44b8aa8 100644
--- a/examples/helloworld/Makefile
+++ b/examples/helloworld/Makefile
@@ -22,7 +22,7 @@ static: build/$(APP)-static
        ln -sf $(APP)-static build/$(APP)

 PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
-CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
+CFLAGS += -O0 -fsanitize=address $(shell $(PKGCONF) --cflags libdpdk)
 LDFLAGS_SHARED = $(shell $(PKGCONF) --libs libdpdk)
 LDFLAGS_STATIC = $(shell $(PKGCONF) --static --libs libdpdk)

diff --git a/examples/helloworld/main.c b/examples/helloworld/main.c
index af509138da..7460fbdfea 100644
--- a/examples/helloworld/main.c
+++ b/examples/helloworld/main.c
@@ -15,6 +15,11 @@
 #include <rte_lcore.h>
 #include <rte_debug.h>

+#include <assert.h>
+#include <rte_hash.h>
+#include <rte_fbk_hash.h>
+#include <rte_jhash.h>
+
 /* Launch a function on lcore. 8< */
 static int
 lcore_hello(__rte_unused void *arg)
@@ -26,18 +31,36 @@ lcore_hello(__rte_unused void *arg)
 }
 /* >8 End of launching function on lcore. */

+static char hash_key[10] = "";
+
+static struct rte_hash_parameters h_params = {
+       .entries = 64,
+       .key_len = sizeof(hash_key),
+       .hash_func = rte_jhash,
+       .hash_func_init_val = 0,
+       .socket_id = 0,
+};
+
 /* Initialization of Environment Abstraction Layer (EAL). 8< */
 int
 main(int argc, char **argv)
 {
        int ret;
        unsigned lcore_id;
+       struct rte_hash *handle;
+       int pos;

        ret = rte_eal_init(argc, argv);
        if (ret < 0)
                rte_panic("Cannot init EAL\n");
        /* >8 End of initialization of Environment Abstraction Layer */

+       handle = rte_hash_create(&h_params);
+       assert(handle != NULL);
+
+       pos = rte_hash_lookup(handle, &hash_key);
+       assert(pos == -ENOENT);
+
        /* Launches the function on each lcore. 8< */
        RTE_LCORE_FOREACH_WORKER(lcore_id) {
                /* Simpler equivalent. 8< */
--
2.31.1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: BUG: AddressSanitizer reports a buffer-overflow on rte_hash_lookup
  2023-02-05 16:54 BUG: AddressSanitizer reports a buffer-overflow on rte_hash_lookup Isaac Boukris
@ 2023-02-05 19:49 ` Stephen Hemminger
  2023-02-05 20:14   ` Isaac Boukris
  0 siblings, 1 reply; 4+ messages in thread
From: Stephen Hemminger @ 2023-02-05 19:49 UTC (permalink / raw)
  To: Isaac Boukris; +Cc: dev

On Sun, 5 Feb 2023 18:54:20 +0200
Isaac Boukris <iboukris@gmail.com> wrote:

> Hi,
> 
> I managed to reproduce it by modifying the helloworld app (see
> attached). The report seem correct, as in case of 10 byte key the code
> tries to look at the key as uint32 array and access k[2] which is two
> bytes over, see:
> https://github.com/DPDK/dpdk/blob/0bf5832222971a0154c9150d4a7a4b82ecbc9ddb/lib/hash/rte_jhash.h#L118
> 
> $ sudo build/helloworld --iova-mode=pa
> EAL: Detected CPU lcores: 8
> EAL: Detected NUMA nodes: 1
> EAL: Detected static linkage of DPDK
> EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
> EAL: Selected IOVA mode 'PA'
> EAL: VFIO support initialized
> EAL: Using IOMMU type 1 (Type 1)
> EAL: Ignore mapping IO port bar(3)
> EAL: Probe PCI driver: net_vmxnet3 (15ad:7b0) device: 0000:0b:00.0 (socket -1)
> =================================================================
> ==21410==ERROR: AddressSanitizer: global-buffer-overflow on address
> 0x0000024fe428 at pc 0x000001293b0b bp 0x7fff126ef2d0 sp
> 0x7fff126ef2c0
> READ of size 4 at 0x0000024fe428 thread T0
>     #0 0x1293b0a in __rte_jhash_2hashes
> (/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x1293b0a)
>     #1 0x12953bf in rte_jhash_2hashes
> (/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x12953bf)
>     #2 0x12954c8 in rte_jhash
> (/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x12954c8)
>     #3 0x1bd7168 in rte_hash_lookup
> (/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x1bd7168)
>     #4 0x1295600 in main
> (/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x1295600)
>     #5 0x7fe8fffbbd84 in __libc_start_main (/lib64/libc.so.6+0x3ad84)
>     #6 0x129356d in _start
> (/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x129356d)
> 
> 0x0000024fe42a is located 0 bytes to the right of global variable
> 'hash_key' defined in 'main.c:34:13' (0x24fe420) of size 10
> SUMMARY: AddressSanitizer: global-buffer-overflow
> (/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x1293b0a)
> in __rte_jhash_2hashes

This code is using the common optimization of doing a full 32 bit access
and masking the result. This will read past the end of the passed input
but ignore the extra bytes. It won't be a problem unless the application
goes out of its way to put a hash key value at the end of a mapped
region.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: BUG: AddressSanitizer reports a buffer-overflow on rte_hash_lookup
  2023-02-05 19:49 ` Stephen Hemminger
@ 2023-02-05 20:14   ` Isaac Boukris
  2023-02-05 21:08     ` Stephen Hemminger
  0 siblings, 1 reply; 4+ messages in thread
From: Isaac Boukris @ 2023-02-05 20:14 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Sun, Feb 5, 2023 at 9:49 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Sun, 5 Feb 2023 18:54:20 +0200
> Isaac Boukris <iboukris@gmail.com> wrote:
>
> > Hi,
> >
> > I managed to reproduce it by modifying the helloworld app (see
> > attached). The report seem correct, as in case of 10 byte key the code
> > tries to look at the key as uint32 array and access k[2] which is two
> > bytes over, see:
> > https://github.com/DPDK/dpdk/blob/0bf5832222971a0154c9150d4a7a4b82ecbc9ddb/lib/hash/rte_jhash.h#L118
> >
> > $ sudo build/helloworld --iova-mode=pa
> > EAL: Detected CPU lcores: 8
> > EAL: Detected NUMA nodes: 1
> > EAL: Detected static linkage of DPDK
> > EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
> > EAL: Selected IOVA mode 'PA'
> > EAL: VFIO support initialized
> > EAL: Using IOMMU type 1 (Type 1)
> > EAL: Ignore mapping IO port bar(3)
> > EAL: Probe PCI driver: net_vmxnet3 (15ad:7b0) device: 0000:0b:00.0 (socket -1)
> > =================================================================
> > ==21410==ERROR: AddressSanitizer: global-buffer-overflow on address
> > 0x0000024fe428 at pc 0x000001293b0b bp 0x7fff126ef2d0 sp
> > 0x7fff126ef2c0
> > READ of size 4 at 0x0000024fe428 thread T0
> >     #0 0x1293b0a in __rte_jhash_2hashes
> > (/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x1293b0a)
> >     #1 0x12953bf in rte_jhash_2hashes
> > (/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x12953bf)
> >     #2 0x12954c8 in rte_jhash
> > (/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x12954c8)
> >     #3 0x1bd7168 in rte_hash_lookup
> > (/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x1bd7168)
> >     #4 0x1295600 in main
> > (/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x1295600)
> >     #5 0x7fe8fffbbd84 in __libc_start_main (/lib64/libc.so.6+0x3ad84)
> >     #6 0x129356d in _start
> > (/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x129356d)
> >
> > 0x0000024fe42a is located 0 bytes to the right of global variable
> > 'hash_key' defined in 'main.c:34:13' (0x24fe420) of size 10
> > SUMMARY: AddressSanitizer: global-buffer-overflow
> > (/home/admin/dpdk/share/dpdk/examples/helloworld/build/helloworld-static+0x1293b0a)
> > in __rte_jhash_2hashes
>
> This code is using the common optimization of doing a full 32 bit access
> and masking the result. This will read past the end of the passed input
> but ignore the extra bytes. It won't be a problem unless the application
> goes out of its way to put a hash key value at the end of a mapped
> region.

Ack, fwiw it still makes it trickier to use AddressSanitizer in user app.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: BUG: AddressSanitizer reports a buffer-overflow on rte_hash_lookup
  2023-02-05 20:14   ` Isaac Boukris
@ 2023-02-05 21:08     ` Stephen Hemminger
  0 siblings, 0 replies; 4+ messages in thread
From: Stephen Hemminger @ 2023-02-05 21:08 UTC (permalink / raw)
  To: Isaac Boukris; +Cc: dev

On Sun, 5 Feb 2023 22:14:28 +0200
Isaac Boukris <iboukris@gmail.com> wrote:

> >
> > This code is using the common optimization of doing a full 32 bit access
> > and masking the result. This will read past the end of the passed input
> > but ignore the extra bytes. It won't be a problem unless the application
> > goes out of its way to put a hash key value at the end of a mapped
> > region.  
> 
> Ack, fwiw it still makes it trickier to use AddressSanitizer in user app.


Probably can be fixed with some annotations.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-02-05 21:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-05 16:54 BUG: AddressSanitizer reports a buffer-overflow on rte_hash_lookup Isaac Boukris
2023-02-05 19:49 ` Stephen Hemminger
2023-02-05 20:14   ` Isaac Boukris
2023-02-05 21:08     ` Stephen Hemminger

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).