Re: [dpdk-dev] [PATCH v6 1/2] Enable ASan for memory detector on DPDK

["Peng, ZhihongX" <zhihongx.peng@intel.com>]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Thursday, September 30, 2021 4:20 PM
> To: Peng, ZhihongX <ZhihongX.Peng@intel.com>; Richardson, Bruce
> <bruce.richardson@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>; Thomas Monjalon <thomas@monjalon.net>
> Subject: Re: [dpdk-dev] [PATCH v6 1/2] Enable ASan for memory detector on
> DPDK
> 
> Hello,
> 
> I see v6 is superseded in pw, I have been cleaning my queue... maybe my
> fault.
> 
> 
> On Thu, Sep 30, 2021 at 7:37 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.
> >
> > `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.
> 
> This patch mixes how to use ASan and instrumenting the DPDK mem
> allocator.
> 
> I would split this patch in two.
> 
> The first patch can add the documentation on enabling/using ASan and
> describe the known issues on enabling it.
> I'd find it better (from a user pov) if we hide all those details about b_lundef
> and installation of libasan on Centos.
> 

V7 version will be modified.

> Something like (only quickly tested):
> 
> diff --git a/config/meson.build b/config/meson.build index
> 4cdf589e20..7d8b71da79 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -411,6 +411,33 @@ if get_option('b_lto')
>      endif
>  endif
> 
> +if get_option('b_sanitize') == 'address'
> +    asan_dep = cc.find_library('asan', required: true)
> +    if (not cc.links('int main(int argc, char *argv[]) { return 0; }',
> +                     dependencies: asan_dep))
> +        error('broken dependency, "libasan"')
> +    endif
> +    add_project_link_arguments('-lasan', language: 'c')
> +    dpdk_extra_ldflags += '-lasan'
> +endif
> +
>  if get_option('default_library') == 'both'
>      error( '''
>   Unsupported value "both" for "default_library" option.
> 
> 
> Bruce, do you see an issue with this approach?

It seems clearer to get the ASan switch in the c code.

> 
> Then a second patch adds the rte_malloc instrumentation, with a check at
> configuration time.
> 
>      endif
>      add_project_link_arguments('-lasan', language: 'c')
>      dpdk_extra_ldflags += '-lasan'
> +    if arch_subdir == 'x86'
> +        asan_check_code = '''
> +#ifdef __SANITIZE_ADDRESS__
> +#define RTE_MALLOC_ASAN
> +#elif defined(__has_feature)
> +# if __has_feature(address_sanitizer)
> +#define RTE_MALLOC_ASAN
> +# endif
> +#endif
> +
> +#ifndef RTE_MALLOC_ASAN
> +#error ASan not available.
> +#endif
> +'''
> +        if cc.compiles(asan_check_code)
> +            dpdk_conf.set10('RTE_MALLOC_ASAN', true)
> +        endif
> +    endif
>  endif
> 
>  if get_option('default_library') == 'both'
> 
> 
> Few more comments:
> 
> 
> >
> > DPDK ASan functionality is currently only supported Linux x86_64.
> > Support other platforms, need to define ASAN_SHADOW_OFFSET value
> > according to google ASan document.
> >
> > 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": This is a non-essential option. When this option
> > is added, if a memory error occurs, ASan can clearly show where the
> > code is wrong.
> > "-Db_lundef=false": When use clang to compile DPDK, this option must
> > be added.
> >
> > Signed-off-by: Xueqin Lin <xueqin.lin@intel.com>
> > Signed-off-by: Zhihong Peng <zhihongx.peng@intel.com>
> > ---
> >  devtools/words-case.txt         |   1 +
> >  doc/guides/prog_guide/ASan.rst  | 108 +++++++++++++++++
> >  doc/guides/prog_guide/index.rst |   1 +
> >  examples/helloworld/main.c      |   5 +
> >  lib/eal/common/malloc_elem.c    |  26 +++-
> >  lib/eal/common/malloc_elem.h    | 204
> +++++++++++++++++++++++++++++++-
> >  lib/eal/common/malloc_heap.c    |  12 ++
> >  lib/eal/common/rte_malloc.c     |   9 +-
> >  8 files changed, 361 insertions(+), 5 deletions(-)  create mode
> > 100644 doc/guides/prog_guide/ASan.rst
> >
> > diff --git a/devtools/words-case.txt b/devtools/words-case.txt index
> > 0bbad48626..3655596d47 100644
> > --- a/devtools/words-case.txt
> > +++ b/devtools/words-case.txt
> > @@ -86,3 +86,4 @@ VXLAN
> >  Windows
> >  XDP
> >  XOR
> > +ASan
> 
> Alphabetical order please.
> 

V7 version will be modified.

> > diff --git a/doc/guides/prog_guide/ASan.rst
> > b/doc/guides/prog_guide/ASan.rst
> 
> Filenames are lowercase in the doc.

V7 version will be modified.

> 
> > new file mode 100644
> > index 0000000000..7145a3b1a1
> > --- /dev/null
> > +++ b/doc/guides/prog_guide/ASan.rst
> > @@ -0,0 +1,108 @@
> > +.. 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+).
> 
> missing spaces around ().

V7 version will be modified.

> 
> > +
> > +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
> 
> Please update this example since the unit test has been removed.

V7 version will be modified.

> 
> > +    #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
> > +
> > +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
> 
> Idem.
> 
> 
> > +    #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
> 
> 
> --
> David Marchand