Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add an AddressSanitizer build option #65236

Closed
neologix mannequin opened this issue Mar 23, 2014 · 15 comments
Closed

add an AddressSanitizer build option #65236

neologix mannequin opened this issue Mar 23, 2014 · 15 comments
Labels
type-feature A feature request or enhancement

Comments

@neologix
Copy link
Mannequin

neologix mannequin commented Mar 23, 2014

BPO 21037
Nosy @pitrou, @vstinner, @tiran, @skrah
Files
  • asan.diff
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2014-05-09.18:49:54.188>
    created_at = <Date 2014-03-23.13:38:47.554>
    labels = ['type-feature']
    title = 'add an AddressSanitizer build option'
    updated_at = <Date 2014-05-09.18:49:54.187>
    user = 'https://bugs.python.org/neologix'

    bugs.python.org fields:

    activity = <Date 2014-05-09.18:49:54.187>
    actor = 'neologix'
    assignee = 'none'
    closed = True
    closed_date = <Date 2014-05-09.18:49:54.188>
    closer = 'neologix'
    components = []
    creation = <Date 2014-03-23.13:38:47.554>
    creator = 'neologix'
    dependencies = []
    files = ['34584']
    hgrepos = []
    issue_num = 21037
    keywords = ['patch']
    message_count = 15.0
    messages = ['214578', '214579', '217473', '217482', '217543', '217544', '217545', '217547', '217725', '217728', '217762', '218126', '218131', '218166', '218192']
    nosy_count = 6.0
    nosy_names = ['pitrou', 'vstinner', 'christian.heimes', 'skrah', 'neologix', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue21037'
    versions = ['Python 3.5']

    @neologix
    Copy link
    Mannequin Author

    neologix mannequin commented Mar 23, 2014

    Adding a compile option to build with ASAN (https://code.google.com/p/address-sanitizer) could allow us to catch many memory-related errors (stack/buffer overflows, etc).

    Of course, the second step would be to setup buildbots to use this flag.

    @neologix neologix mannequin added the type-feature A feature request or enhancement label Mar 23, 2014
    @neologix
    Copy link
    Mannequin Author

    neologix mannequin commented Mar 23, 2014

    Note that ASAN will interfere with the faulthandler's module (since it sets up its own signal handlers), so if we were to incorporate it into the test suite, that's something we should look after.

    @neologix
    Copy link
    Mannequin Author

    neologix mannequin commented Apr 29, 2014

    I'd like to move this forward: it could IMO be a great way to proactively detect potential security defects, and nasty stack/heap/memory corruption in general.

    The remaining - missing - part is buildbot integration: AFAICT the only specific thing to do is to start the process with the ASAN_OPTIONS environment variable set to "handle_segv=0", to avoid interference with faulthandler.

    But I'm not really familiar with the buildbot support, so if anyone has a clue...

    @pitrou
    Copy link
    Member

    pitrou commented Apr 29, 2014

    But I'm not really familiar with the buildbot support, so if anyone
    has a clue...

    I can add environment variables and configure options specific to a buildbot. Just tell me which ones (and which buildbot (preferably yours ? :-)).

    That said, it would be better if you first check said options work locally.

    @neologix
    Copy link
    Mannequin Author

    neologix mannequin commented Apr 29, 2014

    That said, it would be better if you first check said options work locally.

    I wasn't clear, but I did test it, and it works: the only problem I
    encountered is address space exhaustion: I have a 32-bit box, and ASAN
    uses a lot of virtual address space (for shadow pages), so with a lot
    of memory or many thread you can hit the 3G limit.
    So we should only run this on 64-bit machine (see below for more details).

    I can add environment variables and configure options specific to a buildbot. Just tell me which ones (and which buildbot (preferably yours ? :-)).

    Yeah, I barely have a day-to-day machine, so I'm afraid I can't help here :-)

    I guess we could go for any non-stable buildbot meeting the following criteria:

    • Linux 64-bit
    • clang >= 3.1 or gcc >= 4.8

    But it would be great if someone could test the patch locally on a
    64-bit machine before I commit it.
    Basically:

    $ patch -p1 < ~/asan.diff && autoconf && autoheader && ./configure
    --with-address-sanitizer && make
    $ ASAN_OPTIONS=handle_segv=0 ./python -m test -vG -uall

    @pitrou
    Copy link
    Member

    pitrou commented Apr 29, 2014

    I guess we could go for any non-stable buildbot meeting the following > criteria:

    • Linux 64-bit
    • clang >= 3.1 or gcc >= 4.8

    Hmm... perhaps Stefan would like to set something up?

    @pitrou
    Copy link
    Member

    pitrou commented Apr 29, 2014

    How do we spot any ASAN issues, though? Does ASAN change the process' return code on errors?

    @neologix
    Copy link
    Mannequin Author

    neologix mannequin commented Apr 29, 2014

    How do we spot any ASAN issues, though? Does ASAN change the process' return code on errors?

    It aborts:
    $ cat /tmp/test.c
    int main(int argc, char *argv[])
    {
        int bar[16] = {0};
    /* oops \*/
    return bar[16];
    

    }
    $ gcc -Wall -fsanitize=address -o /tmp/test /tmp/test.c
    $ /tmp/test
    =================================================================
    ==15028== ERROR: AddressSanitizer: stack-buffer-overflow on address
    0xbffab500 at pc 0x80485ec bp 0xbffab488 sp 0xbffab47c
    READ of size 4 at 0xbffab500 thread T0
    #0 0x80485eb (/tmp/test+0x80485eb)
    #1 0xb5fd8a62 (/lib/i386-linux-gnu/i686/cmov/libc-2.18.so+0x19a62)
    #2 0x8048490 (/tmp/test+0x8048490)
    Address 0xbffab500 is located at offset 96 in frame <main> of T0's stack:
    This frame has 1 object(s):
    [32, 96) 'bar'
    HINT: this may be a false positive if your program uses some custom
    stack unwind mechanism or swapcontext
    (longjmp and C++ exceptions *are* supported)
    Shadow bytes around the buggy address:
    0x37ff5650: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x37ff5660: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x37ff5670: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x37ff5680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x37ff5690: 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00 00 00
    =>0x37ff56a0:[f3]f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
    0x37ff56b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x37ff56c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x37ff56d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x37ff56e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    0x37ff56f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
    Shadow byte legend (one shadow byte represents 8 application bytes):
    Addressable: 00
    Partially addressable: 01 02 03 04 05 06 07
    Heap left redzone: fa
    Heap righ redzone: fb
    Freed Heap region: fd
    Stack left redzone: f1
    Stack mid redzone: f2
    Stack right redzone: f3
    Stack partial redzone: f4
    Stack after return: f5
    Stack use after scope: f8
    Global redzone: f9
    Global init order: f6
    Poisoned by user: f7
    ASan internal: fe
    ==15028== ABORTING

    You obviously don't see here, but it also colors the output in a terminal :-)

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented May 1, 2014

    Hmm... perhaps Stefan would like to set something up?

    Being a correctness tool hipster, of course I already have the latest toy. :) The patch works on Debian 64-bit + clang.

    I can set up a VM. We may have to react quickly to some of the issues.
    Then again, anyone can run the tool, so there's no real secrecy anyway.

    @neologix
    Copy link
    Mannequin Author

    neologix mannequin commented May 1, 2014

    Being a correctness tool hipster, of course I already have the latest toy. :) The patch works on Debian 64-bit + clang.

    Thanks for testing it.
    I'll leave a few days more in case anyone has a comment, and I'll commit.

    I can set up a VM.

    That would be great.

    We may have to react quickly to some of the issues.
    Then again, anyone can run the tool, so there's no real secrecy anyway.

    Exactly.

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented May 2, 2014

    Antoine, if you send me the buildbot credentials, we can get started.

    Environment vars:

       CC=clang
       ASAN_OPTIONS="allocator_may_return_null=1,handle_segv=0"

    I suggest to compile the release build, just --with-address-sanitizer.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented May 8, 2014

    New changeset 17689e43839a by Charles-François Natali in branch 'default':
    Issue bpo-21037: Add a build option to enable AddressSanitizer support.
    http://hg.python.org/cpython/rev/17689e43839a

    @neologix
    Copy link
    Mannequin Author

    neologix mannequin commented May 8, 2014

    I just pushed the patch.

    Stefan, did you have time to setup a buildbot?

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented May 9, 2014

    The VM is set up. It's on an external unreliable host though. :)

    @neologix
    Copy link
    Mannequin Author

    neologix mannequin commented May 9, 2014

    OK, great, let's see what happens!

    @neologix neologix mannequin closed this as completed May 9, 2014
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant