[v3] Enable AddressSanitizer feature on DPDK
Checks
Commit Message
From: Zhihong Peng <zhihongx.peng@intel.com>
AddressSanitizer (ASan) is a google memory error detect
standard tool. It could help to detect use-after-free and
{heap,stack,global}-buffer overflow bugs in C/C++ programs,
print detailed error information when error happens, large
improve debug efficiency.
By referring to its implementation algorithm
(https://github.com/google/sanitizers/wiki/AddressSanitizerAlgorithm),
enable heap-buffer-overflow and use-after-free functions on dpdk.
DPDK ASAN function currently only supports on Linux x86_64.
Here is an example of heap-buffer-overflow bug:
......
char *p = rte_zmalloc(NULL, 7, 0);
p[7] = 'a';
......
Here is an example of use-after-free bug:
......
char *p = rte_zmalloc(NULL, 7, 0);
rte_free(p);
*p = 'a';
......
If you want to use this feature,
you need to add below compilation options when compiling code:
-Dbuildtype=debug -Db_lundef=false -Db_sanitize=address
"-Dbuildtype=debug": Display code information when coredump occurs
in the program.
"-Db_lundef=false": It is enabled by default, and needs to be
disabled when using asan.
Signed-off-by: Xueqin Lin <xueqin.lin@intel.com>
Signed-off-by: Zhihong Peng <zhihongx.peng@intel.com>
---
doc/guides/prog_guide/asan.rst | 130 ++++++++++++++++++++++
doc/guides/prog_guide/index.rst | 1 +
lib/eal/common/malloc_elem.c | 26 ++++-
lib/eal/common/malloc_elem.h | 184 +++++++++++++++++++++++++++++++-
lib/eal/common/malloc_heap.c | 12 +++
lib/eal/common/rte_malloc.c | 9 +-
lib/pipeline/rte_swx_pipeline.c | 4 +-
7 files changed, 359 insertions(+), 7 deletions(-)
create mode 100644 doc/guides/prog_guide/asan.rst
Comments
On Sat, Sep 18, 2021 at 9:51 AM <zhihongx.peng@intel.com> wrote:
>
> From: Zhihong Peng <zhihongx.peng@intel.com>
- The title is too vague.
I am not sure what the best title is, but my current idea is:
mem: instrument allocator with ASan
- This is a nice feature that must be announced in the release notes.
- How should we spell it?
Asan ?
ASAN ?
ASan ?
Please update devtools/words-case.txt and fix inconsistencies in this patch.
>
> AddressSanitizer (ASan) is a google memory error detect
> standard tool. It could help to detect use-after-free and
> {heap,stack,global}-buffer overflow bugs in C/C++ programs,
> print detailed error information when error happens, large
> improve debug efficiency.
>
> By referring to its implementation algorithm
> (https://github.com/google/sanitizers/wiki/AddressSanitizerAlgorithm),
> enable heap-buffer-overflow and use-after-free functions on dpdk.
> DPDK ASAN function currently only supports on Linux x86_64.
If you don't intend to update other arches, at least explain in the
commitlog what should be done: so that other arches know what to do to
add support.
>
> Here is an example of heap-buffer-overflow bug:
> ......
> char *p = rte_zmalloc(NULL, 7, 0);
> p[7] = 'a';
> ......
>
> Here is an example of use-after-free bug:
> ......
> char *p = rte_zmalloc(NULL, 7, 0);
> rte_free(p);
> *p = 'a';
> ......
>
> If you want to use this feature,
> you need to add below compilation options when compiling code:
> -Dbuildtype=debug -Db_lundef=false -Db_sanitize=address
ASAN is triggered by -Db_sanitize=address, it is the only *needed* option afaiu.
> "-Dbuildtype=debug": Display code information when coredump occurs
> in the program.
In ASan context, there is no coredump.
ASan displays a backtrace which is easier to read when debug symbols
are available.
You can suggest building with debug, but this is *not needed*.
> "-Db_lundef=false": It is enabled by default, and needs to be
> disabled when using asan.
This is an issue with meson and clang.
Tweaking b_lundef is needed with clang, gcc looks fine.
But still, on RHEL with gcc, I need to install libasan.
Maybe we can add libasan at a requirement at project level, did you try it?
>
> Signed-off-by: Xueqin Lin <xueqin.lin@intel.com>
> Signed-off-by: Zhihong Peng <zhihongx.peng@intel.com>
> ---
> doc/guides/prog_guide/asan.rst | 130 ++++++++++++++++++++++
> doc/guides/prog_guide/index.rst | 1 +
> lib/eal/common/malloc_elem.c | 26 ++++-
> lib/eal/common/malloc_elem.h | 184 +++++++++++++++++++++++++++++++-
> lib/eal/common/malloc_heap.c | 12 +++
> lib/eal/common/rte_malloc.c | 9 +-
> lib/pipeline/rte_swx_pipeline.c | 4 +-
This change on pipeline has no explanation, and looks out of place wrt
to current change.
> 7 files changed, 359 insertions(+), 7 deletions(-)
> create mode 100644 doc/guides/prog_guide/asan.rst
>
> diff --git a/doc/guides/prog_guide/asan.rst b/doc/guides/prog_guide/asan.rst
> new file mode 100644
> index 0000000000..a0589d9b8a
> --- /dev/null
> +++ b/doc/guides/prog_guide/asan.rst
> @@ -0,0 +1,130 @@
> +.. Copyright (c) <2021>, Intel Corporation
> + All rights reserved.
> +
> +Memory error detect standard tool - AddressSanitizer(Asan)
> +==========================================================
> +
> +AddressSanitizer (ASan) is a google memory error detect
> +standard tool. It could help to detect use-after-free and
> +{heap,stack,global}-buffer overflow bugs in C/C++ programs,
> +print detailed error information when error happens, large
> +improve debug efficiency.
> +
> +By referring to its implementation algorithm
> +(https://github.com/google/sanitizers/wiki/AddressSanitizerAlgorithm),
> +enabled heap-buffer-overflow and use-after-free functions on dpdk.
> +DPDK ASAN function currently only supports on Linux x86_64.
> +
> +AddressSanitizer is a part of LLVM(3.1+)and GCC(4.8+).
> +
> +Example heap-buffer-overflow error
> +----------------------------------
> +
> +Following error was reported when Asan was enabled::
> +
> + Applied 9 bytes of memory, but accessed the 10th byte of memory,
> + so heap-buffer-overflow appeared.
> +
> +Below code results in this error::
> +
> + char *p = rte_zmalloc(NULL, 9, 0);
> + if (!p) {
> + printf("rte_zmalloc error.");
> + return -1;
> + }
> + p[9] = 'a';
> +
> +The error log::
> +
> + ==49433==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f773fafa249 at pc 0x5556b13bdae4 bp 0x7ffeb4965e40 sp 0x7ffeb4965e30 WRITE of size 1 at 0x7f773fafa249 thread T0
> + #0 0x5556b13bdae3 in asan_heap_buffer_overflow ../app/test/test_asan_heap_buffer_overflow.c:25
> + #1 0x5556b043e9d4 in cmd_autotest_parsed ../app/test/commands.c:71
> + #2 0x5556b1cdd4b0 in cmdline_parse ../lib/cmdline/cmdline_parse.c:290
> + #3 0x5556b1cd8987 in cmdline_valid_buffer ../lib/cmdline/cmdline.c:26
> + #4 0x5556b1ce477a in rdline_char_in ../lib/cmdline/cmdline_rdline.c:421
> + #5 0x5556b1cd923e in cmdline_in ../lib/cmdline/cmdline.c:149
> + #6 0x5556b1cd9769 in cmdline_interact ../lib/cmdline/cmdline.c:223
> + #7 0x5556b045f53b in main ../app/test/test.c:234
> + #8 0x7f7f1eba90b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
> + #9 0x5556b043e70d in _start (/home/pzh/yyy/x86_64-native-linuxapp-gcc/app/test/dpdk-test+0x7ce70d)
> +
> + Address 0x7f773fafa249 is a wild pointer.
> + SUMMARY: AddressSanitizer: heap-buffer-overflow ../app/test/test_asan_heap_buffer_overflow.c:25 in asan_heap_buffer_overflow
> + Shadow bytes around the buggy address:
> + 0x0fef67f573f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0fef67f57400: fa fa 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0fef67f57410: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0fef67f57420: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0fef67f57430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + =>0x0fef67f57440: 00 00 00 00 00 00 fa fa 00[01]fa 00 00 00 00 00
> + 0x0fef67f57450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0fef67f57460: 00 00 00 00 00 00 fa fa 00 00 00 00 00 00 00 00
> + 0x0fef67f57470: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0fef67f57480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +
> +Example use-after-free error
> +----------------------------
> +
> +Following error was reported when Asan was enabled::
> +
> + Applied for 9 bytes of memory, and accessed the first byte after
> + released, so heap-use-after-free appeared.
> +
> +Below code results in this error::
> +
> + char *p = rte_zmalloc(NULL, 9, 0);
> + if (!p) {
> + printf("rte_zmalloc error.");
> + return -1;
> + }
> + rte_free(p);
> + *p = 'a';
> +
> +The error log::
> +
> + ==49478==ERROR: AddressSanitizer: heap-use-after-free on address 0x7fe2ffafa240 at pc 0x56409b084bc8 bp 0x7ffef62c57d0 sp 0x7ffef62c57c0 WRITE of size 1 at 0x7fe2ffafa240 thread T0
> + #0 0x56409b084bc7 in asan_use_after_free ../app/test/test_asan_use_after_free.c:26
> + #1 0x56409a1059d4 in cmd_autotest_parsed ../app/test/commands.c:71
> + #2 0x56409b9a44b0 in cmdline_parse ../lib/cmdline/cmdline_parse.c:290
> + #3 0x56409b99f987 in cmdline_valid_buffer ../lib/cmdline/cmdline.c:26
> + #4 0x56409b9ab77a in rdline_char_in ../lib/cmdline/cmdline_rdline.c:421
> + #5 0x56409b9a023e in cmdline_in ../lib/cmdline/cmdline.c:149
> + #6 0x56409b9a0769 in cmdline_interact ../lib/cmdline/cmdline.c:223
> + #7 0x56409a12653b in main ../app/test/test.c:234
> + #8 0x7feafafc20b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
> + #9 0x56409a10570d in _start (/home/pzh/yyy/x86_64-native-linuxapp-gcc/app/test/dpdk-test+0x7ce70d)
> +
> + Address 0x7fe2ffafa240 is a wild pointer.
> + SUMMARY: AddressSanitizer: heap-use-after-free ../app/test/test_asan_use_after_free.c:26 in asan_use_after_free
> + Shadow bytes around the buggy address:
> + 0x0ffcdff573f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0ffcdff57400: fa fa 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0ffcdff57410: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0ffcdff57420: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0ffcdff57430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + =>0x0ffcdff57440: 00 00 00 00 00 00 00 00[fd]fd fd fd fd fd fd fd
> + 0x0ffcdff57450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0ffcdff57460: 00 00 00 00 00 00 fa fa 00 00 00 00 00 00 00 00
> + 0x0ffcdff57470: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0ffcdff57480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0ffcdff57490: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +
> +Usage
> +-----
> +
> +meson build
> +^^^^^^^^^^^
> +
> +To enable Asan in meson build system, use following meson build command:
> +
> +Example usage::
> +
> + meson build -Dbuildtype=debug -Db_lundef=false -Db_sanitize=address
> + ninja -C build
> +
> +.. Note::
> +
> + a) The issue of asan wild pointer is that dpdk asan tool is not fully adapted to google asan.
> + For example: Address 0x7fe2ffafa240 is a wild pointer.
I can't understand what the "wild pointer" means in this context.
This comment belongs to the traces in the section before.
> + b) Centos8 needs to install libasan separately.
See my previous comment on b_lundef.
> + c) If the program uses cmdline, when a memory bug occurs, need to execute the "stty echo" command.
Yes, this is annoying when executing failing unit tests.
That is something to handle better in the cmdline library, maybe in the future.
Like "wild pointer", I don't think this comment belongs here.
> diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
> index 2dce507f46..df8a4b93e1 100644
> --- a/doc/guides/prog_guide/index.rst
> +++ b/doc/guides/prog_guide/index.rst
> @@ -71,3 +71,4 @@ Programmer's Guide
> lto
> profile_app
> glossary
> + asan
> diff --git a/lib/eal/common/malloc_elem.c b/lib/eal/common/malloc_elem.c
> index c2c9461f1d..bdd20a162e 100644
> --- a/lib/eal/common/malloc_elem.c
> +++ b/lib/eal/common/malloc_elem.c
> @@ -446,6 +446,8 @@ malloc_elem_alloc(struct malloc_elem *elem, size_t size, unsigned align,
> struct malloc_elem *new_free_elem =
> RTE_PTR_ADD(new_elem, size + MALLOC_ELEM_OVERHEAD);
>
> + asan_clear_split_alloczone(new_free_elem);
> +
> split_elem(elem, new_free_elem);
> malloc_elem_free_list_insert(new_free_elem);
>
> @@ -458,6 +460,8 @@ malloc_elem_alloc(struct malloc_elem *elem, size_t size, unsigned align,
> elem->state = ELEM_BUSY;
> elem->pad = old_elem_size;
>
> + asan_clear_alloczone(elem);
> +
> /* put a dummy header in padding, to point to real element header */
> if (elem->pad > 0) { /* pad will be at least 64-bytes, as everything
> * is cache-line aligned */
> @@ -470,12 +474,18 @@ malloc_elem_alloc(struct malloc_elem *elem, size_t size, unsigned align,
> return new_elem;
> }
>
> + asan_clear_split_alloczone(new_elem);
> +
> /* we are going to split the element in two. The original element
> * remains free, and the new element is the one allocated.
> * Re-insert original element, in case its new size makes it
> * belong on a different list.
> */
> +
> split_elem(elem, new_elem);
> +
> + asan_clear_alloczone(new_elem);
> +
> new_elem->state = ELEM_BUSY;
> malloc_elem_free_list_insert(elem);
>
> @@ -601,6 +611,8 @@ malloc_elem_hide_region(struct malloc_elem *elem, void *start, size_t len)
> if (next && next_elem_is_adjacent(elem)) {
> len_after = RTE_PTR_DIFF(next, hide_end);
> if (len_after >= MALLOC_ELEM_OVERHEAD + MIN_DATA_SIZE) {
> + asan_clear_split_alloczone(hide_end);
> +
> /* split after */
> split_elem(elem, hide_end);
>
> @@ -615,6 +627,8 @@ malloc_elem_hide_region(struct malloc_elem *elem, void *start, size_t len)
> if (prev && prev_elem_is_adjacent(elem)) {
> len_before = RTE_PTR_DIFF(hide_start, elem);
> if (len_before >= MALLOC_ELEM_OVERHEAD + MIN_DATA_SIZE) {
> + asan_clear_split_alloczone(hide_start);
> +
> /* split before */
> split_elem(elem, hide_start);
>
> @@ -628,6 +642,8 @@ malloc_elem_hide_region(struct malloc_elem *elem, void *start, size_t len)
> }
> }
>
> + asan_clear_alloczone(elem);
> +
> remove_elem(elem);
> }
>
> @@ -641,8 +657,10 @@ malloc_elem_resize(struct malloc_elem *elem, size_t size)
> const size_t new_size = size + elem->pad + MALLOC_ELEM_OVERHEAD;
>
> /* if we request a smaller size, then always return ok */
> - if (elem->size >= new_size)
> + if (elem->size >= new_size) {
> + asan_clear_alloczone(elem);
> return 0;
> + }
>
> /* check if there is a next element, it's free and adjacent */
> if (!elem->next || elem->next->state != ELEM_FREE ||
> @@ -661,9 +679,15 @@ malloc_elem_resize(struct malloc_elem *elem, size_t size)
> /* now we have a big block together. Lets cut it down a bit, by splitting */
> struct malloc_elem *split_pt = RTE_PTR_ADD(elem, new_size);
> split_pt = RTE_PTR_ALIGN_CEIL(split_pt, RTE_CACHE_LINE_SIZE);
> +
> + asan_clear_split_alloczone(split_pt);
> +
> split_elem(elem, split_pt);
> malloc_elem_free_list_insert(split_pt);
> }
> +
> + asan_clear_alloczone(elem);
> +
> return 0;
> }
>
> diff --git a/lib/eal/common/malloc_elem.h b/lib/eal/common/malloc_elem.h
> index a1e5f7f02c..01a739f2ea 100644
> --- a/lib/eal/common/malloc_elem.h
> +++ b/lib/eal/common/malloc_elem.h
> @@ -7,6 +7,14 @@
>
> #include <stdbool.h>
>
> +#ifdef __SANITIZE_ADDRESS__
> +#define RTE_MALLOC_ASAN
> +#elif defined(__has_feature)
> +# if __has_feature(address_sanitizer)
> +#define RTE_MALLOC_ASAN
> +# endif
> +#endif
> +
> #define MIN_DATA_SIZE (RTE_CACHE_LINE_SIZE)
>
> /* dummy definition of struct so we can use pointers to it in malloc_elem struct */
> @@ -36,10 +44,20 @@ struct malloc_elem {
> uint64_t header_cookie; /* Cookie marking start of data */
> /* trailer cookie at start + size */
> #endif
> +#ifdef RTE_MALLOC_ASAN
> + size_t user_size;
> + uint64_t asan_cookie[2]; /*must be next to header_cookie*/
Fix coding style for comment please.
> +#endif
> } __rte_cache_aligned;
>
> +static const unsigned int MALLOC_ELEM_HEADER_LEN = sizeof(struct malloc_elem);
> +
> #ifndef RTE_MALLOC_DEBUG
> -static const unsigned MALLOC_ELEM_TRAILER_LEN = 0;
> +#ifdef RTE_MALLOC_ASAN
> +static const unsigned int MALLOC_ELEM_TRAILER_LEN = RTE_CACHE_LINE_SIZE;
> +#else
> +static const unsigned int MALLOC_ELEM_TRAILER_LEN;
> +#endif
>
> /* dummy function - just check if pointer is non-null */
> static inline int
> @@ -55,7 +73,7 @@ set_trailer(struct malloc_elem *elem __rte_unused){ }
>
>
> #else
> -static const unsigned MALLOC_ELEM_TRAILER_LEN = RTE_CACHE_LINE_SIZE;
> +static const unsigned int MALLOC_ELEM_TRAILER_LEN = RTE_CACHE_LINE_SIZE;
>
> #define MALLOC_HEADER_COOKIE 0xbadbadbadadd2e55ULL /**< Header cookie. */
> #define MALLOC_TRAILER_COOKIE 0xadd2e55badbadbadULL /**< Trailer cookie.*/
> @@ -90,9 +108,169 @@ malloc_elem_cookies_ok(const struct malloc_elem *elem)
>
> #endif
>
> -static const unsigned MALLOC_ELEM_HEADER_LEN = sizeof(struct malloc_elem);
> #define MALLOC_ELEM_OVERHEAD (MALLOC_ELEM_HEADER_LEN + MALLOC_ELEM_TRAILER_LEN)
>
> +#ifdef RTE_MALLOC_ASAN
> +
> +#define ASAN_SHADOW_GRAIN_SIZE 8
> +#define ASAN_MEM_FREE_FLAG 0xfd
> +#define ASAN_MEM_REDZONE_FLAG 0xfa
> +#define ASAN_MEM_TO_SHADOW(mem) (((mem) >> 3) + 0x00007fff8000)
> +
> +#if defined(__clang__)
> +__attribute__((no_sanitize("address", "hwaddress")))
> +#else
> +__attribute__((no_sanitize_address))
> +#endif
This attribute is only used here, I am ok with leaving this as is.
If later it is needed elsewhere, we'll have to define a new attribute
wrapper in rte_common.h.
> +static inline void
> +asan_set_shadow(void *addr, char val)
> +{
> + *(char *)addr = val;
> +}
> +
> +static inline void
> +asan_set_zone(void *ptr, size_t len, uint32_t val)
> +{
> + size_t offset;
> + char *shadow;
> + size_t zone_len = len / ASAN_SHADOW_GRAIN_SIZE;
+ size_t i;
+ <empty line> to separate declarations from code.
> + if (len % ASAN_SHADOW_GRAIN_SIZE != 0)
> + zone_len += 1;
> +
> + for (size_t i = 0; i < zone_len; i++) {
+ for (i = 0; i < zone_len; i++) {
That's to fix build issue:
In file included from ../lib/eal/common/malloc_mp.c:16:0:
../lib/eal/common/malloc_elem.h: In function ‘asan_set_zone’:
../lib/eal/common/malloc_elem.h:140:2: error: ‘for’ loop initial
declarations are only allowed in C99 mode
for (size_t i = 0; i < zone_len; i++) {
^
../lib/eal/common/malloc_elem.h:140:2: note: use option -std=c99 or
-std=gnu99 to compile your code
> + offset = i * ASAN_SHADOW_GRAIN_SIZE;
> + shadow = (char *)ASAN_MEM_TO_SHADOW(((int64_t)ptr + offset));
> + asan_set_shadow(shadow, val);
> + }
> +}
> +
> +/*
> + * When the memory is released, the release mark is
> + * set in the corresponding range of the shadow area.
> + */
> +static inline void
> +asan_set_freezone(void *ptr, size_t size)
> +{
> + asan_set_zone(ptr, size, ASAN_MEM_FREE_FLAG);
> +}
> +
> +/*
> + * When the memory is allocated, memory state must set as accessible.
> + */
> +static inline void
> +asan_clear_alloczone(struct malloc_elem *elem)
> +{
> + asan_set_zone((void *)elem, elem->size, 0x0);
> +}
> +
> +static inline void
> +asan_clear_split_alloczone(struct malloc_elem *elem)
> +{
> + void *ptr = RTE_PTR_SUB(elem, MALLOC_ELEM_TRAILER_LEN);
> + asan_set_zone(ptr, MALLOC_ELEM_OVERHEAD, 0x0);
> +}
> +
> +/*
> + * When the memory is allocated, the memory boundary is
> + * marked in the corresponding range of the shadow area.
> + */
> +static inline void
> +asan_set_redzone(struct malloc_elem *elem, size_t user_size)
> +{
> + uint64_t ptr;
> + char *shadow;
> + if (elem != NULL) {
> + if (elem->state != ELEM_PAD)
> + elem = RTE_PTR_ADD(elem, elem->pad);
> +
> + elem->user_size = user_size;
> +
> + /* Set mark before the start of the allocated memory */
> + ptr = (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN)
> + - ASAN_SHADOW_GRAIN_SIZE;
> + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
> + asan_set_shadow(shadow, ASAN_MEM_REDZONE_FLAG);
> + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
> + - ASAN_SHADOW_GRAIN_SIZE);
> + asan_set_shadow(shadow, ASAN_MEM_REDZONE_FLAG);
> +
> + /* Set mark after the end of the allocated memory */
> + ptr = (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN
> + + elem->user_size);
> + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
> + uint32_t val = (ptr % ASAN_SHADOW_GRAIN_SIZE);
> + val = (val == 0) ? ASAN_MEM_REDZONE_FLAG : val;
> + asan_set_shadow(shadow, val);
> + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
> + + ASAN_SHADOW_GRAIN_SIZE);
> + asan_set_shadow(shadow, ASAN_MEM_REDZONE_FLAG);
> + }
> +}
> +
> +/*
> + * When the memory is released, the mark of the memory boundary
> + * in the corresponding range of the shadow area is cleared.
> + */
> +static inline void
> +asan_clear_redzone(struct malloc_elem *elem)
> +{
> + uint64_t ptr;
> + char *shadow;
> + if (elem != NULL) {
> + elem = RTE_PTR_ADD(elem, elem->pad);
> +
> + /* Clear mark before the start of the allocated memory */
> + ptr = (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN)
> + - ASAN_SHADOW_GRAIN_SIZE;
> + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
> + asan_set_shadow(shadow, 0x00);
> + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
> + - ASAN_SHADOW_GRAIN_SIZE);
> + asan_set_shadow(shadow, 0x00);
> +
> + /* Clear mark after the end of the allocated memory */
> + ptr = (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN
> + + elem->user_size);
> + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
> + asan_set_shadow(shadow, 0x00);
> + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
> + + ASAN_SHADOW_GRAIN_SIZE);
> + asan_set_shadow(shadow, 0x00);
> + }
> +}
> +
> +static inline size_t
> +old_malloc_size(struct malloc_elem *elem)
> +{
> + if (elem->state != ELEM_PAD)
> + elem = RTE_PTR_ADD(elem, elem->pad);
> +
> + return elem->user_size;
> +}
> +#else
> +static inline void
> +asan_set_freezone(void *ptr __rte_unused, size_t size __rte_unused) { }
> +
> +static inline void
> +asan_clear_alloczone(struct malloc_elem *elem __rte_unused) { }
> +
> +static inline void
> +asan_clear_split_alloczone(struct malloc_elem *elem __rte_unused) { }
> +
> +static inline void
> +asan_set_redzone(struct malloc_elem *elem __rte_unused,
> + size_t user_size __rte_unused) { }
> +
> +static inline void
> +asan_clear_redzone(struct malloc_elem *elem __rte_unused) { }
> +
> +static inline size_t
> +old_malloc_size(struct malloc_elem *elem)
> +{
> + return elem->size - elem->pad - MALLOC_ELEM_OVERHEAD;
> +}
> +#endif
> +
> /*
> * Given a pointer to the start of a memory block returned by malloc, get
> * the actual malloc_elem header for that block.
> diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
> index ee400f38ec..775d6789df 100644
> --- a/lib/eal/common/malloc_heap.c
> +++ b/lib/eal/common/malloc_heap.c
> @@ -237,6 +237,7 @@ heap_alloc(struct malloc_heap *heap, const char *type __rte_unused, size_t size,
> unsigned int flags, size_t align, size_t bound, bool contig)
> {
> struct malloc_elem *elem;
> + size_t user_size = size;
>
> size = RTE_CACHE_LINE_ROUNDUP(size);
> align = RTE_CACHE_LINE_ROUNDUP(align);
> @@ -250,6 +251,8 @@ heap_alloc(struct malloc_heap *heap, const char *type __rte_unused, size_t size,
>
> /* increase heap's count of allocated elements */
> heap->alloc_count++;
> +
> + asan_set_redzone(elem, user_size);
> }
>
> return elem == NULL ? NULL : (void *)(&elem[1]);
> @@ -270,6 +273,8 @@ heap_alloc_biggest(struct malloc_heap *heap, const char *type __rte_unused,
>
> /* increase heap's count of allocated elements */
> heap->alloc_count++;
> +
> + asan_set_redzone(elem, size);
> }
>
> return elem == NULL ? NULL : (void *)(&elem[1]);
> @@ -841,6 +846,8 @@ malloc_heap_free(struct malloc_elem *elem)
> if (!malloc_elem_cookies_ok(elem) || elem->state != ELEM_BUSY)
> return -1;
>
> + asan_clear_redzone(elem);
> +
> /* elem may be merged with previous element, so keep heap address */
> heap = elem->heap;
> msl = elem->msl;
> @@ -848,6 +855,9 @@ malloc_heap_free(struct malloc_elem *elem)
>
> rte_spinlock_lock(&(heap->lock));
>
> + void *asan_ptr = RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN + elem->pad);
> + size_t asan_data_len = elem->size - MALLOC_ELEM_OVERHEAD - elem->pad;
> +
> /* mark element as free */
> elem->state = ELEM_FREE;
>
> @@ -1001,6 +1011,8 @@ malloc_heap_free(struct malloc_elem *elem)
>
> rte_mcfg_mem_write_unlock();
> free_unlock:
> + asan_set_freezone(asan_ptr, asan_data_len);
> +
> rte_spinlock_unlock(&(heap->lock));
> return ret;
> }
> diff --git a/lib/eal/common/rte_malloc.c b/lib/eal/common/rte_malloc.c
> index 9d39e58c08..d0bec26920 100644
> --- a/lib/eal/common/rte_malloc.c
> +++ b/lib/eal/common/rte_malloc.c
> @@ -162,6 +162,8 @@ rte_calloc(const char *type, size_t num, size_t size, unsigned align)
> void *
> rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
> {
> + size_t user_size;
> +
> if (ptr == NULL)
> return rte_malloc_socket(NULL, size, align, socket);
>
> @@ -171,6 +173,8 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
> return NULL;
> }
>
> + user_size = size;
> +
> size = RTE_CACHE_LINE_ROUNDUP(size), align = RTE_CACHE_LINE_ROUNDUP(align);
>
> /* check requested socket id and alignment matches first, and if ok,
> @@ -181,6 +185,9 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
> RTE_PTR_ALIGN(ptr, align) == ptr &&
> malloc_heap_resize(elem, size) == 0) {
> rte_eal_trace_mem_realloc(size, align, socket, ptr);
> +
> + asan_set_redzone(elem, user_size);
> +
> return ptr;
> }
>
> @@ -192,7 +199,7 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
> if (new_ptr == NULL)
> return NULL;
> /* elem: |pad|data_elem|data|trailer| */
> - const size_t old_size = elem->size - elem->pad - MALLOC_ELEM_OVERHEAD;
> + const size_t old_size = old_malloc_size(elem);
> rte_memcpy(new_ptr, ptr, old_size < size ? old_size : size);
> rte_free(ptr);
>
> diff --git a/lib/pipeline/rte_swx_pipeline.c b/lib/pipeline/rte_swx_pipeline.c
> index 8eb978a30c..aaa0107d02 100644
> --- a/lib/pipeline/rte_swx_pipeline.c
> +++ b/lib/pipeline/rte_swx_pipeline.c
> @@ -6340,7 +6340,7 @@ instr_meter_translate(struct rte_swx_pipeline *p,
> return 0;
> }
>
> - CHECK(0, EINVAL);
> + return -EINVAL;
> }
>
> static inline struct meter *
> @@ -8025,7 +8025,7 @@ instr_translate(struct rte_swx_pipeline *p,
> instr,
> data);
>
> - CHECK(0, EINVAL);
> + return -EINVAL;
> }
>
> static struct instruction_data *
> --
> 2.25.1
>
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Monday, September 20, 2021 6:04 PM
> To: Peng, ZhihongX <zhihongx.peng@intel.com>
> Cc: Burakov, Anatoly <anatoly.burakov@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Stephen Hemminger
> <stephen@networkplumber.org>; dev <dev@dpdk.org>; Lin, Xueqin
> <xueqin.lin@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3] Enable AddressSanitizer feature on DPDK
>
> On Sat, Sep 18, 2021 at 9:51 AM <zhihongx.peng@intel.com> wrote:
> >
> > From: Zhihong Peng <zhihongx.peng@intel.com>
>
> - The title is too vague.
> I am not sure what the best title is, but my current idea is:
> mem: instrument allocator with Asan
Do you think this name is accurate: Enable AddressSanitizer for memory detector on DPDK
>
>
> - This is a nice feature that must be announced in the release notes.
>
>
> - How should we spell it?
> Asan ?
> ASAN ?
> ASan ?
Use ASan, google's spelling:
https://github.com/google/sanitizers/wiki/AddressSanitizer
>
> Please update devtools/words-case.txt and fix inconsistencies in this patch.
V4 will update this document.
> >
> > AddressSanitizer (ASan) is a google memory error detect standard tool.
> > It could help to detect use-after-free and {heap,stack,global}-buffer
> > overflow bugs in C/C++ programs, print detailed error information when
> > error happens, large improve debug efficiency.
> >
> > By referring to its implementation algorithm
> > (https://github.com/google/sanitizers/wiki/AddressSanitizerAlgorithm),
> > enable heap-buffer-overflow and use-after-free functions on dpdk.
> > DPDK ASAN function currently only supports on Linux x86_64.
>
> If you don't intend to update other arches, at least explain in the commitlog
> what should be done: so that other arches know what to do to add support.
V4 will add instructions.
>
> >
> > Here is an example of heap-buffer-overflow bug:
> > ......
> > char *p = rte_zmalloc(NULL, 7, 0);
> > p[7] = 'a';
> > ......
> >
> > Here is an example of use-after-free bug:
> > ......
> > char *p = rte_zmalloc(NULL, 7, 0);
> > rte_free(p);
> > *p = 'a';
> > ......
> >
> > If you want to use this feature,
> > you need to add below compilation options when compiling code:
> > -Dbuildtype=debug -Db_lundef=false -Db_sanitize=address
>
> ASAN is triggered by -Db_sanitize=address, it is the only *needed* option
> afaiu.
>
>
> > "-Dbuildtype=debug": Display code information when coredump occurs in
> > the program.
>
> In ASan context, there is no coredump.
> ASan displays a backtrace which is easier to read when debug symbols are
> available.
> You can suggest building with debug, but this is *not needed*.
>
>
> > "-Db_lundef=false": It is enabled by default, and needs to be disabled
> > when using asan.
Dbuildtype and Db_lundef will update the instructions.
> This is an issue with meson and clang.
> Tweaking b_lundef is needed with clang, gcc looks fine.
> But still, on RHEL with gcc, I need to install libasan.
>
> Maybe we can add libasan at a requirement at project level, did you try it?
libasan is installed by default on the ubuntu system. Because libasan
and the compiler are matched, so it is best to manually install the
matching library.
>
> >
> > Signed-off-by: Xueqin Lin <xueqin.lin@intel.com>
> > Signed-off-by: Zhihong Peng <zhihongx.peng@intel.com>
> > ---
> > doc/guides/prog_guide/asan.rst | 130 ++++++++++++++++++++++
> > doc/guides/prog_guide/index.rst | 1 +
> > lib/eal/common/malloc_elem.c | 26 ++++-
> > lib/eal/common/malloc_elem.h | 184
> +++++++++++++++++++++++++++++++-
> > lib/eal/common/malloc_heap.c | 12 +++
> > lib/eal/common/rte_malloc.c | 9 +-
> > lib/pipeline/rte_swx_pipeline.c | 4 +-
>
> This change on pipeline has no explanation, and looks out of place wrt to
> current change.
This is to solve the gcc compilation problem, I will separate this patch.
>
>
>
> > 7 files changed, 359 insertions(+), 7 deletions(-) create mode
> > 100644 doc/guides/prog_guide/asan.rst
> >
> > diff --git a/doc/guides/prog_guide/asan.rst
> > b/doc/guides/prog_guide/asan.rst new file mode 100644 index
> > 0000000000..a0589d9b8a
> > --- /dev/null
> > +++ b/doc/guides/prog_guide/asan.rst
> > @@ -0,0 +1,130 @@
> > +.. Copyright (c) <2021>, Intel Corporation
> > + All rights reserved.
> > +
> > +Memory error detect standard tool - AddressSanitizer(Asan)
> >
> +=========================================================
> =
> > +
> > +AddressSanitizer (ASan) is a google memory error detect standard
> > +tool. It could help to detect use-after-free and
> > +{heap,stack,global}-buffer overflow bugs in C/C++ programs, print
> > +detailed error information when error happens, large improve debug
> > +efficiency.
> > +
> > +By referring to its implementation algorithm
> > +(https://github.com/google/sanitizers/wiki/AddressSanitizerAlgorithm)
> > +, enabled heap-buffer-overflow and use-after-free functions on dpdk.
> > +DPDK ASAN function currently only supports on Linux x86_64.
> > +
> > +AddressSanitizer is a part of LLVM(3.1+)and GCC(4.8+).
> > +
> > +Example heap-buffer-overflow error
> > +----------------------------------
> > +
> > +Following error was reported when Asan was enabled::
> > +
> > + Applied 9 bytes of memory, but accessed the 10th byte of memory,
> > + so heap-buffer-overflow appeared.
> > +
> > +Below code results in this error::
> > +
> > + char *p = rte_zmalloc(NULL, 9, 0);
> > + if (!p) {
> > + printf("rte_zmalloc error.");
> > + return -1;
> > + }
> > + p[9] = 'a';
> > +
> > +The error log::
> > +
> > + ==49433==ERROR: AddressSanitizer: heap-buffer-overflow on address
> 0x7f773fafa249 at pc 0x5556b13bdae4 bp 0x7ffeb4965e40 sp 0x7ffeb4965e30
> WRITE of size 1 at 0x7f773fafa249 thread T0
> > + #0 0x5556b13bdae3 in
> asan_heap_buffer_overflow ../app/test/test_asan_heap_buffer_overflow.
> c:25
> > + #1 0x5556b043e9d4 in
> cmd_autotest_parsed ../app/test/commands.c:71
> > + #2 0x5556b1cdd4b0 in
> cmdline_parse ../lib/cmdline/cmdline_parse.c:290
> > + #3 0x5556b1cd8987 in cmdline_valid_buffer ../lib/cmdline/cmdline.c:26
> > + #4 0x5556b1ce477a in rdline_char_in ../lib/cmdline/cmdline_rdline.c:421
> > + #5 0x5556b1cd923e in cmdline_in ../lib/cmdline/cmdline.c:149
> > + #6 0x5556b1cd9769 in cmdline_interact ../lib/cmdline/cmdline.c:223
> > + #7 0x5556b045f53b in main ../app/test/test.c:234
> > + #8 0x7f7f1eba90b2 in __libc_start_main (/lib/x86_64-linux-
> gnu/libc.so.6+0x270b2)
> > + #9 0x5556b043e70d in _start
> > + (/home/pzh/yyy/x86_64-native-linuxapp-gcc/app/test/dpdk-
> test+0x7ce70
> > + d)
> > +
> > + Address 0x7f773fafa249 is a wild pointer.
> > + SUMMARY: AddressSanitizer: heap-buffer-
> overflow ../app/test/test_asan_heap_buffer_overflow.c:25 in
> asan_heap_buffer_overflow
> > + Shadow bytes around the buggy address:
> > + 0x0fef67f573f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0fef67f57400: fa fa 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0fef67f57410: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0fef67f57420: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0fef67f57430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + =>0x0fef67f57440: 00 00 00 00 00 00 fa fa 00[01]fa 00 00 00 00 00
> > + 0x0fef67f57450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0fef67f57460: 00 00 00 00 00 00 fa fa 00 00 00 00 00 00 00 00
> > + 0x0fef67f57470: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0fef67f57480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > +
> > +Example use-after-free error
> > +----------------------------
> > +
> > +Following error was reported when Asan was enabled::
> > +
> > + Applied for 9 bytes of memory, and accessed the first byte after
> > + released, so heap-use-after-free appeared.
> > +
> > +Below code results in this error::
> > +
> > + char *p = rte_zmalloc(NULL, 9, 0);
> > + if (!p) {
> > + printf("rte_zmalloc error.");
> > + return -1;
> > + }
> > + rte_free(p);
> > + *p = 'a';
> > +
> > +The error log::
> > +
> > + ==49478==ERROR: AddressSanitizer: heap-use-after-free on address
> 0x7fe2ffafa240 at pc 0x56409b084bc8 bp 0x7ffef62c57d0 sp 0x7ffef62c57c0
> WRITE of size 1 at 0x7fe2ffafa240 thread T0
> > + #0 0x56409b084bc7 in
> asan_use_after_free ../app/test/test_asan_use_after_free.c:26
> > + #1 0x56409a1059d4 in
> cmd_autotest_parsed ../app/test/commands.c:71
> > + #2 0x56409b9a44b0 in
> cmdline_parse ../lib/cmdline/cmdline_parse.c:290
> > + #3 0x56409b99f987 in cmdline_valid_buffer ../lib/cmdline/cmdline.c:26
> > + #4 0x56409b9ab77a in
> rdline_char_in ../lib/cmdline/cmdline_rdline.c:421
> > + #5 0x56409b9a023e in cmdline_in ../lib/cmdline/cmdline.c:149
> > + #6 0x56409b9a0769 in cmdline_interact ../lib/cmdline/cmdline.c:223
> > + #7 0x56409a12653b in main ../app/test/test.c:234
> > + #8 0x7feafafc20b2 in __libc_start_main (/lib/x86_64-linux-
> gnu/libc.so.6+0x270b2)
> > + #9 0x56409a10570d in _start
> > + (/home/pzh/yyy/x86_64-native-linuxapp-gcc/app/test/dpdk-
> test+0x7ce70
> > + d)
> > +
> > + Address 0x7fe2ffafa240 is a wild pointer.
> > + SUMMARY: AddressSanitizer: heap-use-after-
> free ../app/test/test_asan_use_after_free.c:26 in asan_use_after_free
> > + Shadow bytes around the buggy address:
> > + 0x0ffcdff573f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0ffcdff57400: fa fa 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0ffcdff57410: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0ffcdff57420: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0ffcdff57430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + =>0x0ffcdff57440: 00 00 00 00 00 00 00 00[fd]fd fd fd fd fd fd fd
> > + 0x0ffcdff57450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0ffcdff57460: 00 00 00 00 00 00 fa fa 00 00 00 00 00 00 00 00
> > + 0x0ffcdff57470: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0ffcdff57480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0ffcdff57490: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > +
> > +Usage
> > +-----
> > +
> > +meson build
> > +^^^^^^^^^^^
> > +
> > +To enable Asan in meson build system, use following meson build
> command:
> > +
> > +Example usage::
> > +
> > + meson build -Dbuildtype=debug -Db_lundef=false -Db_sanitize=address
> > + ninja -C build
> > +
> > +.. Note::
> > +
> > + a) The issue of asan wild pointer is that dpdk asan tool is not fully
> adapted to google asan.
> > + For example: Address 0x7fe2ffafa240 is a wild pointer.
>
> I can't understand what the "wild pointer" means in this context.
> This comment belongs to the traces in the section before.
This is because we only set up the red zone and did not construct the
complete data structure required by libasan. As a result, libasan could
not find the location of the memory request, so "wild pointer" was
reported. The current information has been able to troubleshoot errors,
so this issue is a lower priority.
>
> > + b) Centos8 needs to install libasan separately.
>
> See my previous comment on b_lundef.
>
>
> > + c) If the program uses cmdline, when a memory bug occurs, need to
> execute the "stty echo" command.
>
> Yes, this is annoying when executing failing unit tests.
> That is something to handle better in the cmdline library, maybe in the future.
>
> Like "wild pointer", I don't think this comment belongs here.
cmdline cannot solve this problem temporarily, so this problem must
be explained, but I don’t know which position is better.
>
>
> > diff --git a/doc/guides/prog_guide/index.rst
> > b/doc/guides/prog_guide/index.rst index 2dce507f46..df8a4b93e1 100644
> > --- a/doc/guides/prog_guide/index.rst
> > +++ b/doc/guides/prog_guide/index.rst
> > @@ -71,3 +71,4 @@ Programmer's Guide
> > lto
> > profile_app
> > glossary
> > + asan
> > diff --git a/lib/eal/common/malloc_elem.c
> > b/lib/eal/common/malloc_elem.c index c2c9461f1d..bdd20a162e 100644
> > --- a/lib/eal/common/malloc_elem.c
> > +++ b/lib/eal/common/malloc_elem.c
> > @@ -446,6 +446,8 @@ malloc_elem_alloc(struct malloc_elem *elem,
> size_t size, unsigned align,
> > struct malloc_elem *new_free_elem =
> > RTE_PTR_ADD(new_elem, size +
> > MALLOC_ELEM_OVERHEAD);
> >
> > + asan_clear_split_alloczone(new_free_elem);
> > +
> > split_elem(elem, new_free_elem);
> > malloc_elem_free_list_insert(new_free_elem);
> >
> > @@ -458,6 +460,8 @@ malloc_elem_alloc(struct malloc_elem *elem,
> size_t size, unsigned align,
> > elem->state = ELEM_BUSY;
> > elem->pad = old_elem_size;
> >
> > + asan_clear_alloczone(elem);
> > +
> > /* put a dummy header in padding, to point to real element header
> */
> > if (elem->pad > 0) { /* pad will be at least 64-bytes, as everything
> > * is cache-line aligned */ @@
> > -470,12 +474,18 @@ malloc_elem_alloc(struct malloc_elem *elem, size_t
> size, unsigned align,
> > return new_elem;
> > }
> >
> > + asan_clear_split_alloczone(new_elem);
> > +
> > /* we are going to split the element in two. The original element
> > * remains free, and the new element is the one allocated.
> > * Re-insert original element, in case its new size makes it
> > * belong on a different list.
> > */
> > +
> > split_elem(elem, new_elem);
> > +
> > + asan_clear_alloczone(new_elem);
> > +
> > new_elem->state = ELEM_BUSY;
> > malloc_elem_free_list_insert(elem);
> >
> > @@ -601,6 +611,8 @@ malloc_elem_hide_region(struct malloc_elem
> *elem, void *start, size_t len)
> > if (next && next_elem_is_adjacent(elem)) {
> > len_after = RTE_PTR_DIFF(next, hide_end);
> > if (len_after >= MALLOC_ELEM_OVERHEAD + MIN_DATA_SIZE)
> > {
> > + asan_clear_split_alloczone(hide_end);
> > +
> > /* split after */
> > split_elem(elem, hide_end);
> >
> > @@ -615,6 +627,8 @@ malloc_elem_hide_region(struct malloc_elem
> *elem, void *start, size_t len)
> > if (prev && prev_elem_is_adjacent(elem)) {
> > len_before = RTE_PTR_DIFF(hide_start, elem);
> > if (len_before >= MALLOC_ELEM_OVERHEAD +
> > MIN_DATA_SIZE) {
> > + asan_clear_split_alloczone(hide_start);
> > +
> > /* split before */
> > split_elem(elem, hide_start);
> >
> > @@ -628,6 +642,8 @@ malloc_elem_hide_region(struct malloc_elem
> *elem, void *start, size_t len)
> > }
> > }
> >
> > + asan_clear_alloczone(elem);
> > +
> > remove_elem(elem);
> > }
> >
> > @@ -641,8 +657,10 @@ malloc_elem_resize(struct malloc_elem *elem,
> size_t size)
> > const size_t new_size = size + elem->pad +
> > MALLOC_ELEM_OVERHEAD;
> >
> > /* if we request a smaller size, then always return ok */
> > - if (elem->size >= new_size)
> > + if (elem->size >= new_size) {
> > + asan_clear_alloczone(elem);
> > return 0;
> > + }
> >
> > /* check if there is a next element, it's free and adjacent */
> > if (!elem->next || elem->next->state != ELEM_FREE || @@ -661,9
> > +679,15 @@ malloc_elem_resize(struct malloc_elem *elem, size_t size)
> > /* now we have a big block together. Lets cut it down a bit, by
> splitting */
> > struct malloc_elem *split_pt = RTE_PTR_ADD(elem, new_size);
> > split_pt = RTE_PTR_ALIGN_CEIL(split_pt,
> > RTE_CACHE_LINE_SIZE);
> > +
> > + asan_clear_split_alloczone(split_pt);
> > +
> > split_elem(elem, split_pt);
> > malloc_elem_free_list_insert(split_pt);
> > }
> > +
> > + asan_clear_alloczone(elem);
> > +
> > return 0;
> > }
> >
> > diff --git a/lib/eal/common/malloc_elem.h
> > b/lib/eal/common/malloc_elem.h index a1e5f7f02c..01a739f2ea 100644
> > --- a/lib/eal/common/malloc_elem.h
> > +++ b/lib/eal/common/malloc_elem.h
> > @@ -7,6 +7,14 @@
> >
> > #include <stdbool.h>
> >
> > +#ifdef __SANITIZE_ADDRESS__
> > +#define RTE_MALLOC_ASAN
> > +#elif defined(__has_feature)
> > +# if __has_feature(address_sanitizer) #define RTE_MALLOC_ASAN #
> endif
> > +#endif
> > +
> > #define MIN_DATA_SIZE (RTE_CACHE_LINE_SIZE)
> >
> > /* dummy definition of struct so we can use pointers to it in
> > malloc_elem struct */ @@ -36,10 +44,20 @@ struct malloc_elem {
> > uint64_t header_cookie; /* Cookie marking start of data */
> > /* trailer cookie at start +
> > size */ #endif
> > +#ifdef RTE_MALLOC_ASAN
> > + size_t user_size;
> > + uint64_t asan_cookie[2]; /*must be next to header_cookie*/
>
> Fix coding style for comment please.
V4 will be fixed.
>
> > +#endif
> > } __rte_cache_aligned;
> >
> > +static const unsigned int MALLOC_ELEM_HEADER_LEN = sizeof(struct
> > +malloc_elem);
> > +
> > #ifndef RTE_MALLOC_DEBUG
> > -static const unsigned MALLOC_ELEM_TRAILER_LEN = 0;
> > +#ifdef RTE_MALLOC_ASAN
> > +static const unsigned int MALLOC_ELEM_TRAILER_LEN =
> > +RTE_CACHE_LINE_SIZE; #else static const unsigned int
> > +MALLOC_ELEM_TRAILER_LEN; #endif
> >
> > /* dummy function - just check if pointer is non-null */ static
> > inline int @@ -55,7 +73,7 @@ set_trailer(struct malloc_elem *elem
> > __rte_unused){ }
> >
> >
> > #else
> > -static const unsigned MALLOC_ELEM_TRAILER_LEN =
> RTE_CACHE_LINE_SIZE;
> > +static const unsigned int MALLOC_ELEM_TRAILER_LEN =
> > +RTE_CACHE_LINE_SIZE;
> >
> > #define MALLOC_HEADER_COOKIE 0xbadbadbadadd2e55ULL /**<
> Header cookie. */
> > #define MALLOC_TRAILER_COOKIE 0xadd2e55badbadbadULL /**< Trailer
> > cookie.*/ @@ -90,9 +108,169 @@ malloc_elem_cookies_ok(const struct
> > malloc_elem *elem)
> >
> > #endif
> >
> > -static const unsigned MALLOC_ELEM_HEADER_LEN = sizeof(struct
> > malloc_elem); #define MALLOC_ELEM_OVERHEAD
> (MALLOC_ELEM_HEADER_LEN +
> > MALLOC_ELEM_TRAILER_LEN)
> >
> > +#ifdef RTE_MALLOC_ASAN
> > +
> > +#define ASAN_SHADOW_GRAIN_SIZE 8
> > +#define ASAN_MEM_FREE_FLAG 0xfd
> > +#define ASAN_MEM_REDZONE_FLAG 0xfa
> > +#define ASAN_MEM_TO_SHADOW(mem) (((mem) >> 3) +
> 0x00007fff8000)
> > +
> > +#if defined(__clang__)
> > +__attribute__((no_sanitize("address", "hwaddress"))) #else
> > +__attribute__((no_sanitize_address))
> > +#endif
>
> This attribute is only used here, I am ok with leaving this as is.
> If later it is needed elsewhere, we'll have to define a new attribute wrapper
> in rte_common.h.
>
>
> > +static inline void
> > +asan_set_shadow(void *addr, char val) {
> > + *(char *)addr = val;
> > +}
> > +
> > +static inline void
> > +asan_set_zone(void *ptr, size_t len, uint32_t val) {
> > + size_t offset;
> > + char *shadow;
> > + size_t zone_len = len / ASAN_SHADOW_GRAIN_SIZE;
>
> + size_t i;
> + <empty line> to separate declarations from code.
>
>
> > + if (len % ASAN_SHADOW_GRAIN_SIZE != 0)
> > + zone_len += 1;
> > +
> > + for (size_t i = 0; i < zone_len; i++) {
>
> + for (i = 0; i < zone_len; i++) {
>
> That's to fix build issue:
>
V4 will be fixed.
> In file included from ../lib/eal/common/malloc_mp.c:16:0:
> ../lib/eal/common/malloc_elem.h: In function ‘asan_set_zone’:
> ../lib/eal/common/malloc_elem.h:140:2: error: ‘for’ loop initial declarations
> are only allowed in C99 mode
> for (size_t i = 0; i < zone_len; i++) {
> ^
> ../lib/eal/common/malloc_elem.h:140:2: note: use option -std=c99 or
> -std=gnu99 to compile your code
>
>
>
>
>
>
>
> > + offset = i * ASAN_SHADOW_GRAIN_SIZE;
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(((int64_t)ptr +
> offset));
> > + asan_set_shadow(shadow, val);
> > + }
> > +}
> > +
> > +/*
> > + * When the memory is released, the release mark is
> > + * set in the corresponding range of the shadow area.
> > + */
> > +static inline void
> > +asan_set_freezone(void *ptr, size_t size) {
> > + asan_set_zone(ptr, size, ASAN_MEM_FREE_FLAG); }
> > +
> > +/*
> > + * When the memory is allocated, memory state must set as accessible.
> > + */
> > +static inline void
> > +asan_clear_alloczone(struct malloc_elem *elem)
> > +{
> > + asan_set_zone((void *)elem, elem->size, 0x0);
> > +}
> > +
> > +static inline void
> > +asan_clear_split_alloczone(struct malloc_elem *elem)
> > +{
> > + void *ptr = RTE_PTR_SUB(elem, MALLOC_ELEM_TRAILER_LEN);
> > + asan_set_zone(ptr, MALLOC_ELEM_OVERHEAD, 0x0);
> > +}
> > +
> > +/*
> > + * When the memory is allocated, the memory boundary is
> > + * marked in the corresponding range of the shadow area.
> > + */
> > +static inline void
> > +asan_set_redzone(struct malloc_elem *elem, size_t user_size)
> > +{
> > + uint64_t ptr;
> > + char *shadow;
> > + if (elem != NULL) {
> > + if (elem->state != ELEM_PAD)
> > + elem = RTE_PTR_ADD(elem, elem->pad);
> > +
> > + elem->user_size = user_size;
> > +
> > + /* Set mark before the start of the allocated memory */
> > + ptr = (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN)
> > + - ASAN_SHADOW_GRAIN_SIZE;
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
> > + asan_set_shadow(shadow, ASAN_MEM_REDZONE_FLAG);
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
> > + - ASAN_SHADOW_GRAIN_SIZE);
> > + asan_set_shadow(shadow, ASAN_MEM_REDZONE_FLAG);
> > +
> > + /* Set mark after the end of the allocated memory */
> > + ptr = (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN
> > + + elem->user_size);
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
> > + uint32_t val = (ptr % ASAN_SHADOW_GRAIN_SIZE);
> > + val = (val == 0) ? ASAN_MEM_REDZONE_FLAG : val;
> > + asan_set_shadow(shadow, val);
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
> > + + ASAN_SHADOW_GRAIN_SIZE);
> > + asan_set_shadow(shadow, ASAN_MEM_REDZONE_FLAG);
> > + }
> > +}
> > +
> > +/*
> > + * When the memory is released, the mark of the memory boundary
> > + * in the corresponding range of the shadow area is cleared.
> > + */
> > +static inline void
> > +asan_clear_redzone(struct malloc_elem *elem)
> > +{
> > + uint64_t ptr;
> > + char *shadow;
> > + if (elem != NULL) {
> > + elem = RTE_PTR_ADD(elem, elem->pad);
> > +
> > + /* Clear mark before the start of the allocated memory */
> > + ptr = (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN)
> > + - ASAN_SHADOW_GRAIN_SIZE;
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
> > + asan_set_shadow(shadow, 0x00);
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
> > + - ASAN_SHADOW_GRAIN_SIZE);
> > + asan_set_shadow(shadow, 0x00);
> > +
> > + /* Clear mark after the end of the allocated memory */
> > + ptr = (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN
> > + + elem->user_size);
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
> > + asan_set_shadow(shadow, 0x00);
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
> > + + ASAN_SHADOW_GRAIN_SIZE);
> > + asan_set_shadow(shadow, 0x00);
> > + }
> > +}
> > +
> > +static inline size_t
> > +old_malloc_size(struct malloc_elem *elem)
> > +{
> > + if (elem->state != ELEM_PAD)
> > + elem = RTE_PTR_ADD(elem, elem->pad);
> > +
> > + return elem->user_size;
> > +}
> > +#else
> > +static inline void
> > +asan_set_freezone(void *ptr __rte_unused, size_t size __rte_unused) { }
> > +
> > +static inline void
> > +asan_clear_alloczone(struct malloc_elem *elem __rte_unused) { }
> > +
> > +static inline void
> > +asan_clear_split_alloczone(struct malloc_elem *elem __rte_unused) { }
> > +
> > +static inline void
> > +asan_set_redzone(struct malloc_elem *elem __rte_unused,
> > + size_t user_size __rte_unused) { }
> > +
> > +static inline void
> > +asan_clear_redzone(struct malloc_elem *elem __rte_unused) { }
> > +
> > +static inline size_t
> > +old_malloc_size(struct malloc_elem *elem)
> > +{
> > + return elem->size - elem->pad - MALLOC_ELEM_OVERHEAD;
> > +}
> > +#endif
> > +
> > /*
> > * Given a pointer to the start of a memory block returned by malloc, get
> > * the actual malloc_elem header for that block.
> > diff --git a/lib/eal/common/malloc_heap.c
> b/lib/eal/common/malloc_heap.c
> > index ee400f38ec..775d6789df 100644
> > --- a/lib/eal/common/malloc_heap.c
> > +++ b/lib/eal/common/malloc_heap.c
> > @@ -237,6 +237,7 @@ heap_alloc(struct malloc_heap *heap, const char
> *type __rte_unused, size_t size,
> > unsigned int flags, size_t align, size_t bound, bool contig)
> > {
> > struct malloc_elem *elem;
> > + size_t user_size = size;
> >
> > size = RTE_CACHE_LINE_ROUNDUP(size);
> > align = RTE_CACHE_LINE_ROUNDUP(align);
> > @@ -250,6 +251,8 @@ heap_alloc(struct malloc_heap *heap, const char
> *type __rte_unused, size_t size,
> >
> > /* increase heap's count of allocated elements */
> > heap->alloc_count++;
> > +
> > + asan_set_redzone(elem, user_size);
> > }
> >
> > return elem == NULL ? NULL : (void *)(&elem[1]);
> > @@ -270,6 +273,8 @@ heap_alloc_biggest(struct malloc_heap *heap,
> const char *type __rte_unused,
> >
> > /* increase heap's count of allocated elements */
> > heap->alloc_count++;
> > +
> > + asan_set_redzone(elem, size);
> > }
> >
> > return elem == NULL ? NULL : (void *)(&elem[1]);
> > @@ -841,6 +846,8 @@ malloc_heap_free(struct malloc_elem *elem)
> > if (!malloc_elem_cookies_ok(elem) || elem->state != ELEM_BUSY)
> > return -1;
> >
> > + asan_clear_redzone(elem);
> > +
> > /* elem may be merged with previous element, so keep heap address
> */
> > heap = elem->heap;
> > msl = elem->msl;
> > @@ -848,6 +855,9 @@ malloc_heap_free(struct malloc_elem *elem)
> >
> > rte_spinlock_lock(&(heap->lock));
> >
> > + void *asan_ptr = RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN +
> elem->pad);
> > + size_t asan_data_len = elem->size - MALLOC_ELEM_OVERHEAD -
> elem->pad;
> > +
> > /* mark element as free */
> > elem->state = ELEM_FREE;
> >
> > @@ -1001,6 +1011,8 @@ malloc_heap_free(struct malloc_elem *elem)
> >
> > rte_mcfg_mem_write_unlock();
> > free_unlock:
> > + asan_set_freezone(asan_ptr, asan_data_len);
> > +
> > rte_spinlock_unlock(&(heap->lock));
> > return ret;
> > }
> > diff --git a/lib/eal/common/rte_malloc.c b/lib/eal/common/rte_malloc.c
> > index 9d39e58c08..d0bec26920 100644
> > --- a/lib/eal/common/rte_malloc.c
> > +++ b/lib/eal/common/rte_malloc.c
> > @@ -162,6 +162,8 @@ rte_calloc(const char *type, size_t num, size_t size,
> unsigned align)
> > void *
> > rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
> > {
> > + size_t user_size;
> > +
> > if (ptr == NULL)
> > return rte_malloc_socket(NULL, size, align, socket);
> >
> > @@ -171,6 +173,8 @@ rte_realloc_socket(void *ptr, size_t size, unsigned
> int align, int socket)
> > return NULL;
> > }
> >
> > + user_size = size;
> > +
> > size = RTE_CACHE_LINE_ROUNDUP(size), align =
> RTE_CACHE_LINE_ROUNDUP(align);
> >
> > /* check requested socket id and alignment matches first, and if ok,
> > @@ -181,6 +185,9 @@ rte_realloc_socket(void *ptr, size_t size, unsigned
> int align, int socket)
> > RTE_PTR_ALIGN(ptr, align) == ptr &&
> > malloc_heap_resize(elem, size) == 0) {
> > rte_eal_trace_mem_realloc(size, align, socket, ptr);
> > +
> > + asan_set_redzone(elem, user_size);
> > +
> > return ptr;
> > }
> >
> > @@ -192,7 +199,7 @@ rte_realloc_socket(void *ptr, size_t size, unsigned
> int align, int socket)
> > if (new_ptr == NULL)
> > return NULL;
> > /* elem: |pad|data_elem|data|trailer| */
> > - const size_t old_size = elem->size - elem->pad -
> MALLOC_ELEM_OVERHEAD;
> > + const size_t old_size = old_malloc_size(elem);
> > rte_memcpy(new_ptr, ptr, old_size < size ? old_size : size);
> > rte_free(ptr);
> >
> > diff --git a/lib/pipeline/rte_swx_pipeline.c
> b/lib/pipeline/rte_swx_pipeline.c
> > index 8eb978a30c..aaa0107d02 100644
> > --- a/lib/pipeline/rte_swx_pipeline.c
> > +++ b/lib/pipeline/rte_swx_pipeline.c
> > @@ -6340,7 +6340,7 @@ instr_meter_translate(struct rte_swx_pipeline *p,
> > return 0;
> > }
> >
> > - CHECK(0, EINVAL);
> > + return -EINVAL;
> > }
> >
> > static inline struct meter *
> > @@ -8025,7 +8025,7 @@ instr_translate(struct rte_swx_pipeline *p,
> > instr,
> > data);
> >
> > - CHECK(0, EINVAL);
> > + return -EINVAL;
> > }
> >
> > static struct instruction_data *
> > --
> > 2.25.1
> >
>
>
> --
> David Marchand
On 18-Sep-21 8:41 AM, zhihongx.peng@intel.com wrote:
> From: Zhihong Peng <zhihongx.peng@intel.com>
>
> AddressSanitizer (ASan) is a google memory error detect
> standard tool. It could help to detect use-after-free and
> {heap,stack,global}-buffer overflow bugs in C/C++ programs,
> print detailed error information when error happens, large
> improve debug efficiency.
>
> By referring to its implementation algorithm
> (https://github.com/google/sanitizers/wiki/AddressSanitizerAlgorithm),
> enable heap-buffer-overflow and use-after-free functions on dpdk.
> DPDK ASAN function currently only supports on Linux x86_64.
>
> Here is an example of heap-buffer-overflow bug:
> ......
> char *p = rte_zmalloc(NULL, 7, 0);
> p[7] = 'a';
> ......
>
> Here is an example of use-after-free bug:
> ......
> char *p = rte_zmalloc(NULL, 7, 0);
> rte_free(p);
> *p = 'a';
> ......
>
> If you want to use this feature,
> you need to add below compilation options when compiling code:
> -Dbuildtype=debug -Db_lundef=false -Db_sanitize=address
> "-Dbuildtype=debug": Display code information when coredump occurs
> in the program.
> "-Db_lundef=false": It is enabled by default, and needs to be
> disabled when using asan.
>
> Signed-off-by: Xueqin Lin <xueqin.lin@intel.com>
> Signed-off-by: Zhihong Peng <zhihongx.peng@intel.com>
> ---
> doc/guides/prog_guide/asan.rst | 130 ++++++++++++++++++++++
> doc/guides/prog_guide/index.rst | 1 +
> lib/eal/common/malloc_elem.c | 26 ++++-
> lib/eal/common/malloc_elem.h | 184 +++++++++++++++++++++++++++++++-
> lib/eal/common/malloc_heap.c | 12 +++
> lib/eal/common/rte_malloc.c | 9 +-
> lib/pipeline/rte_swx_pipeline.c | 4 +-
> 7 files changed, 359 insertions(+), 7 deletions(-)
> create mode 100644 doc/guides/prog_guide/asan.rst
>
> diff --git a/doc/guides/prog_guide/asan.rst b/doc/guides/prog_guide/asan.rst
> new file mode 100644
> index 0000000000..a0589d9b8a
> --- /dev/null
> +++ b/doc/guides/prog_guide/asan.rst
> @@ -0,0 +1,130 @@
> +.. Copyright (c) <2021>, Intel Corporation
> + All rights reserved.
> +
> +Memory error detect standard tool - AddressSanitizer(Asan)
> +==========================================================
> +
> +AddressSanitizer (ASan) is a google memory error detect
> +standard tool. It could help to detect use-after-free and
> +{heap,stack,global}-buffer overflow bugs in C/C++ programs,
> +print detailed error information when error happens, large
> +improve debug efficiency.
> +
> +By referring to its implementation algorithm
> +(https://github.com/google/sanitizers/wiki/AddressSanitizerAlgorithm),
> +enabled heap-buffer-overflow and use-after-free functions on dpdk.
> +DPDK ASAN function currently only supports on Linux x86_64.
I think the description is a little to wordy and unnecessarily redundant
(pun intended!) in places. Suggested rewording:
`AddressSanitizer
<https://github.com/google/sanitizers/wiki/AddressSanitizer>` (ASan) is
a widely-used debugging tool to detect memory access errors. It helps
detect issues like use-after-free, various kinds of buffer overruns in
C/C++ programs, and other similar errors, as well as printing out
detailed debug information whenever an error is detected.
DPDK ASan functionality is currently only supported Linux x86_64.
> +
> +AddressSanitizer is a part of LLVM(3.1+)and GCC(4.8+).
> +
> +Example heap-buffer-overflow error
> +----------------------------------
> +
> +Following error was reported when Asan was enabled::
> +
> + Applied 9 bytes of memory, but accessed the 10th byte of memory,
> + so heap-buffer-overflow appeared.
> +
> +Below code results in this error::
> +
> + char *p = rte_zmalloc(NULL, 9, 0);
> + if (!p) {
> + printf("rte_zmalloc error.");
> + return -1;
> + }
> + p[9] = 'a';
> +
> +The error log::
> +
> + ==49433==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f773fafa249 at pc 0x5556b13bdae4 bp 0x7ffeb4965e40 sp 0x7ffeb4965e30 WRITE of size 1 at 0x7f773fafa249 thread T0
> + #0 0x5556b13bdae3 in asan_heap_buffer_overflow ../app/test/test_asan_heap_buffer_overflow.c:25
> + #1 0x5556b043e9d4 in cmd_autotest_parsed ../app/test/commands.c:71
> + #2 0x5556b1cdd4b0 in cmdline_parse ../lib/cmdline/cmdline_parse.c:290
> + #3 0x5556b1cd8987 in cmdline_valid_buffer ../lib/cmdline/cmdline.c:26
> + #4 0x5556b1ce477a in rdline_char_in ../lib/cmdline/cmdline_rdline.c:421
> + #5 0x5556b1cd923e in cmdline_in ../lib/cmdline/cmdline.c:149
> + #6 0x5556b1cd9769 in cmdline_interact ../lib/cmdline/cmdline.c:223
> + #7 0x5556b045f53b in main ../app/test/test.c:234
> + #8 0x7f7f1eba90b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
> + #9 0x5556b043e70d in _start (/home/pzh/yyy/x86_64-native-linuxapp-gcc/app/test/dpdk-test+0x7ce70d)
> +
> + Address 0x7f773fafa249 is a wild pointer.
> + SUMMARY: AddressSanitizer: heap-buffer-overflow ../app/test/test_asan_heap_buffer_overflow.c:25 in asan_heap_buffer_overflow
> + Shadow bytes around the buggy address:
> + 0x0fef67f573f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0fef67f57400: fa fa 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0fef67f57410: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0fef67f57420: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0fef67f57430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + =>0x0fef67f57440: 00 00 00 00 00 00 fa fa 00[01]fa 00 00 00 00 00
> + 0x0fef67f57450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0fef67f57460: 00 00 00 00 00 00 fa fa 00 00 00 00 00 00 00 00
> + 0x0fef67f57470: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0fef67f57480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +
> +Example use-after-free error
> +----------------------------
> +
> +Following error was reported when Asan was enabled::
> +
> + Applied for 9 bytes of memory, and accessed the first byte after
> + released, so heap-use-after-free appeared.
> +
> +Below code results in this error::
> +
> + char *p = rte_zmalloc(NULL, 9, 0);
> + if (!p) {
> + printf("rte_zmalloc error.");
> + return -1;
> + }
> + rte_free(p);
> + *p = 'a';
> +
> +The error log::
> +
> + ==49478==ERROR: AddressSanitizer: heap-use-after-free on address 0x7fe2ffafa240 at pc 0x56409b084bc8 bp 0x7ffef62c57d0 sp 0x7ffef62c57c0 WRITE of size 1 at 0x7fe2ffafa240 thread T0
> + #0 0x56409b084bc7 in asan_use_after_free ../app/test/test_asan_use_after_free.c:26
> + #1 0x56409a1059d4 in cmd_autotest_parsed ../app/test/commands.c:71
> + #2 0x56409b9a44b0 in cmdline_parse ../lib/cmdline/cmdline_parse.c:290
> + #3 0x56409b99f987 in cmdline_valid_buffer ../lib/cmdline/cmdline.c:26
> + #4 0x56409b9ab77a in rdline_char_in ../lib/cmdline/cmdline_rdline.c:421
> + #5 0x56409b9a023e in cmdline_in ../lib/cmdline/cmdline.c:149
> + #6 0x56409b9a0769 in cmdline_interact ../lib/cmdline/cmdline.c:223
> + #7 0x56409a12653b in main ../app/test/test.c:234
> + #8 0x7feafafc20b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
> + #9 0x56409a10570d in _start (/home/pzh/yyy/x86_64-native-linuxapp-gcc/app/test/dpdk-test+0x7ce70d)
> +
> + Address 0x7fe2ffafa240 is a wild pointer.
> + SUMMARY: AddressSanitizer: heap-use-after-free ../app/test/test_asan_use_after_free.c:26 in asan_use_after_free
> + Shadow bytes around the buggy address:
> + 0x0ffcdff573f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0ffcdff57400: fa fa 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0ffcdff57410: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0ffcdff57420: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0ffcdff57430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + =>0x0ffcdff57440: 00 00 00 00 00 00 00 00[fd]fd fd fd fd fd fd fd
> + 0x0ffcdff57450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0ffcdff57460: 00 00 00 00 00 00 fa fa 00 00 00 00 00 00 00 00
> + 0x0ffcdff57470: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0ffcdff57480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> + 0x0ffcdff57490: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +
> +Usage
> +-----
> +
> +meson build
> +^^^^^^^^^^^
> +
> +To enable Asan in meson build system, use following meson build command:
> +
> +Example usage::
> +
> + meson build -Dbuildtype=debug -Db_lundef=false -Db_sanitize=address
> + ninja -C build
> +
> +.. Note::
> +
> + a) The issue of asan wild pointer is that dpdk asan tool is not fully adapted to google asan.
> + For example: Address 0x7fe2ffafa240 is a wild pointer.
> + b) Centos8 needs to install libasan separately.
> + c) If the program uses cmdline, when a memory bug occurs, need to execute the "stty echo" command.
I think all of the terms used here should be properly capitalized, e.g.
"ASan" instead of "asan", "DPDK" instead of "dpdk", etc.
Also, i don't quite follow the 'a)' section - what exactly is a wild
pointer? What makes a pointer 'wild', and why do we care? If the point
is that DPDK's ASan implementation does not support all ASan features,
then maybe this could be worded better, e.g. like so:
a) Some of the features of ASan (for example, 'wild pointers') are not
currently supported by DPDK's implementation
> diff --git a/doc/guides/prog_guide/index.rst b/doc/guides/prog_guide/index.rst
> index 2dce507f46..df8a4b93e1 100644
> --- a/doc/guides/prog_guide/index.rst
> +++ b/doc/guides/prog_guide/index.rst
> @@ -71,3 +71,4 @@ Programmer's Guide
> lto
> profile_app
> glossary
> + asan
> diff --git a/lib/eal/common/malloc_elem.c b/lib/eal/common/malloc_elem.c
> index c2c9461f1d..bdd20a162e 100644
> --- a/lib/eal/common/malloc_elem.c
> +++ b/lib/eal/common/malloc_elem.c
> @@ -446,6 +446,8 @@ malloc_elem_alloc(struct malloc_elem *elem, size_t size, unsigned align,
> struct malloc_elem *new_free_elem =
> RTE_PTR_ADD(new_elem, size + MALLOC_ELEM_OVERHEAD);
>
> + asan_clear_split_alloczone(new_free_elem);
> +
> split_elem(elem, new_free_elem);
> malloc_elem_free_list_insert(new_free_elem);
>
> @@ -458,6 +460,8 @@ malloc_elem_alloc(struct malloc_elem *elem, size_t size, unsigned align,
> elem->state = ELEM_BUSY;
> elem->pad = old_elem_size;
>
> + asan_clear_alloczone(elem);
> +
> /* put a dummy header in padding, to point to real element header */
> if (elem->pad > 0) { /* pad will be at least 64-bytes, as everything
> * is cache-line aligned */
> @@ -470,12 +474,18 @@ malloc_elem_alloc(struct malloc_elem *elem, size_t size, unsigned align,
> return new_elem;
> }
>
> + asan_clear_split_alloczone(new_elem);
> +
> /* we are going to split the element in two. The original element
> * remains free, and the new element is the one allocated.
> * Re-insert original element, in case its new size makes it
> * belong on a different list.
> */
> +
> split_elem(elem, new_elem);
> +
> + asan_clear_alloczone(new_elem);
> +
> new_elem->state = ELEM_BUSY;
> malloc_elem_free_list_insert(elem);
>
> @@ -601,6 +611,8 @@ malloc_elem_hide_region(struct malloc_elem *elem, void *start, size_t len)
> if (next && next_elem_is_adjacent(elem)) {
> len_after = RTE_PTR_DIFF(next, hide_end);
> if (len_after >= MALLOC_ELEM_OVERHEAD + MIN_DATA_SIZE) {
> + asan_clear_split_alloczone(hide_end);
> +
> /* split after */
> split_elem(elem, hide_end);
>
> @@ -615,6 +627,8 @@ malloc_elem_hide_region(struct malloc_elem *elem, void *start, size_t len)
> if (prev && prev_elem_is_adjacent(elem)) {
> len_before = RTE_PTR_DIFF(hide_start, elem);
> if (len_before >= MALLOC_ELEM_OVERHEAD + MIN_DATA_SIZE) {
> + asan_clear_split_alloczone(hide_start);
> +
> /* split before */
> split_elem(elem, hide_start);
>
> @@ -628,6 +642,8 @@ malloc_elem_hide_region(struct malloc_elem *elem, void *start, size_t len)
> }
> }
>
> + asan_clear_alloczone(elem);
> +
> remove_elem(elem);
> }
>
> @@ -641,8 +657,10 @@ malloc_elem_resize(struct malloc_elem *elem, size_t size)
> const size_t new_size = size + elem->pad + MALLOC_ELEM_OVERHEAD;
>
> /* if we request a smaller size, then always return ok */
> - if (elem->size >= new_size)
> + if (elem->size >= new_size) {
> + asan_clear_alloczone(elem);
> return 0;
> + }
>
> /* check if there is a next element, it's free and adjacent */
> if (!elem->next || elem->next->state != ELEM_FREE ||
> @@ -661,9 +679,15 @@ malloc_elem_resize(struct malloc_elem *elem, size_t size)
> /* now we have a big block together. Lets cut it down a bit, by splitting */
> struct malloc_elem *split_pt = RTE_PTR_ADD(elem, new_size);
> split_pt = RTE_PTR_ALIGN_CEIL(split_pt, RTE_CACHE_LINE_SIZE);
> +
> + asan_clear_split_alloczone(split_pt);
> +
> split_elem(elem, split_pt);
> malloc_elem_free_list_insert(split_pt);
> }
> +
> + asan_clear_alloczone(elem);
> +
> return 0;
> }
>
> diff --git a/lib/eal/common/malloc_elem.h b/lib/eal/common/malloc_elem.h
> index a1e5f7f02c..01a739f2ea 100644
> --- a/lib/eal/common/malloc_elem.h
> +++ b/lib/eal/common/malloc_elem.h
> @@ -7,6 +7,14 @@
>
> #include <stdbool.h>
>
> +#ifdef __SANITIZE_ADDRESS__
> +#define RTE_MALLOC_ASAN
> +#elif defined(__has_feature)
> +# if __has_feature(address_sanitizer)
> +#define RTE_MALLOC_ASAN
> +# endif
> +#endif
> +
> #define MIN_DATA_SIZE (RTE_CACHE_LINE_SIZE)
>
> /* dummy definition of struct so we can use pointers to it in malloc_elem struct */
> @@ -36,10 +44,20 @@ struct malloc_elem {
> uint64_t header_cookie; /* Cookie marking start of data */
> /* trailer cookie at start + size */
> #endif
> +#ifdef RTE_MALLOC_ASAN
> + size_t user_size;
> + uint64_t asan_cookie[2]; /*must be next to header_cookie*/
> +#endif
> } __rte_cache_aligned;
>
> +static const unsigned int MALLOC_ELEM_HEADER_LEN = sizeof(struct malloc_elem);
> +
> #ifndef RTE_MALLOC_DEBUG
> -static const unsigned MALLOC_ELEM_TRAILER_LEN = 0;
> +#ifdef RTE_MALLOC_ASAN
> +static const unsigned int MALLOC_ELEM_TRAILER_LEN = RTE_CACHE_LINE_SIZE;
> +#else
> +static const unsigned int MALLOC_ELEM_TRAILER_LEN;
> +#endif
>
> /* dummy function - just check if pointer is non-null */
> static inline int
> @@ -55,7 +73,7 @@ set_trailer(struct malloc_elem *elem __rte_unused){ }
>
>
> #else
> -static const unsigned MALLOC_ELEM_TRAILER_LEN = RTE_CACHE_LINE_SIZE;
> +static const unsigned int MALLOC_ELEM_TRAILER_LEN = RTE_CACHE_LINE_SIZE;
This doesn't look like it's changing anything - maybe drop this
particular hun from the patch?
>
> #define MALLOC_HEADER_COOKIE 0xbadbadbadadd2e55ULL /**< Header cookie. */
> #define MALLOC_TRAILER_COOKIE 0xadd2e55badbadbadULL /**< Trailer cookie.*/
> @@ -90,9 +108,169 @@ malloc_elem_cookies_ok(const struct malloc_elem *elem)
>
> #endif
>
> -static const unsigned MALLOC_ELEM_HEADER_LEN = sizeof(struct malloc_elem);
> #define MALLOC_ELEM_OVERHEAD (MALLOC_ELEM_HEADER_LEN + MALLOC_ELEM_TRAILER_LEN)
>
> +#ifdef RTE_MALLOC_ASAN
> +
> +#define ASAN_SHADOW_GRAIN_SIZE 8
> +#define ASAN_MEM_FREE_FLAG 0xfd
> +#define ASAN_MEM_REDZONE_FLAG 0xfa
> +#define ASAN_MEM_TO_SHADOW(mem) (((mem) >> 3) + 0x00007fff8000)
Maybe make it more pointer-safe, like so?
#define ASAN_MEM_SHIFT(mem) ((void*)((uintptr_t)mem >> 3))
#define ASAN_BASE_ADDR 0x00007fff8000
#define ASAN_MEM_TO_SHADOW(mem) RTE_PTR_ADD(ASAN_MEM_SHIFT(mem),
ASAN_BASE_ADDR)
We could even add a new rte_common.h macros for RTE_PTR_LSHIFT/RSHIFT.
The advantage of this suggestion is that you won't need to make so many
typecasts any more.
> +
> +#if defined(__clang__)
> +__attribute__((no_sanitize("address", "hwaddress")))
> +#else
> +__attribute__((no_sanitize_address))
> +#endif
> +static inline void
> +asan_set_shadow(void *addr, char val)
> +{
> + *(char *)addr = val;
> +}
> +
> +static inline void
> +asan_set_zone(void *ptr, size_t len, uint32_t val)
> +{
> + size_t offset;
> + char *shadow;
> + size_t zone_len = len / ASAN_SHADOW_GRAIN_SIZE;
> + if (len % ASAN_SHADOW_GRAIN_SIZE != 0)
> + zone_len += 1;
> +
> + for (size_t i = 0; i < zone_len; i++) {
> + offset = i * ASAN_SHADOW_GRAIN_SIZE;
> + shadow = (char *)ASAN_MEM_TO_SHADOW(((int64_t)ptr + offset));
How about
shadow = ASAN_MEM_TO_SHADOW(RTE_PTR_ADD(ptr, offset));
? The 'shadow' variable can be void*, so you can drop the cast here too.
> + asan_set_shadow(shadow, val);
> + }
> +}
> +
> +/*
> + * When the memory is released, the release mark is
> + * set in the corresponding range of the shadow area.
> + */
> +static inline void
> +asan_set_freezone(void *ptr, size_t size)
> +{
> + asan_set_zone(ptr, size, ASAN_MEM_FREE_FLAG);
> +}
> +
> +/*
> + * When the memory is allocated, memory state must set as accessible.
> + */
> +static inline void
> +asan_clear_alloczone(struct malloc_elem *elem)
> +{
> + asan_set_zone((void *)elem, elem->size, 0x0);
> +}
> +
> +static inline void
> +asan_clear_split_alloczone(struct malloc_elem *elem)
> +{
> + void *ptr = RTE_PTR_SUB(elem, MALLOC_ELEM_TRAILER_LEN);
> + asan_set_zone(ptr, MALLOC_ELEM_OVERHEAD, 0x0);
> +}
> +
> +/*
> + * When the memory is allocated, the memory boundary is
> + * marked in the corresponding range of the shadow area.
> + */
> +static inline void
> +asan_set_redzone(struct malloc_elem *elem, size_t user_size)
> +{
> + uint64_t ptr;
> + char *shadow;
> + if (elem != NULL) {
> + if (elem->state != ELEM_PAD)
> + elem = RTE_PTR_ADD(elem, elem->pad);
> +
> + elem->user_size = user_size;
> +
> + /* Set mark before the start of the allocated memory */
> + ptr = (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN)
> + - ASAN_SHADOW_GRAIN_SIZE;
> + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
> + asan_set_shadow(shadow, ASAN_MEM_REDZONE_FLAG);
> + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
> + - ASAN_SHADOW_GRAIN_SIZE);
> + asan_set_shadow(shadow, ASAN_MEM_REDZONE_FLAG);
> +
> + /* Set mark after the end of the allocated memory */
> + ptr = (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN
> + + elem->user_size);
> + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
> + uint32_t val = (ptr % ASAN_SHADOW_GRAIN_SIZE);
I think DPDK code style is to declare all variables at the beginning of
the block.
> + val = (val == 0) ? ASAN_MEM_REDZONE_FLAG : val;
> + asan_set_shadow(shadow, val);
> + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
> + + ASAN_SHADOW_GRAIN_SIZE);
> + asan_set_shadow(shadow, ASAN_MEM_REDZONE_FLAG);
It seems to me that the code can be rewritten to be more understandable.
For example, you could opt for more meaningful variable names and not
reuse 'ptr' for everything. Ex.:
elem_start = RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN);
start_ptr = RTE_PTR_SUB(elem_start, ASAN_SHADOW_GRAIN_SIZE);
start_shadow = ASAN_MEM_TO_SHADOW(start_ptr);
// just following your code here, but why are we doing it twice?
start_redzone = RTE_PTR_SUB(start_shadow, ASAN_SHADOW_GRAIN_SIZE);
asan_set_shadow(start_shadow, ASAN_MEM_REDZONE_FLAG);
asan_set_shadow(start_redzone, ASAN_MEM_REDZONE_FLAG);
...
And similar for the rest of the code: more meaningful variable names,
more intermediate variables, less raw bit/ptr manipulation please :)
> + }
> +}
> +
> +/*
> + * When the memory is released, the mark of the memory boundary
> + * in the corresponding range of the shadow area is cleared.
> + */
> +static inline void
> +asan_clear_redzone(struct malloc_elem *elem)
> +{
> + uint64_t ptr;
> + char *shadow;
> + if (elem != NULL) {
> + elem = RTE_PTR_ADD(elem, elem->pad);
> +
> + /* Clear mark before the start of the allocated memory */
> + ptr = (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN)
> + - ASAN_SHADOW_GRAIN_SIZE;
> + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
> + asan_set_shadow(shadow, 0x00);
> + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
> + - ASAN_SHADOW_GRAIN_SIZE);
> + asan_set_shadow(shadow, 0x00);
> +
> + /* Clear mark after the end of the allocated memory */
> + ptr = (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN
> + + elem->user_size);
> + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
> + asan_set_shadow(shadow, 0x00);
> + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
> + + ASAN_SHADOW_GRAIN_SIZE);
> + asan_set_shadow(shadow, 0x00);
It seems to me that you're repeating a lot of code here that you already
have in `asan_set_redzone()` - maybe factor common bits out into a
function? For example,
static inlind void ptr_set_shadow(void *ptr, int shadow_flags) {
startptr = RTE_PTR_SUB(ptr, ASAN_SHADOW_GRAIN_SIZE);
shadow = ASAN_MEM_TO_SHADOW(startptr);
asan_set_shadow(shadow, shadow_flags);
shadow = RTE_PTR_SUB(shadow, ASAN_SHADOW_GRAIN_SIZE);
asan_set_shadow(shadow, shadow_flags);
}
...
and then call this from wherever you like? Because this pattern seems to
be repeating everywhere, the only difference being the ptr value and the
set_shadow flags.
> + }
> +}
> +
> +static inline size_t
> +old_malloc_size(struct malloc_elem *elem)
> +{
> + if (elem->state != ELEM_PAD)
> + elem = RTE_PTR_ADD(elem, elem->pad);
> +
> + return elem->user_size;
> +}
> +#else
> +static inline void
> +asan_set_freezone(void *ptr __rte_unused, size_t size __rte_unused) { }
> +
> +static inline void
> +asan_clear_alloczone(struct malloc_elem *elem __rte_unused) { }
> +
> +static inline void
> +asan_clear_split_alloczone(struct malloc_elem *elem __rte_unused) { }
> +
> +static inline void
> +asan_set_redzone(struct malloc_elem *elem __rte_unused,
> + size_t user_size __rte_unused) { }
> +
> +static inline void
> +asan_clear_redzone(struct malloc_elem *elem __rte_unused) { }
> +
> +static inline size_t
> +old_malloc_size(struct malloc_elem *elem)
> +{
> + return elem->size - elem->pad - MALLOC_ELEM_OVERHEAD;
> +}
> +#endif
> +
> /*
> * Given a pointer to the start of a memory block returned by malloc, get
> * the actual malloc_elem header for that block.
> diff --git a/lib/eal/common/malloc_heap.c b/lib/eal/common/malloc_heap.c
> index ee400f38ec..775d6789df 100644
> --- a/lib/eal/common/malloc_heap.c
> +++ b/lib/eal/common/malloc_heap.c
> @@ -237,6 +237,7 @@ heap_alloc(struct malloc_heap *heap, const char *type __rte_unused, size_t size,
> unsigned int flags, size_t align, size_t bound, bool contig)
> {
> struct malloc_elem *elem;
> + size_t user_size = size;
>
> size = RTE_CACHE_LINE_ROUNDUP(size);
> align = RTE_CACHE_LINE_ROUNDUP(align);
> @@ -250,6 +251,8 @@ heap_alloc(struct malloc_heap *heap, const char *type __rte_unused, size_t size,
>
> /* increase heap's count of allocated elements */
> heap->alloc_count++;
> +
> + asan_set_redzone(elem, user_size);
> }
>
> return elem == NULL ? NULL : (void *)(&elem[1]);
> @@ -270,6 +273,8 @@ heap_alloc_biggest(struct malloc_heap *heap, const char *type __rte_unused,
>
> /* increase heap's count of allocated elements */
> heap->alloc_count++;
> +
> + asan_set_redzone(elem, size);
> }
>
> return elem == NULL ? NULL : (void *)(&elem[1]);
> @@ -841,6 +846,8 @@ malloc_heap_free(struct malloc_elem *elem)
> if (!malloc_elem_cookies_ok(elem) || elem->state != ELEM_BUSY)
> return -1;
>
> + asan_clear_redzone(elem);
> +
> /* elem may be merged with previous element, so keep heap address */
> heap = elem->heap;
> msl = elem->msl;
> @@ -848,6 +855,9 @@ malloc_heap_free(struct malloc_elem *elem)
>
> rte_spinlock_lock(&(heap->lock));
>
> + void *asan_ptr = RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN + elem->pad);
> + size_t asan_data_len = elem->size - MALLOC_ELEM_OVERHEAD - elem->pad;
> +
> /* mark element as free */
> elem->state = ELEM_FREE;
>
> @@ -1001,6 +1011,8 @@ malloc_heap_free(struct malloc_elem *elem)
>
> rte_mcfg_mem_write_unlock();
> free_unlock:
> + asan_set_freezone(asan_ptr, asan_data_len);
> +
> rte_spinlock_unlock(&(heap->lock));
> return ret;
> }
> diff --git a/lib/eal/common/rte_malloc.c b/lib/eal/common/rte_malloc.c
> index 9d39e58c08..d0bec26920 100644
> --- a/lib/eal/common/rte_malloc.c
> +++ b/lib/eal/common/rte_malloc.c
> @@ -162,6 +162,8 @@ rte_calloc(const char *type, size_t num, size_t size, unsigned align)
> void *
> rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
> {
> + size_t user_size;
> +
> if (ptr == NULL)
> return rte_malloc_socket(NULL, size, align, socket);
>
> @@ -171,6 +173,8 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
> return NULL;
> }
>
> + user_size = size;
> +
> size = RTE_CACHE_LINE_ROUNDUP(size), align = RTE_CACHE_LINE_ROUNDUP(align);
>
> /* check requested socket id and alignment matches first, and if ok,
> @@ -181,6 +185,9 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
> RTE_PTR_ALIGN(ptr, align) == ptr &&
> malloc_heap_resize(elem, size) == 0) {
> rte_eal_trace_mem_realloc(size, align, socket, ptr);
> +
> + asan_set_redzone(elem, user_size);
> +
> return ptr;
> }
>
> @@ -192,7 +199,7 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
> if (new_ptr == NULL)
> return NULL;
> /* elem: |pad|data_elem|data|trailer| */
> - const size_t old_size = elem->size - elem->pad - MALLOC_ELEM_OVERHEAD;
> + const size_t old_size = old_malloc_size(elem);
> rte_memcpy(new_ptr, ptr, old_size < size ? old_size : size);
> rte_free(ptr);
>
> diff --git a/lib/pipeline/rte_swx_pipeline.c b/lib/pipeline/rte_swx_pipeline.c
> index 8eb978a30c..aaa0107d02 100644
> --- a/lib/pipeline/rte_swx_pipeline.c
> +++ b/lib/pipeline/rte_swx_pipeline.c
> @@ -6340,7 +6340,7 @@ instr_meter_translate(struct rte_swx_pipeline *p,
> return 0;
> }
>
> - CHECK(0, EINVAL);
> + return -EINVAL;
Why is this here? Accidental change?
> }
>
> static inline struct meter *
> @@ -8025,7 +8025,7 @@ instr_translate(struct rte_swx_pipeline *p,
> instr,
> data);
>
> - CHECK(0, EINVAL);
> + return -EINVAL;
Same as above - accidental change?
> }
>
> static struct instruction_data *
>
> -----Original Message-----
> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> Sent: Monday, September 27, 2021 8:03 PM
> To: Peng, ZhihongX <zhihongx.peng@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; stephen@networkplumber.org
> Cc: dev@dpdk.org; Lin, Xueqin <xueqin.lin@intel.com>
> Subject: Re: [PATCH v3] Enable AddressSanitizer feature on DPDK
>
> On 18-Sep-21 8:41 AM, zhihongx.peng@intel.com wrote:
> > From: Zhihong Peng <zhihongx.peng@intel.com>
> >
> > AddressSanitizer (ASan) is a google memory error detect standard tool.
> > It could help to detect use-after-free and {heap,stack,global}-buffer
> > overflow bugs in C/C++ programs, print detailed error information when
> > error happens, large improve debug efficiency.
> >
> > By referring to its implementation algorithm
> > (https://github.com/google/sanitizers/wiki/AddressSanitizerAlgorithm),
> > enable heap-buffer-overflow and use-after-free functions on dpdk.
> > DPDK ASAN function currently only supports on Linux x86_64.
> >
> > Here is an example of heap-buffer-overflow bug:
> > ......
> > char *p = rte_zmalloc(NULL, 7, 0);
> > p[7] = 'a';
> > ......
> >
> > Here is an example of use-after-free bug:
> > ......
> > char *p = rte_zmalloc(NULL, 7, 0);
> > rte_free(p);
> > *p = 'a';
> > ......
> >
> > If you want to use this feature,
> > you need to add below compilation options when compiling code:
> > -Dbuildtype=debug -Db_lundef=false -Db_sanitize=address
> > "-Dbuildtype=debug": Display code information when coredump occurs in
> > the program.
> > "-Db_lundef=false": It is enabled by default, and needs to be disabled
> > when using asan.
> >
> > Signed-off-by: Xueqin Lin <xueqin.lin@intel.com>
> > Signed-off-by: Zhihong Peng <zhihongx.peng@intel.com>
> > ---
> > doc/guides/prog_guide/asan.rst | 130 ++++++++++++++++++++++
> > doc/guides/prog_guide/index.rst | 1 +
> > lib/eal/common/malloc_elem.c | 26 ++++-
> > lib/eal/common/malloc_elem.h | 184
> +++++++++++++++++++++++++++++++-
> > lib/eal/common/malloc_heap.c | 12 +++
> > lib/eal/common/rte_malloc.c | 9 +-
> > lib/pipeline/rte_swx_pipeline.c | 4 +-
> > 7 files changed, 359 insertions(+), 7 deletions(-)
> > create mode 100644 doc/guides/prog_guide/asan.rst
> >
> > diff --git a/doc/guides/prog_guide/asan.rst
> > b/doc/guides/prog_guide/asan.rst new file mode 100644 index
> > 0000000000..a0589d9b8a
> > --- /dev/null
> > +++ b/doc/guides/prog_guide/asan.rst
> > @@ -0,0 +1,130 @@
> > +.. Copyright (c) <2021>, Intel Corporation
> > + All rights reserved.
> > +
> > +Memory error detect standard tool - AddressSanitizer(Asan)
> >
> +=========================================================
> =
> > +
> > +AddressSanitizer (ASan) is a google memory error detect standard
> > +tool. It could help to detect use-after-free and
> > +{heap,stack,global}-buffer overflow bugs in C/C++ programs, print
> > +detailed error information when error happens, large improve debug
> > +efficiency.
> > +
> > +By referring to its implementation algorithm
> > +(https://github.com/google/sanitizers/wiki/AddressSanitizerAlgorithm)
> > +, enabled heap-buffer-overflow and use-after-free functions on dpdk.
> > +DPDK ASAN function currently only supports on Linux x86_64.
>
> I think the description is a little to wordy and unnecessarily redundant (pun
> intended!) in places. Suggested rewording:
>
> `AddressSanitizer
> <https://github.com/google/sanitizers/wiki/AddressSanitizer>` (ASan) is a
> widely-used debugging tool to detect memory access errors. It helps detect
> issues like use-after-free, various kinds of buffer overruns in C/C++ programs,
> and other similar errors, as well as printing out detailed debug information
> whenever an error is detected.
>
> DPDK ASan functionality is currently only supported Linux x86_64.
>
The v6 version will be modified.
> > +
> > +AddressSanitizer is a part of LLVM(3.1+)and GCC(4.8+).
> > +
> > +Example heap-buffer-overflow error
> > +----------------------------------
> > +
> > +Following error was reported when Asan was enabled::
> > +
> > + Applied 9 bytes of memory, but accessed the 10th byte of memory,
> > + so heap-buffer-overflow appeared.
> > +
> > +Below code results in this error::
> > +
> > + char *p = rte_zmalloc(NULL, 9, 0);
> > + if (!p) {
> > + printf("rte_zmalloc error.");
> > + return -1;
> > + }
> > + p[9] = 'a';
> > +
> > +The error log::
> > +
> > + ==49433==ERROR: AddressSanitizer: heap-buffer-overflow on address
> 0x7f773fafa249 at pc 0x5556b13bdae4 bp 0x7ffeb4965e40 sp 0x7ffeb4965e30
> WRITE of size 1 at 0x7f773fafa249 thread T0
> > + #0 0x5556b13bdae3 in
> asan_heap_buffer_overflow ../app/test/test_asan_heap_buffer_overflow.
> c:25
> > + #1 0x5556b043e9d4 in
> cmd_autotest_parsed ../app/test/commands.c:71
> > + #2 0x5556b1cdd4b0 in
> cmdline_parse ../lib/cmdline/cmdline_parse.c:290
> > + #3 0x5556b1cd8987 in cmdline_valid_buffer ../lib/cmdline/cmdline.c:26
> > + #4 0x5556b1ce477a in rdline_char_in ../lib/cmdline/cmdline_rdline.c:421
> > + #5 0x5556b1cd923e in cmdline_in ../lib/cmdline/cmdline.c:149
> > + #6 0x5556b1cd9769 in cmdline_interact ../lib/cmdline/cmdline.c:223
> > + #7 0x5556b045f53b in main ../app/test/test.c:234
> > + #8 0x7f7f1eba90b2 in __libc_start_main (/lib/x86_64-linux-
> gnu/libc.so.6+0x270b2)
> > + #9 0x5556b043e70d in _start
> > + (/home/pzh/yyy/x86_64-native-linuxapp-gcc/app/test/dpdk-
> test+0x7ce70
> > + d)
> > +
> > + Address 0x7f773fafa249 is a wild pointer.
> > + SUMMARY: AddressSanitizer: heap-buffer-
> overflow ../app/test/test_asan_heap_buffer_overflow.c:25 in
> asan_heap_buffer_overflow
> > + Shadow bytes around the buggy address:
> > + 0x0fef67f573f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0fef67f57400: fa fa 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0fef67f57410: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0fef67f57420: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0fef67f57430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + =>0x0fef67f57440: 00 00 00 00 00 00 fa fa 00[01]fa 00 00 00 00 00
> > + 0x0fef67f57450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0fef67f57460: 00 00 00 00 00 00 fa fa 00 00 00 00 00 00 00 00
> > + 0x0fef67f57470: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0fef67f57480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > +
> > +Example use-after-free error
> > +----------------------------
> > +
> > +Following error was reported when Asan was enabled::
> > +
> > + Applied for 9 bytes of memory, and accessed the first byte after
> > + released, so heap-use-after-free appeared.
> > +
> > +Below code results in this error::
> > +
> > + char *p = rte_zmalloc(NULL, 9, 0);
> > + if (!p) {
> > + printf("rte_zmalloc error.");
> > + return -1;
> > + }
> > + rte_free(p);
> > + *p = 'a';
> > +
> > +The error log::
> > +
> > + ==49478==ERROR: AddressSanitizer: heap-use-after-free on address
> 0x7fe2ffafa240 at pc 0x56409b084bc8 bp 0x7ffef62c57d0 sp 0x7ffef62c57c0
> WRITE of size 1 at 0x7fe2ffafa240 thread T0
> > + #0 0x56409b084bc7 in
> asan_use_after_free ../app/test/test_asan_use_after_free.c:26
> > + #1 0x56409a1059d4 in
> cmd_autotest_parsed ../app/test/commands.c:71
> > + #2 0x56409b9a44b0 in
> cmdline_parse ../lib/cmdline/cmdline_parse.c:290
> > + #3 0x56409b99f987 in cmdline_valid_buffer ../lib/cmdline/cmdline.c:26
> > + #4 0x56409b9ab77a in
> rdline_char_in ../lib/cmdline/cmdline_rdline.c:421
> > + #5 0x56409b9a023e in cmdline_in ../lib/cmdline/cmdline.c:149
> > + #6 0x56409b9a0769 in cmdline_interact ../lib/cmdline/cmdline.c:223
> > + #7 0x56409a12653b in main ../app/test/test.c:234
> > + #8 0x7feafafc20b2 in __libc_start_main (/lib/x86_64-linux-
> gnu/libc.so.6+0x270b2)
> > + #9 0x56409a10570d in _start
> > + (/home/pzh/yyy/x86_64-native-linuxapp-gcc/app/test/dpdk-
> test+0x7ce70
> > + d)
> > +
> > + Address 0x7fe2ffafa240 is a wild pointer.
> > + SUMMARY: AddressSanitizer: heap-use-after-
> free ../app/test/test_asan_use_after_free.c:26 in asan_use_after_free
> > + Shadow bytes around the buggy address:
> > + 0x0ffcdff573f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0ffcdff57400: fa fa 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0ffcdff57410: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0ffcdff57420: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0ffcdff57430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + =>0x0ffcdff57440: 00 00 00 00 00 00 00 00[fd]fd fd fd fd fd fd fd
> > + 0x0ffcdff57450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0ffcdff57460: 00 00 00 00 00 00 fa fa 00 00 00 00 00 00 00 00
> > + 0x0ffcdff57470: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0ffcdff57480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > + 0x0ffcdff57490: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > +
> > +Usage
> > +-----
> > +
> > +meson build
> > +^^^^^^^^^^^
> > +
> > +To enable Asan in meson build system, use following meson build
> command:
> > +
> > +Example usage::
> > +
> > + meson build -Dbuildtype=debug -Db_lundef=false -Db_sanitize=address
> > + ninja -C build
> > +
> > +.. Note::
> > +
> > + a) The issue of asan wild pointer is that dpdk asan tool is not fully
> adapted to google asan.
> > + For example: Address 0x7fe2ffafa240 is a wild pointer.
> > + b) Centos8 needs to install libasan separately.
> > + c) If the program uses cmdline, when a memory bug occurs, need to
> execute the "stty echo" command.
>
> I think all of the terms used here should be properly capitalized, e.g.
> "ASan" instead of "asan", "DPDK" instead of "dpdk", etc.
>
> Also, i don't quite follow the 'a)' section - what exactly is a wild pointer? What
> makes a pointer 'wild', and why do we care? If the point is that DPDK's ASan
> implementation does not support all ASan features, then maybe this could
> be worded better, e.g. like so:
>
> a) Some of the features of ASan (for example, 'wild pointers') are not
> currently supported by DPDK's implementation
The v6 version will be modified.
> > diff --git a/doc/guides/prog_guide/index.rst
> > b/doc/guides/prog_guide/index.rst index 2dce507f46..df8a4b93e1 100644
> > --- a/doc/guides/prog_guide/index.rst
> > +++ b/doc/guides/prog_guide/index.rst
> > @@ -71,3 +71,4 @@ Programmer's Guide
> > lto
> > profile_app
> > glossary
> > + asan
> > diff --git a/lib/eal/common/malloc_elem.c
> > b/lib/eal/common/malloc_elem.c index c2c9461f1d..bdd20a162e 100644
> > --- a/lib/eal/common/malloc_elem.c
> > +++ b/lib/eal/common/malloc_elem.c
> > @@ -446,6 +446,8 @@ malloc_elem_alloc(struct malloc_elem *elem,
> size_t size, unsigned align,
> > struct malloc_elem *new_free_elem =
> > RTE_PTR_ADD(new_elem, size +
> MALLOC_ELEM_OVERHEAD);
> >
> > + asan_clear_split_alloczone(new_free_elem);
> > +
> > split_elem(elem, new_free_elem);
> > malloc_elem_free_list_insert(new_free_elem);
> >
> > @@ -458,6 +460,8 @@ malloc_elem_alloc(struct malloc_elem *elem,
> size_t size, unsigned align,
> > elem->state = ELEM_BUSY;
> > elem->pad = old_elem_size;
> >
> > + asan_clear_alloczone(elem);
> > +
> > /* put a dummy header in padding, to point to real element
> header */
> > if (elem->pad > 0) { /* pad will be at least 64-bytes, as
> everything
> > * is cache-line aligned */ @@ -470,12 +474,18
> > @@ malloc_elem_alloc(struct malloc_elem *elem, size_t size, unsigned
> align,
> > return new_elem;
> > }
> >
> > + asan_clear_split_alloczone(new_elem);
> > +
> > /* we are going to split the element in two. The original element
> > * remains free, and the new element is the one allocated.
> > * Re-insert original element, in case its new size makes it
> > * belong on a different list.
> > */
> > +
> > split_elem(elem, new_elem);
> > +
> > + asan_clear_alloczone(new_elem);
> > +
> > new_elem->state = ELEM_BUSY;
> > malloc_elem_free_list_insert(elem);
> >
> > @@ -601,6 +611,8 @@ malloc_elem_hide_region(struct malloc_elem
> *elem, void *start, size_t len)
> > if (next && next_elem_is_adjacent(elem)) {
> > len_after = RTE_PTR_DIFF(next, hide_end);
> > if (len_after >= MALLOC_ELEM_OVERHEAD +
> MIN_DATA_SIZE) {
> > + asan_clear_split_alloczone(hide_end);
> > +
> > /* split after */
> > split_elem(elem, hide_end);
> >
> > @@ -615,6 +627,8 @@ malloc_elem_hide_region(struct malloc_elem
> *elem, void *start, size_t len)
> > if (prev && prev_elem_is_adjacent(elem)) {
> > len_before = RTE_PTR_DIFF(hide_start, elem);
> > if (len_before >= MALLOC_ELEM_OVERHEAD +
> MIN_DATA_SIZE) {
> > + asan_clear_split_alloczone(hide_start);
> > +
> > /* split before */
> > split_elem(elem, hide_start);
> >
> > @@ -628,6 +642,8 @@ malloc_elem_hide_region(struct malloc_elem
> *elem, void *start, size_t len)
> > }
> > }
> >
> > + asan_clear_alloczone(elem);
> > +
> > remove_elem(elem);
> > }
> >
> > @@ -641,8 +657,10 @@ malloc_elem_resize(struct malloc_elem *elem,
> size_t size)
> > const size_t new_size = size + elem->pad +
> MALLOC_ELEM_OVERHEAD;
> >
> > /* if we request a smaller size, then always return ok */
> > - if (elem->size >= new_size)
> > + if (elem->size >= new_size) {
> > + asan_clear_alloczone(elem);
> > return 0;
> > + }
> >
> > /* check if there is a next element, it's free and adjacent */
> > if (!elem->next || elem->next->state != ELEM_FREE || @@ -661,9
> > +679,15 @@ malloc_elem_resize(struct malloc_elem *elem, size_t size)
> > /* now we have a big block together. Lets cut it down a bit,
> by splitting */
> > struct malloc_elem *split_pt = RTE_PTR_ADD(elem,
> new_size);
> > split_pt = RTE_PTR_ALIGN_CEIL(split_pt,
> RTE_CACHE_LINE_SIZE);
> > +
> > + asan_clear_split_alloczone(split_pt);
> > +
> > split_elem(elem, split_pt);
> > malloc_elem_free_list_insert(split_pt);
> > }
> > +
> > + asan_clear_alloczone(elem);
> > +
> > return 0;
> > }
> >
> > diff --git a/lib/eal/common/malloc_elem.h
> > b/lib/eal/common/malloc_elem.h index a1e5f7f02c..01a739f2ea 100644
> > --- a/lib/eal/common/malloc_elem.h
> > +++ b/lib/eal/common/malloc_elem.h
> > @@ -7,6 +7,14 @@
> >
> > #include <stdbool.h>
> >
> > +#ifdef __SANITIZE_ADDRESS__
> > +#define RTE_MALLOC_ASAN
> > +#elif defined(__has_feature)
> > +# if __has_feature(address_sanitizer) #define RTE_MALLOC_ASAN #
> endif
> > +#endif
> > +
> > #define MIN_DATA_SIZE (RTE_CACHE_LINE_SIZE)
> >
> > /* dummy definition of struct so we can use pointers to it in
> > malloc_elem struct */ @@ -36,10 +44,20 @@ struct malloc_elem {
> > uint64_t header_cookie; /* Cookie marking start of data */
> > /* trailer cookie at start + size */
> > #endif
> > +#ifdef RTE_MALLOC_ASAN
> > + size_t user_size;
> > + uint64_t asan_cookie[2]; /*must be next to header_cookie*/ #endif
> > } __rte_cache_aligned;
> >
> > +static const unsigned int MALLOC_ELEM_HEADER_LEN = sizeof(struct
> > +malloc_elem);
> > +
> > #ifndef RTE_MALLOC_DEBUG
> > -static const unsigned MALLOC_ELEM_TRAILER_LEN = 0;
> > +#ifdef RTE_MALLOC_ASAN
> > +static const unsigned int MALLOC_ELEM_TRAILER_LEN =
> > +RTE_CACHE_LINE_SIZE; #else static const unsigned int
> > +MALLOC_ELEM_TRAILER_LEN; #endif
> >
> > /* dummy function - just check if pointer is non-null */
> > static inline int
> > @@ -55,7 +73,7 @@ set_trailer(struct malloc_elem *elem __rte_unused){
> > }
> >
> >
> > #else
> > -static const unsigned MALLOC_ELEM_TRAILER_LEN =
> RTE_CACHE_LINE_SIZE;
> > +static const unsigned int MALLOC_ELEM_TRAILER_LEN =
> > +RTE_CACHE_LINE_SIZE;
>
> This doesn't look like it's changing anything - maybe drop this particular hun
> from the patch?
>
Solve the issue of static code inspection.
> >
> > #define MALLOC_HEADER_COOKIE 0xbadbadbadadd2e55ULL /**<
> Header cookie. */
> > #define MALLOC_TRAILER_COOKIE 0xadd2e55badbadbadULL /**< Trailer
> > cookie.*/ @@ -90,9 +108,169 @@ malloc_elem_cookies_ok(const struct
> > malloc_elem *elem)
> >
> > #endif
> >
> > -static const unsigned MALLOC_ELEM_HEADER_LEN = sizeof(struct
> malloc_elem);
> > #define MALLOC_ELEM_OVERHEAD (MALLOC_ELEM_HEADER_LEN +
> > MALLOC_ELEM_TRAILER_LEN)
> >
> > +#ifdef RTE_MALLOC_ASAN
> > +
> > +#define ASAN_SHADOW_GRAIN_SIZE 8
> > +#define ASAN_MEM_FREE_FLAG 0xfd
> > +#define ASAN_MEM_REDZONE_FLAG 0xfa
> > +#define ASAN_MEM_TO_SHADOW(mem) (((mem) >> 3) +
> 0x00007fff8000)
>
> Maybe make it more pointer-safe, like so?
>
> #define ASAN_MEM_SHIFT(mem) ((void*)((uintptr_t)mem >> 3)) #define
> ASAN_BASE_ADDR 0x00007fff8000 #define ASAN_MEM_TO_SHADOW(mem)
> RTE_PTR_ADD(ASAN_MEM_SHIFT(mem),
> ASAN_BASE_ADDR)
>
The v6 version will be modified.
> We could even add a new rte_common.h macros for
> RTE_PTR_LSHIFT/RSHIFT.
> The advantage of this suggestion is that you won't need to make so many
> typecasts any more.
>
> > +
> > +#if defined(__clang__)
> > +__attribute__((no_sanitize("address", "hwaddress"))) #else
> > +__attribute__((no_sanitize_address))
> > +#endif
> > +static inline void
> > +asan_set_shadow(void *addr, char val) {
> > + *(char *)addr = val;
> > +}
> > +
> > +static inline void
> > +asan_set_zone(void *ptr, size_t len, uint32_t val) {
> > + size_t offset;
> > + char *shadow;
> > + size_t zone_len = len / ASAN_SHADOW_GRAIN_SIZE;
> > + if (len % ASAN_SHADOW_GRAIN_SIZE != 0)
> > + zone_len += 1;
> > +
> > + for (size_t i = 0; i < zone_len; i++) {
> > + offset = i * ASAN_SHADOW_GRAIN_SIZE;
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(((int64_t)ptr +
> offset));
>
> How about
>
> shadow = ASAN_MEM_TO_SHADOW(RTE_PTR_ADD(ptr, offset));
>
> ? The 'shadow' variable can be void*, so you can drop the cast here too.
>
The v6 version will be modified.
> > + asan_set_shadow(shadow, val);
> > + }
> > +}
> > +
> > +/*
> > + * When the memory is released, the release mark is
> > + * set in the corresponding range of the shadow area.
> > + */
> > +static inline void
> > +asan_set_freezone(void *ptr, size_t size) {
> > + asan_set_zone(ptr, size, ASAN_MEM_FREE_FLAG); }
> > +
> > +/*
> > + * When the memory is allocated, memory state must set as accessible.
> > + */
> > +static inline void
> > +asan_clear_alloczone(struct malloc_elem *elem)
> > +{
> > + asan_set_zone((void *)elem, elem->size, 0x0);
> > +}
> > +
> > +static inline void
> > +asan_clear_split_alloczone(struct malloc_elem *elem)
> > +{
> > + void *ptr = RTE_PTR_SUB(elem, MALLOC_ELEM_TRAILER_LEN);
> > + asan_set_zone(ptr, MALLOC_ELEM_OVERHEAD, 0x0);
> > +}
> > +
> > +/*
> > + * When the memory is allocated, the memory boundary is
> > + * marked in the corresponding range of the shadow area.
> > + */
> > +static inline void
> > +asan_set_redzone(struct malloc_elem *elem, size_t user_size)
> > +{
> > + uint64_t ptr;
> > + char *shadow;
> > + if (elem != NULL) {
> > + if (elem->state != ELEM_PAD)
> > + elem = RTE_PTR_ADD(elem, elem->pad);
> > +
> > + elem->user_size = user_size;
> > +
> > + /* Set mark before the start of the allocated memory */
> > + ptr = (uint64_t)RTE_PTR_ADD(elem,
> MALLOC_ELEM_HEADER_LEN)
> > + - ASAN_SHADOW_GRAIN_SIZE;
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
> > + asan_set_shadow(shadow, ASAN_MEM_REDZONE_FLAG);
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
> > + - ASAN_SHADOW_GRAIN_SIZE);
> > + asan_set_shadow(shadow, ASAN_MEM_REDZONE_FLAG);
> > +
> > + /* Set mark after the end of the allocated memory */
> > + ptr = (uint64_t)RTE_PTR_ADD(elem,
> MALLOC_ELEM_HEADER_LEN
> > + + elem->user_size);
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
> > + uint32_t val = (ptr % ASAN_SHADOW_GRAIN_SIZE);
>
> I think DPDK code style is to declare all variables at the beginning of
> the block.
The v6 version will be modified.
>
> > + val = (val == 0) ? ASAN_MEM_REDZONE_FLAG : val;
> > + asan_set_shadow(shadow, val);
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
> > + + ASAN_SHADOW_GRAIN_SIZE);
> > + asan_set_shadow(shadow, ASAN_MEM_REDZONE_FLAG);
>
> It seems to me that the code can be rewritten to be more understandable.
> For example, you could opt for more meaningful variable names and not
> reuse 'ptr' for everything. Ex.:
>
> elem_start = RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN);
> start_ptr = RTE_PTR_SUB(elem_start, ASAN_SHADOW_GRAIN_SIZE);
> start_shadow = ASAN_MEM_TO_SHADOW(start_ptr);
> // just following your code here, but why are we doing it twice?
> start_redzone = RTE_PTR_SUB(start_shadow,
> ASAN_SHADOW_GRAIN_SIZE);
>
> asan_set_shadow(start_shadow, ASAN_MEM_REDZONE_FLAG);
> asan_set_shadow(start_redzone, ASAN_MEM_REDZONE_FLAG);
>
> ...
>
> And similar for the rest of the code: more meaningful variable names,
> more intermediate variables, less raw bit/ptr manipulation please :)
The v6 version will be modified.
> > + }
> > +}
> > +
> > +/*
> > + * When the memory is released, the mark of the memory boundary
> > + * in the corresponding range of the shadow area is cleared.
> > + */
> > +static inline void
> > +asan_clear_redzone(struct malloc_elem *elem)
> > +{
> > + uint64_t ptr;
> > + char *shadow;
> > + if (elem != NULL) {
> > + elem = RTE_PTR_ADD(elem, elem->pad);
> > +
> > + /* Clear mark before the start of the allocated memory */
> > + ptr = (uint64_t)RTE_PTR_ADD(elem,
> MALLOC_ELEM_HEADER_LEN)
> > + - ASAN_SHADOW_GRAIN_SIZE;
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
> > + asan_set_shadow(shadow, 0x00);
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
> > + - ASAN_SHADOW_GRAIN_SIZE);
> > + asan_set_shadow(shadow, 0x00);
> > +
> > + /* Clear mark after the end of the allocated memory */
> > + ptr = (uint64_t)RTE_PTR_ADD(elem,
> MALLOC_ELEM_HEADER_LEN
> > + + elem->user_size);
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
> > + asan_set_shadow(shadow, 0x00);
> > + shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
> > + + ASAN_SHADOW_GRAIN_SIZE);
> > + asan_set_shadow(shadow, 0x00);
>
> It seems to me that you're repeating a lot of code here that you already
> have in `asan_set_redzone()` - maybe factor common bits out into a
> function? For example,
>
> static inlind void ptr_set_shadow(void *ptr, int shadow_flags) {
> startptr = RTE_PTR_SUB(ptr, ASAN_SHADOW_GRAIN_SIZE);
> shadow = ASAN_MEM_TO_SHADOW(startptr);
> asan_set_shadow(shadow, shadow_flags);
> shadow = RTE_PTR_SUB(shadow, ASAN_SHADOW_GRAIN_SIZE);
> asan_set_shadow(shadow, shadow_flags);
> }
>
> ...
>
> and then call this from wherever you like? Because this pattern seems to
> be repeating everywhere, the only difference being the ptr value and the
> set_shadow flags.
The variable names have been modified to make the code clearer.
Extracting public functions is not particularly useful, and
this is a logical implementation, not general code.
static inline void
asan_clear_redzone(struct malloc_elem *elem)
{
uintptr_t head_redzone;
uintptr_t tail_redzone;
void *head_shadow;
void *tail_shadow;
if (elem != NULL) {
elem = RTE_PTR_ADD(elem, elem->pad);
/* Clear mark before the start of the allocated memory */
head_redzone = (uintptr_t)RTE_PTR_ADD(elem,
MALLOC_ELEM_HEADER_LEN - ASAN_SHADOW_GRAIN_SIZE);
head_shadow = ASAN_MEM_TO_SHADOW(head_redzone);
asan_set_shadow(head_shadow, 0x00);
head_shadow = ASAN_MEM_TO_SHADOW(head_redzone
- ASAN_SHADOW_GRAIN_SIZE);
asan_set_shadow(head_shadow, 0x00);
/* Clear mark after the end of the allocated memory */
tail_redzone = (uintptr_t)RTE_PTR_ADD(elem,
MALLOC_ELEM_HEADER_LEN + elem->user_size);
tail_shadow = ASAN_MEM_TO_SHADOW(tail_redzone);
asan_set_shadow(tail_shadow, 0x00);
tail_shadow = ASAN_MEM_TO_SHADOW(tail_redzone
+ ASAN_SHADOW_GRAIN_SIZE);
asan_set_shadow(tail_shadow, 0x00);
}
}
> > + }
> > +}
> > +
> > +static inline size_t
> > +old_malloc_size(struct malloc_elem *elem)
> > +{
> > + if (elem->state != ELEM_PAD)
> > + elem = RTE_PTR_ADD(elem, elem->pad);
> > +
> > + return elem->user_size;
> > +}
> > +#else
> > +static inline void
> > +asan_set_freezone(void *ptr __rte_unused, size_t size __rte_unused) { }
> > +
> > +static inline void
> > +asan_clear_alloczone(struct malloc_elem *elem __rte_unused) { }
> > +
> > +static inline void
> > +asan_clear_split_alloczone(struct malloc_elem *elem __rte_unused) { }
> > +
> > +static inline void
> > +asan_set_redzone(struct malloc_elem *elem __rte_unused,
> > + size_t user_size __rte_unused) { }
> > +
> > +static inline void
> > +asan_clear_redzone(struct malloc_elem *elem __rte_unused) { }
> > +
> > +static inline size_t
> > +old_malloc_size(struct malloc_elem *elem)
> > +{
> > + return elem->size - elem->pad - MALLOC_ELEM_OVERHEAD;
> > +}
> > +#endif
> > +
> > /*
> > * Given a pointer to the start of a memory block returned by malloc, get
> > * the actual malloc_elem header for that block.
> > diff --git a/lib/eal/common/malloc_heap.c
> b/lib/eal/common/malloc_heap.c
> > index ee400f38ec..775d6789df 100644
> > --- a/lib/eal/common/malloc_heap.c
> > +++ b/lib/eal/common/malloc_heap.c
> > @@ -237,6 +237,7 @@ heap_alloc(struct malloc_heap *heap, const char
> *type __rte_unused, size_t size,
> > unsigned int flags, size_t align, size_t bound, bool contig)
> > {
> > struct malloc_elem *elem;
> > + size_t user_size = size;
> >
> > size = RTE_CACHE_LINE_ROUNDUP(size);
> > align = RTE_CACHE_LINE_ROUNDUP(align);
> > @@ -250,6 +251,8 @@ heap_alloc(struct malloc_heap *heap, const char
> *type __rte_unused, size_t size,
> >
> > /* increase heap's count of allocated elements */
> > heap->alloc_count++;
> > +
> > + asan_set_redzone(elem, user_size);
> > }
> >
> > return elem == NULL ? NULL : (void *)(&elem[1]);
> > @@ -270,6 +273,8 @@ heap_alloc_biggest(struct malloc_heap *heap,
> const char *type __rte_unused,
> >
> > /* increase heap's count of allocated elements */
> > heap->alloc_count++;
> > +
> > + asan_set_redzone(elem, size);
> > }
> >
> > return elem == NULL ? NULL : (void *)(&elem[1]);
> > @@ -841,6 +846,8 @@ malloc_heap_free(struct malloc_elem *elem)
> > if (!malloc_elem_cookies_ok(elem) || elem->state != ELEM_BUSY)
> > return -1;
> >
> > + asan_clear_redzone(elem);
> > +
> > /* elem may be merged with previous element, so keep heap
> address */
> > heap = elem->heap;
> > msl = elem->msl;
> > @@ -848,6 +855,9 @@ malloc_heap_free(struct malloc_elem *elem)
> >
> > rte_spinlock_lock(&(heap->lock));
> >
> > + void *asan_ptr = RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN
> + elem->pad);
> > + size_t asan_data_len = elem->size - MALLOC_ELEM_OVERHEAD -
> elem->pad;
> > +
> > /* mark element as free */
> > elem->state = ELEM_FREE;
> >
> > @@ -1001,6 +1011,8 @@ malloc_heap_free(struct malloc_elem *elem)
> >
> > rte_mcfg_mem_write_unlock();
> > free_unlock:
> > + asan_set_freezone(asan_ptr, asan_data_len);
> > +
> > rte_spinlock_unlock(&(heap->lock));
> > return ret;
> > }
> > diff --git a/lib/eal/common/rte_malloc.c b/lib/eal/common/rte_malloc.c
> > index 9d39e58c08..d0bec26920 100644
> > --- a/lib/eal/common/rte_malloc.c
> > +++ b/lib/eal/common/rte_malloc.c
> > @@ -162,6 +162,8 @@ rte_calloc(const char *type, size_t num, size_t size,
> unsigned align)
> > void *
> > rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
> > {
> > + size_t user_size;
> > +
> > if (ptr == NULL)
> > return rte_malloc_socket(NULL, size, align, socket);
> >
> > @@ -171,6 +173,8 @@ rte_realloc_socket(void *ptr, size_t size, unsigned
> int align, int socket)
> > return NULL;
> > }
> >
> > + user_size = size;
> > +
> > size = RTE_CACHE_LINE_ROUNDUP(size), align =
> RTE_CACHE_LINE_ROUNDUP(align);
> >
> > /* check requested socket id and alignment matches first, and if ok,
> > @@ -181,6 +185,9 @@ rte_realloc_socket(void *ptr, size_t size, unsigned
> int align, int socket)
> > RTE_PTR_ALIGN(ptr, align) == ptr &&
> > malloc_heap_resize(elem, size) == 0) {
> > rte_eal_trace_mem_realloc(size, align, socket, ptr);
> > +
> > + asan_set_redzone(elem, user_size);
> > +
> > return ptr;
> > }
> >
> > @@ -192,7 +199,7 @@ rte_realloc_socket(void *ptr, size_t size, unsigned
> int align, int socket)
> > if (new_ptr == NULL)
> > return NULL;
> > /* elem: |pad|data_elem|data|trailer| */
> > - const size_t old_size = elem->size - elem->pad -
> MALLOC_ELEM_OVERHEAD;
> > + const size_t old_size = old_malloc_size(elem);
> > rte_memcpy(new_ptr, ptr, old_size < size ? old_size : size);
> > rte_free(ptr);
> >
> > diff --git a/lib/pipeline/rte_swx_pipeline.c
> b/lib/pipeline/rte_swx_pipeline.c
> > index 8eb978a30c..aaa0107d02 100644
> > --- a/lib/pipeline/rte_swx_pipeline.c
> > +++ b/lib/pipeline/rte_swx_pipeline.c
> > @@ -6340,7 +6340,7 @@ instr_meter_translate(struct rte_swx_pipeline *p,
> > return 0;
> > }
> >
> > - CHECK(0, EINVAL);
> > + return -EINVAL;
>
> Why is this here? Accidental change?
>
After adding ASan, the gcc compilation check will be stricter.
"Control reaches end of non-void function" error occurs here. Will be split into two patches.
> > }
> >
> > static inline struct meter *
> > @@ -8025,7 +8025,7 @@ instr_translate(struct rte_swx_pipeline *p,
> > instr,
> > data);
> >
> > - CHECK(0, EINVAL);
> > + return -EINVAL;
>
> Same as above - accidental change?
>
> > }
> >
> > static struct instruction_data *
> >
>
> --
> Thanks,
> Anatoly
new file mode 100644
@@ -0,0 +1,130 @@
+.. Copyright (c) <2021>, Intel Corporation
+ All rights reserved.
+
+Memory error detect standard tool - AddressSanitizer(Asan)
+==========================================================
+
+AddressSanitizer (ASan) is a google memory error detect
+standard tool. It could help to detect use-after-free and
+{heap,stack,global}-buffer overflow bugs in C/C++ programs,
+print detailed error information when error happens, large
+improve debug efficiency.
+
+By referring to its implementation algorithm
+(https://github.com/google/sanitizers/wiki/AddressSanitizerAlgorithm),
+enabled heap-buffer-overflow and use-after-free functions on dpdk.
+DPDK ASAN function currently only supports on Linux x86_64.
+
+AddressSanitizer is a part of LLVM(3.1+)and GCC(4.8+).
+
+Example heap-buffer-overflow error
+----------------------------------
+
+Following error was reported when Asan was enabled::
+
+ Applied 9 bytes of memory, but accessed the 10th byte of memory,
+ so heap-buffer-overflow appeared.
+
+Below code results in this error::
+
+ char *p = rte_zmalloc(NULL, 9, 0);
+ if (!p) {
+ printf("rte_zmalloc error.");
+ return -1;
+ }
+ p[9] = 'a';
+
+The error log::
+
+ ==49433==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f773fafa249 at pc 0x5556b13bdae4 bp 0x7ffeb4965e40 sp 0x7ffeb4965e30 WRITE of size 1 at 0x7f773fafa249 thread T0
+ #0 0x5556b13bdae3 in asan_heap_buffer_overflow ../app/test/test_asan_heap_buffer_overflow.c:25
+ #1 0x5556b043e9d4 in cmd_autotest_parsed ../app/test/commands.c:71
+ #2 0x5556b1cdd4b0 in cmdline_parse ../lib/cmdline/cmdline_parse.c:290
+ #3 0x5556b1cd8987 in cmdline_valid_buffer ../lib/cmdline/cmdline.c:26
+ #4 0x5556b1ce477a in rdline_char_in ../lib/cmdline/cmdline_rdline.c:421
+ #5 0x5556b1cd923e in cmdline_in ../lib/cmdline/cmdline.c:149
+ #6 0x5556b1cd9769 in cmdline_interact ../lib/cmdline/cmdline.c:223
+ #7 0x5556b045f53b in main ../app/test/test.c:234
+ #8 0x7f7f1eba90b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
+ #9 0x5556b043e70d in _start (/home/pzh/yyy/x86_64-native-linuxapp-gcc/app/test/dpdk-test+0x7ce70d)
+
+ Address 0x7f773fafa249 is a wild pointer.
+ SUMMARY: AddressSanitizer: heap-buffer-overflow ../app/test/test_asan_heap_buffer_overflow.c:25 in asan_heap_buffer_overflow
+ Shadow bytes around the buggy address:
+ 0x0fef67f573f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ 0x0fef67f57400: fa fa 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ 0x0fef67f57410: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ 0x0fef67f57420: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ 0x0fef67f57430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ =>0x0fef67f57440: 00 00 00 00 00 00 fa fa 00[01]fa 00 00 00 00 00
+ 0x0fef67f57450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ 0x0fef67f57460: 00 00 00 00 00 00 fa fa 00 00 00 00 00 00 00 00
+ 0x0fef67f57470: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ 0x0fef67f57480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+
+Example use-after-free error
+----------------------------
+
+Following error was reported when Asan was enabled::
+
+ Applied for 9 bytes of memory, and accessed the first byte after
+ released, so heap-use-after-free appeared.
+
+Below code results in this error::
+
+ char *p = rte_zmalloc(NULL, 9, 0);
+ if (!p) {
+ printf("rte_zmalloc error.");
+ return -1;
+ }
+ rte_free(p);
+ *p = 'a';
+
+The error log::
+
+ ==49478==ERROR: AddressSanitizer: heap-use-after-free on address 0x7fe2ffafa240 at pc 0x56409b084bc8 bp 0x7ffef62c57d0 sp 0x7ffef62c57c0 WRITE of size 1 at 0x7fe2ffafa240 thread T0
+ #0 0x56409b084bc7 in asan_use_after_free ../app/test/test_asan_use_after_free.c:26
+ #1 0x56409a1059d4 in cmd_autotest_parsed ../app/test/commands.c:71
+ #2 0x56409b9a44b0 in cmdline_parse ../lib/cmdline/cmdline_parse.c:290
+ #3 0x56409b99f987 in cmdline_valid_buffer ../lib/cmdline/cmdline.c:26
+ #4 0x56409b9ab77a in rdline_char_in ../lib/cmdline/cmdline_rdline.c:421
+ #5 0x56409b9a023e in cmdline_in ../lib/cmdline/cmdline.c:149
+ #6 0x56409b9a0769 in cmdline_interact ../lib/cmdline/cmdline.c:223
+ #7 0x56409a12653b in main ../app/test/test.c:234
+ #8 0x7feafafc20b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
+ #9 0x56409a10570d in _start (/home/pzh/yyy/x86_64-native-linuxapp-gcc/app/test/dpdk-test+0x7ce70d)
+
+ Address 0x7fe2ffafa240 is a wild pointer.
+ SUMMARY: AddressSanitizer: heap-use-after-free ../app/test/test_asan_use_after_free.c:26 in asan_use_after_free
+ Shadow bytes around the buggy address:
+ 0x0ffcdff573f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ 0x0ffcdff57400: fa fa 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ 0x0ffcdff57410: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ 0x0ffcdff57420: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ 0x0ffcdff57430: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ =>0x0ffcdff57440: 00 00 00 00 00 00 00 00[fd]fd fd fd fd fd fd fd
+ 0x0ffcdff57450: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ 0x0ffcdff57460: 00 00 00 00 00 00 fa fa 00 00 00 00 00 00 00 00
+ 0x0ffcdff57470: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ 0x0ffcdff57480: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+ 0x0ffcdff57490: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
+
+Usage
+-----
+
+meson build
+^^^^^^^^^^^
+
+To enable Asan in meson build system, use following meson build command:
+
+Example usage::
+
+ meson build -Dbuildtype=debug -Db_lundef=false -Db_sanitize=address
+ ninja -C build
+
+.. Note::
+
+ a) The issue of asan wild pointer is that dpdk asan tool is not fully adapted to google asan.
+ For example: Address 0x7fe2ffafa240 is a wild pointer.
+ b) Centos8 needs to install libasan separately.
+ c) If the program uses cmdline, when a memory bug occurs, need to execute the "stty echo" command.
@@ -71,3 +71,4 @@ Programmer's Guide
lto
profile_app
glossary
+ asan
@@ -446,6 +446,8 @@ malloc_elem_alloc(struct malloc_elem *elem, size_t size, unsigned align,
struct malloc_elem *new_free_elem =
RTE_PTR_ADD(new_elem, size + MALLOC_ELEM_OVERHEAD);
+ asan_clear_split_alloczone(new_free_elem);
+
split_elem(elem, new_free_elem);
malloc_elem_free_list_insert(new_free_elem);
@@ -458,6 +460,8 @@ malloc_elem_alloc(struct malloc_elem *elem, size_t size, unsigned align,
elem->state = ELEM_BUSY;
elem->pad = old_elem_size;
+ asan_clear_alloczone(elem);
+
/* put a dummy header in padding, to point to real element header */
if (elem->pad > 0) { /* pad will be at least 64-bytes, as everything
* is cache-line aligned */
@@ -470,12 +474,18 @@ malloc_elem_alloc(struct malloc_elem *elem, size_t size, unsigned align,
return new_elem;
}
+ asan_clear_split_alloczone(new_elem);
+
/* we are going to split the element in two. The original element
* remains free, and the new element is the one allocated.
* Re-insert original element, in case its new size makes it
* belong on a different list.
*/
+
split_elem(elem, new_elem);
+
+ asan_clear_alloczone(new_elem);
+
new_elem->state = ELEM_BUSY;
malloc_elem_free_list_insert(elem);
@@ -601,6 +611,8 @@ malloc_elem_hide_region(struct malloc_elem *elem, void *start, size_t len)
if (next && next_elem_is_adjacent(elem)) {
len_after = RTE_PTR_DIFF(next, hide_end);
if (len_after >= MALLOC_ELEM_OVERHEAD + MIN_DATA_SIZE) {
+ asan_clear_split_alloczone(hide_end);
+
/* split after */
split_elem(elem, hide_end);
@@ -615,6 +627,8 @@ malloc_elem_hide_region(struct malloc_elem *elem, void *start, size_t len)
if (prev && prev_elem_is_adjacent(elem)) {
len_before = RTE_PTR_DIFF(hide_start, elem);
if (len_before >= MALLOC_ELEM_OVERHEAD + MIN_DATA_SIZE) {
+ asan_clear_split_alloczone(hide_start);
+
/* split before */
split_elem(elem, hide_start);
@@ -628,6 +642,8 @@ malloc_elem_hide_region(struct malloc_elem *elem, void *start, size_t len)
}
}
+ asan_clear_alloczone(elem);
+
remove_elem(elem);
}
@@ -641,8 +657,10 @@ malloc_elem_resize(struct malloc_elem *elem, size_t size)
const size_t new_size = size + elem->pad + MALLOC_ELEM_OVERHEAD;
/* if we request a smaller size, then always return ok */
- if (elem->size >= new_size)
+ if (elem->size >= new_size) {
+ asan_clear_alloczone(elem);
return 0;
+ }
/* check if there is a next element, it's free and adjacent */
if (!elem->next || elem->next->state != ELEM_FREE ||
@@ -661,9 +679,15 @@ malloc_elem_resize(struct malloc_elem *elem, size_t size)
/* now we have a big block together. Lets cut it down a bit, by splitting */
struct malloc_elem *split_pt = RTE_PTR_ADD(elem, new_size);
split_pt = RTE_PTR_ALIGN_CEIL(split_pt, RTE_CACHE_LINE_SIZE);
+
+ asan_clear_split_alloczone(split_pt);
+
split_elem(elem, split_pt);
malloc_elem_free_list_insert(split_pt);
}
+
+ asan_clear_alloczone(elem);
+
return 0;
}
@@ -7,6 +7,14 @@
#include <stdbool.h>
+#ifdef __SANITIZE_ADDRESS__
+#define RTE_MALLOC_ASAN
+#elif defined(__has_feature)
+# if __has_feature(address_sanitizer)
+#define RTE_MALLOC_ASAN
+# endif
+#endif
+
#define MIN_DATA_SIZE (RTE_CACHE_LINE_SIZE)
/* dummy definition of struct so we can use pointers to it in malloc_elem struct */
@@ -36,10 +44,20 @@ struct malloc_elem {
uint64_t header_cookie; /* Cookie marking start of data */
/* trailer cookie at start + size */
#endif
+#ifdef RTE_MALLOC_ASAN
+ size_t user_size;
+ uint64_t asan_cookie[2]; /*must be next to header_cookie*/
+#endif
} __rte_cache_aligned;
+static const unsigned int MALLOC_ELEM_HEADER_LEN = sizeof(struct malloc_elem);
+
#ifndef RTE_MALLOC_DEBUG
-static const unsigned MALLOC_ELEM_TRAILER_LEN = 0;
+#ifdef RTE_MALLOC_ASAN
+static const unsigned int MALLOC_ELEM_TRAILER_LEN = RTE_CACHE_LINE_SIZE;
+#else
+static const unsigned int MALLOC_ELEM_TRAILER_LEN;
+#endif
/* dummy function - just check if pointer is non-null */
static inline int
@@ -55,7 +73,7 @@ set_trailer(struct malloc_elem *elem __rte_unused){ }
#else
-static const unsigned MALLOC_ELEM_TRAILER_LEN = RTE_CACHE_LINE_SIZE;
+static const unsigned int MALLOC_ELEM_TRAILER_LEN = RTE_CACHE_LINE_SIZE;
#define MALLOC_HEADER_COOKIE 0xbadbadbadadd2e55ULL /**< Header cookie. */
#define MALLOC_TRAILER_COOKIE 0xadd2e55badbadbadULL /**< Trailer cookie.*/
@@ -90,9 +108,169 @@ malloc_elem_cookies_ok(const struct malloc_elem *elem)
#endif
-static const unsigned MALLOC_ELEM_HEADER_LEN = sizeof(struct malloc_elem);
#define MALLOC_ELEM_OVERHEAD (MALLOC_ELEM_HEADER_LEN + MALLOC_ELEM_TRAILER_LEN)
+#ifdef RTE_MALLOC_ASAN
+
+#define ASAN_SHADOW_GRAIN_SIZE 8
+#define ASAN_MEM_FREE_FLAG 0xfd
+#define ASAN_MEM_REDZONE_FLAG 0xfa
+#define ASAN_MEM_TO_SHADOW(mem) (((mem) >> 3) + 0x00007fff8000)
+
+#if defined(__clang__)
+__attribute__((no_sanitize("address", "hwaddress")))
+#else
+__attribute__((no_sanitize_address))
+#endif
+static inline void
+asan_set_shadow(void *addr, char val)
+{
+ *(char *)addr = val;
+}
+
+static inline void
+asan_set_zone(void *ptr, size_t len, uint32_t val)
+{
+ size_t offset;
+ char *shadow;
+ size_t zone_len = len / ASAN_SHADOW_GRAIN_SIZE;
+ if (len % ASAN_SHADOW_GRAIN_SIZE != 0)
+ zone_len += 1;
+
+ for (size_t i = 0; i < zone_len; i++) {
+ offset = i * ASAN_SHADOW_GRAIN_SIZE;
+ shadow = (char *)ASAN_MEM_TO_SHADOW(((int64_t)ptr + offset));
+ asan_set_shadow(shadow, val);
+ }
+}
+
+/*
+ * When the memory is released, the release mark is
+ * set in the corresponding range of the shadow area.
+ */
+static inline void
+asan_set_freezone(void *ptr, size_t size)
+{
+ asan_set_zone(ptr, size, ASAN_MEM_FREE_FLAG);
+}
+
+/*
+ * When the memory is allocated, memory state must set as accessible.
+ */
+static inline void
+asan_clear_alloczone(struct malloc_elem *elem)
+{
+ asan_set_zone((void *)elem, elem->size, 0x0);
+}
+
+static inline void
+asan_clear_split_alloczone(struct malloc_elem *elem)
+{
+ void *ptr = RTE_PTR_SUB(elem, MALLOC_ELEM_TRAILER_LEN);
+ asan_set_zone(ptr, MALLOC_ELEM_OVERHEAD, 0x0);
+}
+
+/*
+ * When the memory is allocated, the memory boundary is
+ * marked in the corresponding range of the shadow area.
+ */
+static inline void
+asan_set_redzone(struct malloc_elem *elem, size_t user_size)
+{
+ uint64_t ptr;
+ char *shadow;
+ if (elem != NULL) {
+ if (elem->state != ELEM_PAD)
+ elem = RTE_PTR_ADD(elem, elem->pad);
+
+ elem->user_size = user_size;
+
+ /* Set mark before the start of the allocated memory */
+ ptr = (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN)
+ - ASAN_SHADOW_GRAIN_SIZE;
+ shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
+ asan_set_shadow(shadow, ASAN_MEM_REDZONE_FLAG);
+ shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
+ - ASAN_SHADOW_GRAIN_SIZE);
+ asan_set_shadow(shadow, ASAN_MEM_REDZONE_FLAG);
+
+ /* Set mark after the end of the allocated memory */
+ ptr = (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN
+ + elem->user_size);
+ shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
+ uint32_t val = (ptr % ASAN_SHADOW_GRAIN_SIZE);
+ val = (val == 0) ? ASAN_MEM_REDZONE_FLAG : val;
+ asan_set_shadow(shadow, val);
+ shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
+ + ASAN_SHADOW_GRAIN_SIZE);
+ asan_set_shadow(shadow, ASAN_MEM_REDZONE_FLAG);
+ }
+}
+
+/*
+ * When the memory is released, the mark of the memory boundary
+ * in the corresponding range of the shadow area is cleared.
+ */
+static inline void
+asan_clear_redzone(struct malloc_elem *elem)
+{
+ uint64_t ptr;
+ char *shadow;
+ if (elem != NULL) {
+ elem = RTE_PTR_ADD(elem, elem->pad);
+
+ /* Clear mark before the start of the allocated memory */
+ ptr = (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN)
+ - ASAN_SHADOW_GRAIN_SIZE;
+ shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
+ asan_set_shadow(shadow, 0x00);
+ shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
+ - ASAN_SHADOW_GRAIN_SIZE);
+ asan_set_shadow(shadow, 0x00);
+
+ /* Clear mark after the end of the allocated memory */
+ ptr = (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN
+ + elem->user_size);
+ shadow = (char *)ASAN_MEM_TO_SHADOW(ptr);
+ asan_set_shadow(shadow, 0x00);
+ shadow = (char *)ASAN_MEM_TO_SHADOW(ptr
+ + ASAN_SHADOW_GRAIN_SIZE);
+ asan_set_shadow(shadow, 0x00);
+ }
+}
+
+static inline size_t
+old_malloc_size(struct malloc_elem *elem)
+{
+ if (elem->state != ELEM_PAD)
+ elem = RTE_PTR_ADD(elem, elem->pad);
+
+ return elem->user_size;
+}
+#else
+static inline void
+asan_set_freezone(void *ptr __rte_unused, size_t size __rte_unused) { }
+
+static inline void
+asan_clear_alloczone(struct malloc_elem *elem __rte_unused) { }
+
+static inline void
+asan_clear_split_alloczone(struct malloc_elem *elem __rte_unused) { }
+
+static inline void
+asan_set_redzone(struct malloc_elem *elem __rte_unused,
+ size_t user_size __rte_unused) { }
+
+static inline void
+asan_clear_redzone(struct malloc_elem *elem __rte_unused) { }
+
+static inline size_t
+old_malloc_size(struct malloc_elem *elem)
+{
+ return elem->size - elem->pad - MALLOC_ELEM_OVERHEAD;
+}
+#endif
+
/*
* Given a pointer to the start of a memory block returned by malloc, get
* the actual malloc_elem header for that block.
@@ -237,6 +237,7 @@ heap_alloc(struct malloc_heap *heap, const char *type __rte_unused, size_t size,
unsigned int flags, size_t align, size_t bound, bool contig)
{
struct malloc_elem *elem;
+ size_t user_size = size;
size = RTE_CACHE_LINE_ROUNDUP(size);
align = RTE_CACHE_LINE_ROUNDUP(align);
@@ -250,6 +251,8 @@ heap_alloc(struct malloc_heap *heap, const char *type __rte_unused, size_t size,
/* increase heap's count of allocated elements */
heap->alloc_count++;
+
+ asan_set_redzone(elem, user_size);
}
return elem == NULL ? NULL : (void *)(&elem[1]);
@@ -270,6 +273,8 @@ heap_alloc_biggest(struct malloc_heap *heap, const char *type __rte_unused,
/* increase heap's count of allocated elements */
heap->alloc_count++;
+
+ asan_set_redzone(elem, size);
}
return elem == NULL ? NULL : (void *)(&elem[1]);
@@ -841,6 +846,8 @@ malloc_heap_free(struct malloc_elem *elem)
if (!malloc_elem_cookies_ok(elem) || elem->state != ELEM_BUSY)
return -1;
+ asan_clear_redzone(elem);
+
/* elem may be merged with previous element, so keep heap address */
heap = elem->heap;
msl = elem->msl;
@@ -848,6 +855,9 @@ malloc_heap_free(struct malloc_elem *elem)
rte_spinlock_lock(&(heap->lock));
+ void *asan_ptr = RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN + elem->pad);
+ size_t asan_data_len = elem->size - MALLOC_ELEM_OVERHEAD - elem->pad;
+
/* mark element as free */
elem->state = ELEM_FREE;
@@ -1001,6 +1011,8 @@ malloc_heap_free(struct malloc_elem *elem)
rte_mcfg_mem_write_unlock();
free_unlock:
+ asan_set_freezone(asan_ptr, asan_data_len);
+
rte_spinlock_unlock(&(heap->lock));
return ret;
}
@@ -162,6 +162,8 @@ rte_calloc(const char *type, size_t num, size_t size, unsigned align)
void *
rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
{
+ size_t user_size;
+
if (ptr == NULL)
return rte_malloc_socket(NULL, size, align, socket);
@@ -171,6 +173,8 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
return NULL;
}
+ user_size = size;
+
size = RTE_CACHE_LINE_ROUNDUP(size), align = RTE_CACHE_LINE_ROUNDUP(align);
/* check requested socket id and alignment matches first, and if ok,
@@ -181,6 +185,9 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
RTE_PTR_ALIGN(ptr, align) == ptr &&
malloc_heap_resize(elem, size) == 0) {
rte_eal_trace_mem_realloc(size, align, socket, ptr);
+
+ asan_set_redzone(elem, user_size);
+
return ptr;
}
@@ -192,7 +199,7 @@ rte_realloc_socket(void *ptr, size_t size, unsigned int align, int socket)
if (new_ptr == NULL)
return NULL;
/* elem: |pad|data_elem|data|trailer| */
- const size_t old_size = elem->size - elem->pad - MALLOC_ELEM_OVERHEAD;
+ const size_t old_size = old_malloc_size(elem);
rte_memcpy(new_ptr, ptr, old_size < size ? old_size : size);
rte_free(ptr);
@@ -6340,7 +6340,7 @@ instr_meter_translate(struct rte_swx_pipeline *p,
return 0;
}
- CHECK(0, EINVAL);
+ return -EINVAL;
}
static inline struct meter *
@@ -8025,7 +8025,7 @@ instr_translate(struct rte_swx_pipeline *p,
instr,
data);
- CHECK(0, EINVAL);
+ return -EINVAL;
}
static struct instruction_data *