Enable address sanitizer in Go

[Fannie Zhang]

The address sanitizer (Asan) is a memory error detector, it finds a number of memory access errors that can occur in C, such as out-of-bounds accesses to heap, stack and global objects,  and use-after-free. In general, these issues do not occur in the pure Go code, unless people intentionally use the unsafe package. However, people can use cgo to pass memory back and forth between C and Go, which will cause memory access problems.

 

Please refer to the following cases.

In this case https://play.golang.org/p/eE_8k4poelj, C passes Go an invalid pointer, causing the Go code to have use-after-free errors.

In this casehttps://play.golang.org/p/wtHjV-4xuiL, Go passes C a pointer,  the C code has out-of-bounds access  to heap objects errors.

In this case https://play.golang.org/p/rOyEo7I7vTY, Go passes C a pointer, the C code has out-of-bounds access to global objects errors.

 

Integrating Asan with Go would detect the above memory errors. So it is useful.  And we can instrument ASan in exactly the same way as MSan. As with the -msan option, if we do not run with -asan option, it has no effect on code execution, if you use it, it may find some memory errors. In addition, Asan can help to detect some memory errors that MSan cannot.  For example, use-after-free memory error case https://play.golang.org/p/eE_8k4poelj,  MSan cannot detect writes on uninitialized memory, while ASan can.

 

Although the above cases are not real world cases. However, on the other hand, when debugging a memory error access in a Go program that calls C code, it is really good to be able to use Asan to ensure that code is correct. Obviously, this will require more code to be maintained in the toolchain, but since almost all of the code is next to the existing MSan code, this is quite limited. Besides, Arm has a hardware feature MTE (Memory Tagging Extension), it aims to further mitigate these memory safety bugs by enabling us to detect them with low overhead. If ASan will be proven to be useful, we may consider enabling MTE for ASan. In this way, ASan even be enabled in the production environment to help to detect memory errors with low overhead. I saw a blog that introduces enable MTE in Android to detect the most common classes of memory safety bugs. Please refer to the document https://security.googleblog.com/2019/08/adopting-arm-memory-tagging-extension.html.

 

I have updated the CLs for address sanitizer support, The implementation is divided into some smaller CLs  298610, 298611, 298612, 298613, 298614 for review. And with this implementation, the -asan option can check for error memory access to heap objects.

 

Any opinions? Thank you.

 

Fannie Zhang

 

 

 

[Ian Lance Taylor]

Thanks for the explanation. This seems reasonable to me. Can you
open a proposal issue for this? See https://golang.org/s/proposal. I
think the main change is that "go build" will get a new -asan option.
Thanks.

Ian

[Fannie...@arm.com]

Thank you  Ian. I will do it right now. 

I
think the main change is that "go build" will get a new -asan option.

Yes, we need to add this change, but it is not the main. Adding a new "-asan" option has been implenmented by CLs 298610, 298611, 298612, 298613. With these implementations,  the "go build -asan" can detect the error memory access for the above first case https://play.golang.org/p/eE_8k4poelj, but it can not detect the error memory access for the above second case https://play.golang.org/p/wtHjV-4xuiL. So CL 298614 (https://go-review.googlesource.com/c/go/+/298614/1) refactors the mallocgc(), allocates extra memory as the rezone, around the returned memory region and marks the redzone as unaddressable. With this change, it can detect the C code has out-of-bounds errors in cgo code. 

The address sanitizer also can detect the error memory access to heap,  global and stack objects in C.  But the above changes just detects the error memory to heap objects. For stack objexts, I got an answer from this discussion https://groups.google.com/g/golang-dev/c/YUkg5KIUov0/m/GYnYiZd5AAAJ that Go does not pass a stack-allocated object to C code, so ASan in Go does not need to consider the error memory access to stack objects.  For global objects,  I am still looking at it. 

Thank you

Fannie

Thanks.

Ian

[Ian Lance Taylor]

On Sun, Mar 7, 2021, 6:42 PM Fannie...@arm.com <Fannie...@arm.com> wrote:

On Saturday, March 6, 2021 at 6:45:19 AM UTC+8 Ian Lance Taylor wrote:

