classification
Title: add an AddressSanitizer build option
Type: enhancement Stage: resolved
Components: Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: christian.heimes, haypo, neologix, pitrou, python-dev, skrah
Priority: normal Keywords: patch

Created on 2014-03-23 13:38 by neologix, last changed 2014-05-09 18:49 by neologix. This issue is now closed.

Files
File name Uploaded Description Edit
asan.diff neologix, 2014-03-23 13:38 review
Messages (15)
msg214578 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-03-23 13:38
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.
msg214579 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-03-23 13:41
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.
msg217473 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-04-29 06:01
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...
msg217482 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-04-29 07:58
> 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.
msg217543 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-04-29 20:08
> 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
msg217544 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-04-29 20:22
> 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?
msg217545 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2014-04-29 20:27
How do we spot any ASAN issues, though? Does ASAN change the process' return code on errors?
msg217547 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-04-29 20:54
> 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 :-)
msg217725 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2014-05-01 21:06
> 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.
msg217728 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-05-01 21:31
> 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.
msg217762 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2014-05-02 17:19
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.
msg218126 - (view) Author: Roundup Robot (python-dev) Date: 2014-05-08 22:10
New changeset 17689e43839a by Charles-François Natali in branch 'default':
Issue #21037: Add a build option to enable AddressSanitizer support.
http://hg.python.org/cpython/rev/17689e43839a
msg218131 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-05-08 22:30
I just pushed the patch.

Stefan, did you have time to setup a buildbot?
msg218166 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2014-05-09 14:23
The VM is set up. It's on an external unreliable host though. :)
msg218192 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2014-05-09 18:49
OK, great, let's see what happens!
History
Date User Action Args
2014-05-09 18:49:54neologixsetstatus: open -> closed
resolution: fixed
messages: + msg218192

stage: resolved
2014-05-09 14:23:22skrahsetmessages: + msg218166
2014-05-08 22:30:53neologixsetmessages: + msg218131
2014-05-08 22:10:21python-devsetnosy: + python-dev
messages: + msg218126
2014-05-02 17:19:57skrahsetmessages: + msg217762
2014-05-01 21:31:30neologixsetmessages: + msg217728
2014-05-01 21:06:55skrahsetmessages: + msg217725
2014-04-29 20:54:46neologixsetmessages: + msg217547
2014-04-29 20:27:14pitrousetmessages: + msg217545
2014-04-29 20:22:57pitrousetnosy: + skrah
messages: + msg217544
2014-04-29 20:08:22neologixsetmessages: + msg217543
2014-04-29 07:58:13pitrousetmessages: + msg217482
2014-04-29 06:01:12neologixsetnosy: + pitrou, haypo
messages: + msg217473
2014-03-23 13:41:44neologixsetmessages: + msg214579
2014-03-23 13:38:47neologixcreate