From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 311ACA0548;
	Mon, 20 Sep 2021 12:04:05 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id 992EE40DF7;
	Mon, 20 Sep 2021 12:04:04 +0200 (CEST)
Received: from us-smtp-delivery-124.mimecast.com
 (us-smtp-delivery-124.mimecast.com [170.10.133.124])
 by mails.dpdk.org (Postfix) with ESMTP id 73EF040DF5
 for <dev@dpdk.org>; Mon, 20 Sep 2021 12:04:02 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;
 s=mimecast20190719; t=1632132241;
 h=from:from:reply-to:subject:subject:date:date:message-id:message-id:
 to:to:cc:cc:mime-version:mime-version:content-type:content-type:
 content-transfer-encoding:content-transfer-encoding:
 in-reply-to:in-reply-to:references:references;
 bh=BxuX2mVay+x52XXdySzXWTNnE6+alsibMGOCUhht0HU=;
 b=SS0EiwLHh0aluX1FQdomtgxpxPoDlcjzr9pN/ycud4j7Rag/Hf0QDUZ8fIro9fdEHXfVR7
 u631/UmFC+Qg+Amf5zWCyzfz9d97C66QJIP0lqZzhHWoyRCgN2GslrlJGgvoyE3UOtq/CL
 5C/6La8VeUA/r01PrIslShupxVSVg3s=
Received: from mail-lf1-f70.google.com (mail-lf1-f70.google.com
 [209.85.167.70]) (Using TLS) by relay.mimecast.com with ESMTP id
 us-mta-254-otll3huWMuS_pUOxZt7NlA-1; Mon, 20 Sep 2021 06:03:59 -0400
X-MC-Unique: otll3huWMuS_pUOxZt7NlA-1
Received: by mail-lf1-f70.google.com with SMTP id
 g4-20020a19ac04000000b003eb3973e4e2so11852436lfc.17
 for <dev@dpdk.org>; Mon, 20 Sep 2021 03:03:59 -0700 (PDT)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20210112;
 h=x-gm-message-state:mime-version:references:in-reply-to:from:date
 :message-id:subject:to:cc:content-transfer-encoding;
 bh=BxuX2mVay+x52XXdySzXWTNnE6+alsibMGOCUhht0HU=;
 b=kv1tSRGI5sE2IJ5Cq3A2qjmL2YPQTNJzwsfXA9OUdhNY3oXn+G+dfn9qKDfICtFAEM
 PeG2IdS1fwdHSa1WhZ+87Y32bXYnegcyxzk0Fc33I6S+vgkjUhcpjEkGPqdl53G56BJ/
 vAhumykYJIWy8hTardy+uHNRwd17B6qTa+SqCpdEsXOPEaq34ohpoz+K9s/lioCbS7iV
 SmqGqyrS/4C/ptQFJDV2MyUk/QyVogEVlmm/r2RIluCJZo00puF3o7jRoCnlLiEeWk6/
 I1REPt2gif/OzcTjZJsb8eQdt7GjeNARtA3r3gHLpHjm7BjxE2qy+gAcCV+IV/kRxtDI
 I1bA==
X-Gm-Message-State: AOAM530YDjEFCtz8aRevCvm9VrTL9QjbuKfkSIUnWdYlx9ia3r7qsYq4
 hINKuI0XU/bmPI1VYdvcDzTBbqiRbUmcLCpNsatQQaLpQ4dsACCfp+qy8w7Bd9JnPyinDTeigp+
 rmy+X1U+/l57dL5LvAU0=
X-Received: by 2002:a2e:91d4:: with SMTP id u20mr15530711ljg.81.1632132237366; 
 Mon, 20 Sep 2021 03:03:57 -0700 (PDT)
X-Google-Smtp-Source: ABdhPJwNTfaJkUF0+estJNEiKgkWHfHWt5luW9iQnClYKOhw46rMYq66kcHAEZlz+eo6od4GIDSnJQUInTDiUuwSVu4=
X-Received: by 2002:a2e:91d4:: with SMTP id u20mr15530683ljg.81.1632132236818; 
 Mon, 20 Sep 2021 03:03:56 -0700 (PDT)