On Thu, Mar 4, 2021 at 11:37 PM Fannie Zhang <Fannie...@arm.com> wrote:
>
> The address sanitizer (Asan) is a memory error detector, it finds a number of memory access errors that can occur in C, such as out-of-bounds accesses to heap, stack and global objects, and use-after-free. In general, these issues do not occur in the pure Go code, unless people intentionally use the unsafe package. However, people can use cgo to pass memory back and forth between C and Go, which will cause memory access problems.
>
>
>
> Please refer to the following cases.
>
> In this case https://play.golang.org/p/eE_8k4poelj, C passes Go an invalid pointer, causing the Go code to have use-after-free errors.
>
> In this casehttps://play.golang.org/p/wtHjV-4xuiL, Go passes C a pointer, the C code has out-of-bounds access to heap objects errors.
>
> In this case https://play.golang.org/p/rOyEo7I7vTY, Go passes C a pointer, the C code has out-of-bounds access to global objects errors.
>
>
>
> Integrating Asan with Go would detect the above memory errors. So it is useful. And we can instrument ASan in exactly the same way as MSan. As with the -msan option, if we do not run with -asan option, it has no effect on code execution, if you use it, it may find some memory errors. In addition, Asan can help to detect some memory errors that MSan cannot. For example, use-after-free memory error case https://play.golang.org/p/eE_8k4poelj, MSan cannot detect writes on uninitialized memory, while ASan can.
>
>
>
> Although the above cases are not real world cases. However, on the other hand, when debugging a memory error access in a Go program that calls C code, it is really good to be able to use Asan to ensure that code is correct. Obviously, this will require more code to be maintained in the toolchain, but since almost all of the code is next to the existing MSan code, this is quite limited. Besides, Arm has a hardware feature MTE (Memory Tagging Extension), it aims to further mitigate these memory safety bugs by enabling us to detect them with low overhead. If ASan will be proven to be useful, we may consider enabling MTE for ASan. In this way, ASan even be enabled in the production environment to help to detect memory errors with low overhead. I saw a blog that introduces enable MTE in Android to detect the most common classes of memory safety bugs. Please refer to the document https://security.googleblog.com/2019/08/adopting-arm-memory-tagging-extension.html.
>
>
>
> I have updated the CLs for address sanitizer support, The implementation is divided into some smaller CLs 298610, 298611, 298612, 298613, 298614 for review. And with this implementation, the -asan option can check for error memory access to heap objects.
>
>
>
> Any opinions? Thank you.


Thanks for the explanation. This seems reasonable to me. Can you
open a proposal issue for this? See https://golang.org/s/proposal.

Thank you  Ian. I will do it right now. 

Thanks.

I
think the main change is that "go build" will get a new -asan option.

Yes, we need to add this change, but it is not the main.

Sorry, I meant that that is the main user visible charge.

Ian

[Fannie...@arm.com]

OK. I got it. Thank you. 🙂

Fannie

Ian

[Andrey Bokhanko]

Hi Zhang,

Great new development! -- definitely can be useful for us as well.

One question: do you plan to support detecting memory errors in pure Go code -- caused by sloppy use of unsafe pointers? Or is it purely for C/Go interaction?

Yours,
Andrey

===

Advanced Software Technology Lab

Huawei

пятница, 5 марта 2021 г. в 10:37:11 UTC+3, Fannie...@arm.com:

[Dmitry Vyukov]

Fannie...@arm.com wrote:

On Saturday, March 6, 2021 at 6:45:19 AM UTC+8 Ian Lance Taylor wrote:

