From: "Lin, Xueqin" <xueqin.lin@intel.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"Peng, ZhihongX" <zhihongx.peng@intel.com>,
"Burakov, Anatoly" <anatoly.burakov@intel.com>,
"stephen@networkplumber.org" <stephen@networkplumber.org>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"Peng, ZhihongX" <zhihongx.peng@intel.com>
Subject: Re: [dpdk-dev] [RFC] porting AddressSanitizer feature to DPDK
Date: Fri, 11 Jun 2021 04:49:12 +0000 [thread overview]
Message-ID: <BN7PR11MB2658365F6B3393D81CF3AB0394349@BN7PR11MB2658.namprd11.prod.outlook.com> (raw)
In-Reply-To: <DM6PR11MB449187C6ADE40D875E9D9F3A9A359@DM6PR11MB4491.namprd11.prod.outlook.com>
> -----Original Message-----
> From: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Sent: Thursday, June 10, 2021 5:12 PM
> To: Peng, ZhihongX <zhihongx.peng@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>; stephen@networkplumber.org
> Cc: dev@dpdk.org; Lin, Xueqin <xueqin.lin@intel.com>; Peng, ZhihongX
> <zhihongx.peng@intel.com>
> Subject: RE: [dpdk-dev] [RFC] porting AddressSanitizer feature to DPDK
>
>
> >
> > 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),
> > ported heap-buffer-overflow and use-after-freefunctions to dpdk.
> >
> > 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 use the following compilation options:
> > -Dc_args='-DRTE_MALLOC_ASAN'
> > -Db_lundef=false -Db_sanitize=address
> >
> > Signed-off-by: Xueqin Lin <xueqin.lin@intel.com>
> > Signed-off-by: Zhihong Peng <zhihongx.peng@intel.com>
> > ---
> > lib/eal/common/malloc_elem.c | 33 +++++++-
> > lib/eal/common/malloc_elem.h | 141
> ++++++++++++++++++++++++++++++++++-
> > lib/eal/common/malloc_heap.c | 19 +++++
> > lib/eal/common/rte_malloc.c | 6 ++
> > 4 files changed, 197 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/eal/common/malloc_elem.c
> > b/lib/eal/common/malloc_elem.c index c2c9461f1..4a146b1b9 100644
> > --- a/lib/eal/common/malloc_elem.c
> > +++ b/lib/eal/common/malloc_elem.c
> > @@ -446,6 +446,9 @@ 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);
> >
> > +#ifdef RTE_MALLOC_ASAN
> > + asan_clear_split_alloczone(new_free_elem);
> > +#endif
>
> Here and everywhere:
> Instead of polluting code with all these ifdefs, I think it would be better to
> move all these asan_*() functions into a separate *.h.
> And have all these ifdefs inside it.
> Something like that:
> asan.h:
>
> #ifdef RTE_MALLOC_ASAN
> static inline void asan_clear_split_alloczone(...) {
> /* actual code */
> }
> ....
> #else
> /* dummy one */
> static inline void asan_clear_split_alloczone(...) { } ...
> #endif
>
Good idea, we can improve it in v2, thanks.
> > split_elem(elem, new_free_elem);
> > malloc_elem_free_list_insert(new_free_elem);
> >
> > @@ -458,6 +461,9 @@ malloc_elem_alloc(struct malloc_elem *elem,
> size_t size, unsigned align,
> > elem->state = ELEM_BUSY;
> > elem->pad = old_elem_size;
> >
> > +#ifdef RTE_MALLOC_ASAN
> > + asan_clear_alloczone(elem);
> > +#endif
> > /* 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 */ @@ -475,7 +481,13
> > @@ malloc_elem_alloc(struct malloc_elem *elem, size_t size, unsigned
> align,
> > * Re-insert original element, in case its new size makes it
> > * belong on a different list.
> > */
> > +#ifdef RTE_MALLOC_ASAN
> > + asan_clear_split_alloczone(new_elem);
> > +#endif
> > split_elem(elem, new_elem);
> > +#ifdef RTE_MALLOC_ASAN
> > + asan_clear_alloczone(new_elem);
> > +#endif
> > new_elem->state = ELEM_BUSY;
> > malloc_elem_free_list_insert(elem);
> >
> > @@ -601,6 +613,9 @@ 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)
> {
> > +#ifdef RTE_MALLOC_ASAN
> > + asan_clear_split_alloczone(hide_end);
> > +#endif
> > /* split after */
> > split_elem(elem, hide_end);
> >
> > @@ -615,6 +630,9 @@ 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) {
> > +#ifdef RTE_MALLOC_ASAN
> > + asan_clear_split_alloczone(hide_start);
> > +#endif
> > /* split before */
> > split_elem(elem, hide_start);
> >
> > @@ -628,6 +646,9 @@ malloc_elem_hide_region(struct malloc_elem
> *elem, void *start, size_t len)
> > }
> > }
> >
> > +#ifdef RTE_MALLOC_ASAN
> > + asan_clear_alloczone(elem);
> > +#endif
> > remove_elem(elem);
> > }
> >
> > @@ -641,8 +662,12 @@ 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) {
> > +#ifdef RTE_MALLOC_ASAN
> > + asan_clear_alloczone(elem);
> > +#endif
> > return 0;
> > + }
> >
> > /* check if there is a next element, it's free and adjacent */
> > if (!elem->next || elem->next->state != ELEM_FREE || @@ -661,9
> > +686,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);
> > +#ifdef RTE_MALLOC_ASAN
> > + asan_clear_split_alloczone(split_pt);
> > +#endif
> > split_elem(elem, split_pt);
> > malloc_elem_free_list_insert(split_pt);
> > }
> > +#ifdef RTE_MALLOC_ASAN
> > + asan_clear_alloczone(elem);
> > +#endif
> > return 0;
> > }
> >
> > diff --git a/lib/eal/common/malloc_elem.h
> > b/lib/eal/common/malloc_elem.h index a1e5f7f02..d0d8bbb48 100644
> > --- a/lib/eal/common/malloc_elem.h
> > +++ b/lib/eal/common/malloc_elem.h
> > @@ -36,10 +36,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 MALLOC_ELEM_HEADER_LEN = sizeof(struct
> malloc_elem);
> > +
> > #ifndef RTE_MALLOC_DEBUG
> > +#ifdef RTE_MALLOC_ASAN
> > +static const unsigned MALLOC_ELEM_TRAILER_LEN =
> RTE_CACHE_LINE_SIZE;
> > +#else
> > static const unsigned MALLOC_ELEM_TRAILER_LEN = 0;
> > +#endif
> >
> > /* dummy function - just check if pointer is non-null */
> > static inline int
> > @@ -90,9 +100,138 @@ 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 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);
> > + }
> > +}
> > +#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 ee400f38e..6d39549d3 100644
> > --- a/lib/eal/common/malloc_heap.c
> > +++ b/lib/eal/common/malloc_heap.c
> > @@ -238,6 +238,9 @@ heap_alloc(struct malloc_heap *heap, const char
> *type __rte_unused, size_t size,
> > {
> > struct malloc_elem *elem;
> >
> > +#ifdef RTE_MALLOC_ASAN
> > + size_t user_size = size;
> > +#endif
> > size = RTE_CACHE_LINE_ROUNDUP(size);
> > align = RTE_CACHE_LINE_ROUNDUP(align);
> >
> > @@ -250,6 +253,9 @@ heap_alloc(struct malloc_heap *heap, const char
> *type __rte_unused, size_t size,
> >
> > /* increase heap's count of allocated elements */
> > heap->alloc_count++;
> > +#ifdef RTE_MALLOC_ASAN
> > + asan_set_redzone(elem, user_size);
> > +#endif
> > }
> >
> > return elem == NULL ? NULL : (void *)(&elem[1]);
> > @@ -270,6 +276,9 @@ heap_alloc_biggest(struct malloc_heap *heap,
> const char *type __rte_unused,
> >
> > /* increase heap's count of allocated elements */
> > heap->alloc_count++;
> > +#ifdef RTE_MALLOC_ASAN
> > + asan_set_redzone(elem, size);
> > +#endif
> > }
> >
> > return elem == NULL ? NULL : (void *)(&elem[1]);
> > @@ -841,6 +850,9 @@ malloc_heap_free(struct malloc_elem *elem)
> > if (!malloc_elem_cookies_ok(elem) || elem->state != ELEM_BUSY)
> > return -1;
> >
> > +#ifdef RTE_MALLOC_ASAN
> > + asan_clear_redzone(elem);
> > +#endif
> > /* elem may be merged with previous element, so keep heap
> address */
> > heap = elem->heap;
> > msl = elem->msl;
> > @@ -848,6 +860,10 @@ malloc_heap_free(struct malloc_elem *elem)
> >
> > rte_spinlock_lock(&(heap->lock));
> >
> > +#ifdef RTE_MALLOC_ASAN
> > + 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;
> > +#endif
> > /* mark element as free */
> > elem->state = ELEM_FREE;
> >
> > @@ -1001,6 +1017,9 @@ malloc_heap_free(struct malloc_elem *elem)
> >
> > rte_mcfg_mem_write_unlock();
> > free_unlock:
> > +#ifdef RTE_MALLOC_ASAN
> > + asan_set_freezone(asan_ptr, asan_data_len);
> > +#endif
> > rte_spinlock_unlock(&(heap->lock));
> > return ret;
> > }
> > diff --git a/lib/eal/common/rte_malloc.c b/lib/eal/common/rte_malloc.c
> > index 9d39e58c0..fe70ee938 100644
> > --- a/lib/eal/common/rte_malloc.c
> > +++ b/lib/eal/common/rte_malloc.c
> > @@ -170,6 +170,9 @@ rte_realloc_socket(void *ptr, size_t size, unsigned
> int align, int socket)
> > RTE_LOG(ERR, EAL, "Error: memory corruption detected\n");
> > return NULL;
> > }
> > +#ifdef RTE_MALLOC_ASAN
> > + size_t user_size = size;
> > +#endif
> >
> > size = RTE_CACHE_LINE_ROUNDUP(size), align =
> RTE_CACHE_LINE_ROUNDUP(align);
> >
> > @@ -181,6 +184,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);
> > +#ifdef RTE_MALLOC_ASAN
> > + asan_set_redzone(elem, user_size);
> > +#endif
> > return ptr;
> > }
> >
> > --
> > 2.17.1
next prev parent reply other threads:[~2021-06-11 4:49 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-10 5:13 zhihongx.peng
2021-06-10 8:32 ` Bruce Richardson
2021-06-11 4:42 ` Lin, Xueqin
2021-06-10 9:12 ` Ananyev, Konstantin
2021-06-11 4:49 ` Lin, Xueqin [this message]
2021-06-10 20:03 ` Stephen Hemminger
2021-06-11 6:15 ` Lin, Xueqin
2021-06-15 8:12 ` [dpdk-dev] [RFC v2] " zhihongx.peng
2021-06-15 8:40 ` Jerin Jacob
2021-06-16 9:13 ` Lin, Xueqin
2021-06-16 11:34 ` Jerin Jacob
2021-06-18 7:48 ` Lin, Xueqin
2021-06-18 9:04 ` David Marchand
2021-06-22 3:26 ` Lin, Xueqin
2021-06-28 14:22 ` Burakov, Anatoly
2021-06-28 14:23 ` Jerin Jacob
2021-06-30 8:15 ` Lin, Xueqin
2021-06-30 8:34 ` David Marchand
2021-07-01 6:48 ` Lin, Xueqin
2021-07-01 7:40 ` David Marchand
2021-07-02 11:05 ` Lin, Xueqin
2021-07-06 20:40 ` David Christensen
2021-07-06 23:12 ` Stephen Hemminger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BN7PR11MB2658365F6B3393D81CF3AB0394349@BN7PR11MB2658.namprd11.prod.outlook.com \
--to=xueqin.lin@intel.com \
--cc=anatoly.burakov@intel.com \
--cc=dev@dpdk.org \
--cc=konstantin.ananyev@intel.com \
--cc=stephen@networkplumber.org \
--cc=zhihongx.peng@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).