MIME-Version: 1.0
References: <20210916013843.342366-1-zhihongx.peng@intel.com>
 <20210918074155.872358-1-zhihongx.peng@intel.com>
In-Reply-To: <20210918074155.872358-1-zhihongx.peng@intel.com>
From: David Marchand <david.marchand@redhat.com>
Date: Mon, 20 Sep 2021 12:03:45 +0200
Message-ID: <CAJFAV8wtt=3w8E+cABH83ZdY2LqRscyS8UG52ZNPvfvHc8MK5A@mail.gmail.com>
To: Zhihong Peng <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>, 
 Xueqin Lin <xueqin.lin@intel.com>
Authentication-Results: relay.mimecast.com;
 auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dmarchan@redhat.com
X-Mimecast-Spam-Score: 0
X-Mimecast-Originator: redhat.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Subject: Re: [dpdk-dev] [PATCH v3] Enable AddressSanitizer feature on DPDK
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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 =3D rte_zmalloc(NULL, 7, 0);
>         p[7] =3D 'a';
>         ......
>
> Here is an example of use-after-free bug:
>         ......
>         char *p =3D rte_zmalloc(NULL, 7, 0);
>         rte_free(p);
>         *p =3D 'a';
>         ......
>
> If you want to use this feature,
> you need to add below compilation options when compiling code:
> -Dbuildtype=3Ddebug -Db_lundef=3Dfalse -Db_sanitize=3Daddress

ASAN is triggered by -Db_sanitize=3Daddress, it is the only *needed* option=
 afaiu.