On Thu, Mar 4, 2021 at 11:37 PM Fannie Zhang <Fannie...@arm.com> wrote:
>
> The address sanitizer (Asan) is a memory error detector, it finds a number of memory access errors that can occur in C, such as out-of-bounds accesses to heap, stack and global objects, and use-after-free. In general, these issues do not occur in the pure Go code, unless people intentionally use the unsafe package. However, people can use cgo to pass memory back and forth between C and Go, which will cause memory access problems.
>
>
>
> Please refer to the following cases.
>
> In this case https://play.golang.org/p/eE_8k4poelj, C passes Go an invalid pointer, causing the Go code to have use-after-free errors.
>
> In this casehttps://play.golang.org/p/wtHjV-4xuiL, Go passes C a pointer, the C code has out-of-bounds access to heap objects errors.
>
> In this case https://play.golang.org/p/rOyEo7I7vTY, Go passes C a pointer, the C code has out-of-bounds access to global objects errors.
>
>
>
> Integrating Asan with Go would detect the above memory errors. So it is useful. And we can instrument ASan in exactly the same way as MSan. As with the -msan option, if we do not run with -asan option, it has no effect on code execution, if you use it, it may find some memory errors. In addition, Asan can help to detect some memory errors that MSan cannot. For example, use-after-free memory error case https://play.golang.org/p/eE_8k4poelj, MSan cannot detect writes on uninitialized memory, while ASan can.
>
>
>
> Although the above cases are not real world cases. However, on the other hand, when debugging a memory error access in a Go program that calls C code, it is really good to be able to use Asan to ensure that code is correct. Obviously, this will require more code to be maintained in the toolchain, but since almost all of the code is next to the existing MSan code, this is quite limited. Besides, Arm has a hardware feature MTE (Memory Tagging Extension), it aims to further mitigate these memory safety bugs by enabling us to detect them with low overhead. If ASan will be proven to be useful, we may consider enabling MTE for ASan. In this way, ASan even be enabled in the production environment to help to detect memory errors with low overhead. I saw a blog that introduces enable MTE in Android to detect the most common classes of memory safety bugs. Please refer to the document https://security.googleblog.com/2019/08/adopting-arm-memory-tagging-extension.html.
>
>
>
> I have updated the CLs for address sanitizer support, The implementation is divided into some smaller CLs 298610, 298611, 298612, 298613, 298614 for review. And with this implementation, the -asan option can check for error memory access to heap objects.
>
>
>
> Any opinions? Thank you.


Thanks for the explanation. This seems reasonable to me. Can you
open a proposal issue for this? See https://golang.org/s/proposal.

Thank you  Ian. I will do it right now. 

Hi Fannie,

Have you filed the issue? I can't find it here:
https://github.com/golang/go/issues?q=is%3Aissue+is%3Aopen+label%3Aproposal

[Ian Lance Taylor]

On Thu, Mar 18, 2021 at 11:58 AM 'Dmitry Vyukov' via golang-dev
<golan...@googlegroups.com> wrote:
>
> Fannie...@arm.com wrote:
>>
>> On Saturday, March 6, 2021 at 6:45:19 AM UTC+8 Ian Lance Taylor wrote:
>>>
>>> Thanks for the explanation. This seems reasonable to me. Can you
>>> open a proposal issue for this? See https://golang.org/s/proposal.
>>
>> Thank you Ian. I will do it right now.
>

> Have you filed the issue? I can't find it here:
> https://github.com/golang/go/issues?q=is%3Aissue+is%3Aopen+label%3Aproposal

It is https://golang.org/issue/44853.

Ian

[Dmitry Vyukov]

Thanks (I guess I only searched by "asan").

Though somewhat orthogonal it may be useful to support memory
tagging/MTE in Go natively: runtime tags Go heap.
This would allow us to detect:
- bugs related to use of unsafe
- bugs in C when it operates on Go heap objects (out-of-bounds,
use-after-free's)
- bugs in GC when it messes things
- and some interesting Go bugs like misuses of sync.Pool, namely
sync.Pool could reuse memory but retag it so that all old references
will be invalidated
- similarly, it could enforce rules like "you must not access the
returned []byte after the next call to Scan" (some types could retag
leaked internal buffers to invalidate old pointers)

[Fannie...@arm.com]

On Friday, March 19, 2021 at 12:54:36 AM UTC+8 Andrey Bokhanko wrote:

Hi Zhang,

Great new development! -- definitely can be useful for us as well.

One question: do you plan to support detecting memory errors in pure Go code -- caused by sloppy use of unsafe pointers? Or is it purely for C/Go interaction?

The current implenmentation of ASan in Go just detect the error memory access to heap objects. So if the pointer is allocated on heap, the ASan can detecte the memory access errors caused by the use of unsafe pointers both in pure Go codes and CGO codes. 

The pure Go case: https://play.golang.org/p/W-i_5YC_qjX

The cgo case: https://play.golang.org/p/T_4YXgd2QrR

Therefore, whether ASan can detect memory access errors is not directly related to whether the unsafe packager is used.

We plan to support detecting the error memory access to global objects.  For stack objexts, I got an answer from this discussion https://groups.google.com/g/golang-dev/c/YUkg5KIUov0/m/GYnYiZd5AAAJ that Go does not pass a stack-allocated object to C code, for pure Go code, it is inherently asan-safe, unless people intentionally use the unsafe package, in which case they are on their own.so we may not support  stack objects.

Thank you.

Fannie

[Andrey Bokhanko]

вторник, 23 марта 2021 г. в 13:55:16 UTC+3, Fannie...@arm.com:

> The current implenmentation of ASan in Go just detect the error memory access to heap objects. So if the pointer is allocated on heap, the ASan can detecte the memory access errors caused by the use of unsafe pointers both in pure Go codes and CGO codes. 

> The pure Go case: https://play.golang.org/p/W-i_5YC_qjX

Let me modify your case a bit: https://play.golang.org/p/nKoBcMA2rjO

It seems that in the modified case (the only difference is stack vs heap for intSlice object) ASan won't detect a memory error, right? TBH, this is odd. Especially given that Go places objects on stack via escape analysis, that might became better in the future and start placing some previously heap objects on stack, essentially inadvertently removing them from ASan scope.

To clarify, I don't think this is a deal breaker for accepting your development; just a potential point of improvement.

Ian, Dmitry, do you think that an ASan extension that would add support for stack objects have a chance to be accepted? The crucial difference here is that Fannie presented a valid use case (for heap objects), while the only use case for checking stack objects I can think of is "sloppy usage of unsafe pointers".

Yours,

Andrey

[Fannie...@arm.com]

On Wednesday, March 24, 2021 at 11:36:25 PM UTC+8 Andrey Bokhanko wrote:

вторник, 23 марта 2021 г. в 13:55:16 UTC+3, Fannie...@arm.com:

> The current implenmentation of ASan in Go just detect the error memory access to heap objects. So if the pointer is allocated on heap, the ASan can detecte the memory access errors caused by the use of unsafe pointers both in pure Go codes and CGO codes. 

> The pure Go case: https://play.golang.org/p/W-i_5YC_qjX

Let me modify your case a bit: https://play.golang.org/p/nKoBcMA2rjO

It seems that in the modified case (the only difference is stack vs heap for intSlice object) ASan won't detect a memory error, right?

The  ASan can detect the memory error in the modified case. Because "intSlice" is moved to heap via escape analysis.

In this pure Go case https://play.golang.org/p/wPzqFFLTcoi, the memory error cannot be detected by ASan.

 

TBH, this is odd. Especially given that Go places objects on stack via escape analysis, that might became better in the future and start placing some previously heap objects on stack, essentially inadvertently removing them from ASan scope.

To clarify, I don't think this is a deal breaker for accepting your development; just a potential point of improvement 

Can you show us an example that Go placing some previously heap objects on stack via escape analysis?  Thank you. 

Best regards,

Fannie

[Fannie...@arm.com]

On Wednesday, March 24, 2021 at 11:36:25 PM UTC+8 Andrey Bokhanko wrote:

вторник, 23 марта 2021 г. в 13:55:16 UTC+3, Fannie...@arm.com:

Ian, Dmitry, do you think that an ASan extension that would add support for stack objects have a chance to be accepted? The crucial difference here is that Fannie presented a valid use case (for heap objects), while the only use case for checking stack objects I can think of is "sloppy usage of unsafe pointers".

To support for heap objects, we change the go memeory allocator and insert redzone around the heap objects. Support for stack objects also need to change the frame layout and insert redzone around the stack objects, this refactor may cause some problems.  1. In the assmebly code, we use pseudo-register SP and FP to pass parameters and get input parameters,  changing the frame layout will cause incompatibility and break the exostomg assembly codes. 2. Spill varibales need to recosider their offset. 

Support for stack objects may be able to detect some memory errors, but it has little effect(as I said above, it is only used for pure Go with usafe package), and the workload is unpredicatbale. At present, we have no good ideas and no plan to implement it. 

Any suggestions are welcome.  

Thank you.

Fannie 

[Dmitry Vyukov]

Sounds reasonable to me.
Either way, I would consider stack only after heap is implemented and
deployed. There is little point in planning/deciding re stack before
that.
And if I go for stack, I would consider only supporting it with memory
tagging (MTE), or at least with HWASAN mode (software tagging). It
won't require quarantining stack frames at least.

[Fannie...@arm.com]

We also planned this way. Thank you.

Best regards,

Fannie

[Andrey Bokhanko]

Fannie,

четверг, 25 марта 2021 г. в 05:31:13 UTC+3, Fannie...@arm.com:

It seems that in the modified case (the only difference is stack vs heap for intSlice object) ASan won't detect a memory error, right?

The  ASan can detect the memory error in the modified case. Because "intSlice" is moved to heap via escape analysis.

Ahhh, yes -- Println call prevents placing of intSlice on stack. My bad.

 

In this pure Go case https://play.golang.org/p/wPzqFFLTcoi, the memory error cannot be detected by ASan.