> "-Dbuildtype=3Ddebug": 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=3Dfalse": 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)
> +=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D
> +
> +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 =3D rte_zmalloc(NULL, 9, 0);
> +    if (!p) {
> +        printf("rte_zmalloc error.");
> +        return -1;
> +    }
> +    p[9] =3D 'a';
> +
> +The error log::
> +
> +    =3D=3D49433=3D=3DERROR: AddressSanitizer: heap-buffer-overflow on ad=
dress 0x7f773fafa249 at pc 0x5556b13bdae4 bp 0x7ffeb4965e40 sp 0x7ffeb4965e=
30 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:29=
0
> +    #3 0x5556b1cd8987 in cmdline_valid_buffer ../lib/cmdline/cmdline.c:2=
6
> +    #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.s=
o.6+0x270b2)
> +    #9 0x5556b043e70d in _start (/home/pzh/yyy/x86_64-native-linuxapp-gc=
c/app/test/dpdk-test+0x7ce70d)
> +
> +    Address 0x7f773fafa249 is a wild pointer.
> +    SUMMARY: AddressSanitizer: heap-buffer-overflow ../app/test/test_asa=
n_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
> +    =3D>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 =3D rte_zmalloc(NULL, 9, 0);
> +    if (!p) {
> +        printf("rte_zmalloc error.");
> +        return -1;
> +    }
> +    rte_free(p);
> +    *p =3D 'a';
> +
> +The error log::
> +
> +    =3D=3D49478=3D=3DERROR: AddressSanitizer: heap-use-after-free on add=
ress 0x7fe2ffafa240 at pc 0x56409b084bc8 bp 0x7ffef62c57d0 sp 0x7ffef62c57c=
0 WRITE of size 1 at 0x7fe2ffafa240 thread T0
> +    #0 0x56409b084bc7 in asan_use_after_free ../app/test/test_asan_use_a=
fter_free.c:26
> +    #1 0x56409a1059d4 in cmd_autotest_parsed ../app/test/commands.c:71
> +    #2 0x56409b9a44b0 in cmdline_parse ../lib/cmdline/cmdline_parse.c:29=
0
> +    #3 0x56409b99f987 in cmdline_valid_buffer ../lib/cmdline/cmdline.c:2=
6
> +    #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.s=
o.6+0x270b2)
> +    #9 0x56409a10570d in _start (/home/pzh/yyy/x86_64-native-linuxapp-gc=
c/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
> +    =3D>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=3Ddebug -Db_lundef=3Dfalse -Db_sanitize=3Daddre=
ss
> + 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 exec=
ute 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 fut=
ure.

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/inde=
x.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 si=
ze, unsigned align,
>                 struct malloc_elem *new_free_elem =3D
>                                 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 si=
ze, unsigned align,
>                 elem->state =3D ELEM_BUSY;
>                 elem->pad =3D old_elem_size;
>
> +               asan_clear_alloczone(elem);
> +
>                 /* put a dummy header in padding, to point to real elemen=
t 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 =3D ELEM_BUSY;
>         malloc_elem_free_list_insert(elem);
>
> @@ -601,6 +611,8 @@ malloc_elem_hide_region(struct malloc_elem *elem, voi=
d *start, size_t len)
>         if (next && next_elem_is_adjacent(elem)) {
>                 len_after =3D RTE_PTR_DIFF(next, hide_end);
>                 if (len_after >=3D 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, voi=
d *start, size_t len)
>         if (prev && prev_elem_is_adjacent(elem)) {
>                 len_before =3D RTE_PTR_DIFF(hide_start, elem);
>                 if (len_before >=3D 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, voi=
d *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 =3D size + elem->pad + MALLOC_ELEM_OVERHEAD=
;
>
>         /* if we request a smaller size, then always return ok */
> -       if (elem->size >=3D new_size)
> +       if (elem->size >=3D 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 !=3D 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 b=
it, by splitting */
>                 struct malloc_elem *split_pt =3D RTE_PTR_ADD(elem, new_si=
ze);
>                 split_pt =3D 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_ele=
m 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 =3D sizeof(struct mallo=
c_elem);
> +
>  #ifndef RTE_MALLOC_DEBUG
> -static const unsigned MALLOC_ELEM_TRAILER_LEN =3D 0;
> +#ifdef RTE_MALLOC_ASAN
> +static const unsigned int MALLOC_ELEM_TRAILER_LEN =3D RTE_CACHE_LINE_SIZ=
E;
> +#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 =3D RTE_CACHE_LINE_SIZE;
> +static const unsigned int MALLOC_ELEM_TRAILER_LEN =3D RTE_CACHE_LINE_SIZ=
E;
>
>  #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 *ele=
m)
>
>  #endif
>
> -static const unsigned MALLOC_ELEM_HEADER_LEN =3D sizeof(struct malloc_el=
em);
>  #define MALLOC_ELEM_OVERHEAD (MALLOC_ELEM_HEADER_LEN + MALLOC_ELEM_TRAIL=
ER_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 =3D val;
> +}
> +
> +static inline void
> +asan_set_zone(void *ptr, size_t len, uint32_t val)
> +{
> +       size_t offset;
> +       char *shadow;
> +       size_t zone_len =3D len / ASAN_SHADOW_GRAIN_SIZE;

+ size_t i;
+ <empty line> to separate declarations from code.


> +       if (len % ASAN_SHADOW_GRAIN_SIZE !=3D 0)
> +               zone_len +=3D 1;
> +
> +       for (size_t i =3D 0; i < zone_len; i++) {

+       for (i =3D 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 =E2=80=98asan_set_zone=E2=80=
=99:
../lib/eal/common/malloc_elem.h:140:2: error: =E2=80=98for=E2=80=99 loop in=
itial
declarations are only allowed in C99 mode
  for (size_t i =3D 0; i < zone_len; i++) {
  ^
../lib/eal/common/malloc_elem.h:140:2: note: use option -std=3Dc99 or
-std=3Dgnu99 to compile your code







> +               offset =3D i * ASAN_SHADOW_GRAIN_SIZE;
> +               shadow =3D (char *)ASAN_MEM_TO_SHADOW(((int64_t)ptr + off=
set));
> +               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 =3D 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 !=3D NULL) {
> +               if (elem->state !=3D ELEM_PAD)
> +                       elem =3D RTE_PTR_ADD(elem, elem->pad);
> +
> +               elem->user_size =3D user_size;
> +
> +               /* Set mark before the start of the allocated memory */
> +               ptr =3D (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LE=
N)
> +                       - ASAN_SHADOW_GRAIN_SIZE;
> +               shadow =3D (char *)ASAN_MEM_TO_SHADOW(ptr);
> +               asan_set_shadow(shadow, ASAN_MEM_REDZONE_FLAG);
> +               shadow =3D (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 =3D (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LE=
N
> +                               + elem->user_size);
> +               shadow =3D (char *)ASAN_MEM_TO_SHADOW(ptr);
> +               uint32_t val =3D (ptr % ASAN_SHADOW_GRAIN_SIZE);
> +               val =3D (val =3D=3D 0) ? ASAN_MEM_REDZONE_FLAG : val;
> +               asan_set_shadow(shadow, val);
> +               shadow =3D (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 !=3D NULL) {
> +               elem =3D RTE_PTR_ADD(elem, elem->pad);
> +
> +               /* Clear mark before the start of the allocated memory */
> +               ptr =3D (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LE=
N)
> +                       - ASAN_SHADOW_GRAIN_SIZE;
> +               shadow =3D (char *)ASAN_MEM_TO_SHADOW(ptr);
> +               asan_set_shadow(shadow, 0x00);
> +               shadow =3D (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 =3D (uint64_t)RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LE=
N
> +                               + elem->user_size);
> +               shadow =3D (char *)ASAN_MEM_TO_SHADOW(ptr);
> +               asan_set_shadow(shadow, 0x00);
> +               shadow =3D (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 !=3D ELEM_PAD)
> +               elem =3D 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, ge=
t
>   * 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 cont=
ig)
>  {
>         struct malloc_elem *elem;
> +       size_t user_size =3D size;
>
>         size =3D RTE_CACHE_LINE_ROUNDUP(size);
>         align =3D 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 =3D=3D NULL ? NULL : (void *)(&elem[1]);
> @@ -270,6 +273,8 @@ heap_alloc_biggest(struct malloc_heap *heap, const ch=
ar *type __rte_unused,
>
>                 /* increase heap's count of allocated elements */
>                 heap->alloc_count++;
> +
> +               asan_set_redzone(elem, size);
>         }
>
>         return elem =3D=3D NULL ? NULL : (void *)(&elem[1]);
> @@ -841,6 +846,8 @@ malloc_heap_free(struct malloc_elem *elem)
>         if (!malloc_elem_cookies_ok(elem) || elem->state !=3D ELEM_BUSY)
>                 return -1;
>
> +       asan_clear_redzone(elem);
> +
>         /* elem may be merged with previous element, so keep heap address=
 */
>         heap =3D elem->heap;
>         msl =3D elem->msl;
> @@ -848,6 +855,9 @@ malloc_heap_free(struct malloc_elem *elem)
>
>         rte_spinlock_lock(&(heap->lock));
>
> +       void *asan_ptr =3D RTE_PTR_ADD(elem, MALLOC_ELEM_HEADER_LEN + ele=
m->pad);
> +       size_t asan_data_len =3D elem->size - MALLOC_ELEM_OVERHEAD - elem=
->pad;
> +
>         /* mark element as free */
>         elem->state =3D 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 socke=
t)
>  {
> +       size_t user_size;
> +
>         if (ptr =3D=3D NULL)
>                 return rte_malloc_socket(NULL, size, align, socket);
>
> @@ -171,6 +173,8 @@ rte_realloc_socket(void *ptr, size_t size, unsigned i=
nt align, int socket)
>                 return NULL;
>         }
>
> +       user_size =3D size;
> +
>         size =3D RTE_CACHE_LINE_ROUNDUP(size), align =3D RTE_CACHE_LINE_R=
OUNDUP(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 i=
nt align, int socket)
>                         RTE_PTR_ALIGN(ptr, align) =3D=3D ptr &&
>                         malloc_heap_resize(elem, size) =3D=3D 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 i=
nt align, int socket)
>         if (new_ptr =3D=3D NULL)
>                 return NULL;
>         /* elem: |pad|data_elem|data|trailer| */
> -       const size_t old_size =3D elem->size - elem->pad - MALLOC_ELEM_OV=
ERHEAD;
> +       const size_t old_size =3D 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_pipel=
ine.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
>


--=20
David Marchand