Yeah, this case illustrates my point.

 

Can you show us an example that Go placing some previously heap objects on stack via escape analysis?  Thank you. 

You wrote such a case youself, no? https://play.golang.org/p/wPzqFFLTcoi. Here escape analysis moved intSlice from heap to stack.

 

Compare our two cases. Adding a simple Println call enables ASan for intSlice. I still maintain this would be odd for most users. :-)

Yours,

Andrey

[Andrey Bokhanko]

четверг, 25 марта 2021 г. в 10:47:09 UTC+3, Dmitry Vyukov:

Does it mean you won't consider accepting ASan for stack objects for non-ARM platforms, implemented purely in software?

Yours,

Andrey

 

[Александр Калинин]

Hello!

The problem is that both MTE and HWASAN work perfectly only on AArch64 (with an exception of amd64 HWASAN implementation on Clang, which is limited in functionality). Do you consider accepting classic gcc/libasan implementation for stack objects on amd64 platform?

Best wishes, Alex

четверг, 25 марта 2021 г. в 10:47:09 UTC+3, Dmitry Vyukov:

On Thu, Mar 25, 2021 at 8:35 AM Fannie...@arm.com <Fannie...@arm.com> wrote:

[Fannie...@arm.com]

As we discussed above,   we currently have no good ideas and no plan to implement it. Thank you.

[Andrey Bokhanko]

среда, 26 мая 2021 г. в 06:39:41 UTC+3, Fannie...@arm.com:

As we discussed above,   we currently have no good ideas and no plan to implement it. Thank you.

IMHO, this leaves a big hole -- no proper ASan support for stack objects on x86-64 hardware. x86 is still a dominant server platform, which is the primary use case for Go.

TBH, right now we're silently accepting a dead end by design.

Yours,

Andrey

[Alexander Kalinin]

Fannie, can you also have a look at this snippet? 

https://play.golang.org/p/CiMwFRkwre3

As classic gcc-libasan sets global signal handler, I guess it may tamper with panic handling. Could you please try this sample under ASan? The expected output is "Handler executed".

Best wishes,

Alex

среда, 26 мая 2021 г. в 17:09:40 UTC+3, Andrey Bokhanko:

[Andrey Bokhanko]

(For the full disclosure, myself and Alexander work for the same company -- Huawei. However, our opinions are individual and we may even disagree with each other :-))

среда, 26 мая 2021 г. в 17:45:18 UTC+3, Alexander Kalinin:

[Fannie...@arm.com]

I tested your case on my development machines of x86 and arm64, with -asan option,  the output is "Handler executed", as you expected. 

Thank you,

Fannie

[Alexander Kalinin]

Fannie, thanks for this test! Glad you do not have this problem. I had this problem on libasan5 and lower, but libasan6 seems to be fine. Just to note, workaround for older versions would be using environment variable ASAN_OPTIONS="handle_segv=0".

P.S.

I also would be happy to review your patches, if you do not mind.

Best wishes, 
Alexander

четверг, 27 мая 2021 г. в 05:56:49 UTC+3, Fannie...@arm.com:

[Fannie...@arm.com]

Use gcc-7 (libasan.so.4 ) as C compiler,  this test gets segmentation fault error as the below.

"ASAN:DEADLYSIGNAL

=================================================================

==23021==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x00000052833c bp 0x20400021fde8 sp 0x20400021fdf0 T0)

==23021==The signal is caused by a READ memory access.

==23021==Hint: address points to the zero page.

    #0 0x52833b in regexp.(*Regexp).doExecute /home/fanzha02/work/go_project/govscode/gomain/src/regexp/exec.go:527

"

And as you said, set  ASAN_OPTIONS="handle_segv=0" environment varibale, it gets expected output "Handler executed". 

This error is go's issue or gcc's issue? 

Very welcome to review these patches.  🙂

Thank you,

Fannie

[Alexander Kalinin]

On the one hand, it is not go-related issue. It's either libasan or gcc-related, but the problem is seems to be not solvable in nice ways, because libasan sets its handler too early: if you set this environment inside go startup code, it will be too late for libasan to read.

On the other hand, in this case, go does not insert nil-check, and it seems to be strange: not sure if it is the best way to handle this situation by handling segv. I will try to research this issue regarding nilcheck.

Best wishes,

Alexander

понедельник, 31 мая 2021 г. в 05:15:32 UTC+3, Fannie...@arm.com: