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 the current implementation, the -asan option can check for error memory access to heap objects. Thank you.

I care about size of binary. Can you compare both size including small samples(<5M) and big samples(>10M)?

@AZ-X If you run the go program without the -asan option, the size of binary will not change. Generally, we only enable the address sanitizer in the test environment to help detect memory access issues, so I think the size of binary is not important. Thank you.

@AZ-X Regarding your question, my answer above seems too one-sided. I wrote a test case to compare the size of binary, and the size of binary with -asan is almost 10% larger than that without -asan. Please refer to the following results.

func main() {
        var overall [][]int
        var m runtime.MemStats
        s := 2*1024*1024                 //  >10M big samples
        //s := 10*1024                     // <1M small samples
        for i := 0; i < 10; i++ {
                a := make([]int, 0, 10*s)
                overall = append(overall, a)
        }
        runtime.ReadMemStats(&m)
        fmt.Printf("Alloc = %v MB", bToMb(m.Alloc))
}
func bToMb(b uint64) uint64 {
        return b / 1024 / 1024
}

// the binay size:

  size           binary
2154616   big_asan               // build with -asan and big samples
1950161   big_noasan           // build without -asan and big samples  
2154624   small_asan            // build with -asan and small samples
1950161   small_noasan        // build without -asan and small samples
1940941   big_main              // build with master go and big samples
1940949   small_main           // build with master go and small samples

From the above result, the size of binary is bigger. But as I mentioned above, we don't recommend opening the asan option in a production environment. The factors to consider are not only size, but also performance. The good news is that ARM64 has a new feature of MTE, which is designed to detect error memory accesses with lower overhead. If ASan with MTE is enabled in Go in the future, the impact on performance and the size of the binary will be relatively small. Thank you.

If ASan with MTE is enabled in Go in the future

Do we need any support for this at all?
I would assume this will be done by just exporting some env var to enable MTE in the system malloc, e.g.:
https://sourceware.org/pipermail/libc-alpha/attachments/20200615/c8ef9e4e/attachment-0001.bin
https://patchwork.ozlabs.org/project/glibc/patch/8306e032-f980-a409-5239-74629e79d041@arm.com/
Or maybe in future it will be the default, so one doesn't need to do anything at all.

Though it can make sense to support MTE in Go runtime:
https://groups.google.com/g/golang-dev/c/ACKO07YeeW8/m/DX61CERlAwAJ

Or maybe in future it will be the default, so one doesn't need to do anything at all.

In fact, we need to do something. Because Go has its own memory allocator, see https://golang.org/src/runtime/malloc.go, we may follow the implementaion in glic, refactor Go memory allcator (regarding the implementation details, I haven't done in-depth research. 🙂), setup memory tagging support if the hardware support it and if the user has requested it. Thank you.

Yes, that's what I mean by "support MTE in Go runtime". But we don't need anything related to asan (linking asan, asan hooks, etc)

Yes, that's what I mean by "support MTE in Go runtime". But we don't need anything related to asan (linking asan, asan hooks, etc)

Yes, you are right. With MTE, all load and store instructions, except for with SP base register and immediate offset, do check tags. Here https://github.com/google/sanitizers/wiki/Stack-instrumentation-with-ARM-Memory-Tagging-Extension-(MTE) introduces how MTE works for stack. Thank you.

Change https://golang.org/cl/298610 mentions this issue: cmd/link: add -asan option

Change https://golang.org/cl/298611 mentions this issue: cmd/compile: add -asan option

Change https://golang.org/cl/298612 mentions this issue: cmd/go: add -asan option

Change https://golang.org/cl/298613 mentions this issue: runtime, runtime/asan: add asan runtime support

Change https://golang.org/cl/298614 mentions this issue: runtime, syscall: add calls to asan functions

Change https://golang.org/cl/298615 mentions this issue: cmd/dist: add asan tests in misc/cgo/testsanitizers package

any updates?

any updates?

It has entered the code freeze for Go 1.17. 😟

@ianlancetaylor Any plans for it in 1.18? Thank you.

@AZ-X Regarding your question, my answer above seems too one-sided. I wrote a test case to compare the size of binary, and the size of binary with -asan is almost 10% larger than that without -asan. Please refer to the following results.

func main() {
        var overall [][]int
        var m runtime.MemStats
        s := 2*1024*1024                 //  >10M big samples
        //s := 10*1024                     // <1M small samples
        for i := 0; i < 10; i++ {
                a := make([]int, 0, 10*s)
                overall = append(overall, a)
        }
        runtime.ReadMemStats(&m)
        fmt.Printf("Alloc = %v MB", bToMb(m.Alloc))
}
func bToMb(b uint64) uint64 {
        return b / 1024 / 1024
}

// the binay size:

  size           binary
2154616   big_asan               // build with -asan and big samples
1950161   big_noasan           // build without -asan and big samples  
2154624   small_asan            // build with -asan and small samples
1950161   small_noasan        // build without -asan and small samples
1940941   big_main              // build with master go and big samples
1940949   small_main           // build with master go and small samples

From the above result, the size of binary is bigger. But as I mentioned above, we don't recommend opening the asan option in a production environment. The factors to consider are not only size, but also performance. The good news is that ARM64 has a new feature of MTE, which is designed to detect error memory accesses with lower overhead. If ASan with MTE is enabled in Go in the future, the impact on performance and the size of the binary will be relatively small. Thank you.

I assume all these measurements are done on ARM, right? What about x86?

Also, do you plan to implement first-class ASan support on x86-64? (Obviously, without MTE, as it's not available on x86)

Yours,
Andrey
===
Advanced Software Technology Lab
Huawei

I assume all these measurements are done on ARM, right? What about x86?

The size of binary with -asan is almost 11% larger than that without -asan. Please refer to the following results.

The binary size on x86-64.

      size                 binary
    1998048          big_asan
    1785270          big_noasan
    1785246          big_main
    1998048         small_asan
    1785270         small_noasan
    1785246         small_main

Also, do you plan to implement first-class ASan support on x86-64? (Obviously, without MTE, as it's not available on x86)

The current implementation also enables the ASan support on x86-64. Thank you.

Change https://golang.org/cl/321715 mentions this issue: cmd/compile: enable Asan check for global variables

Change https://golang.org/cl/321716 mentions this issue: cmd/dist: add asan tests for global objects in testsanitizers package

Hello!

What are your plans on supporting ASan for stack-based objects? The problem is that usage of unsafe.Pointer does not affect escape analysis, and there are lots of cases with potentially insecure dereference of unsafe.Pointer to stack objects. AFAIK go does not generate any stack canary code, so those vulnerabilities should be easily exploitable.
Simple example:
https://play.golang.org/p/5BLnDJtleF4
Here, due to misuse of unsafe.Pointer we have easy IP control.

Also, if you add MTE support, which runtime are you going to use, and will it be part of Go Asan implementation, or another feature?

Best wishes, Alex

For reference, there was some discussion of some different aspects of this proposal in this golang-dev thread:

https://groups.google.com/g/golang-dev/c/ACKO07YeeW8

@ph1048, FYI, there was some discussion there around stack objects, though perhaps @zhangfannie will have more to say here.

Also, if you add MTE support, which runtime are you going to use, and will it be part of Go Asan implementation, or another feature?

@ph1048 Add MTE support is another feature, as @dvyukov commented above, we do not need to call any compiler-rt address sanitizer runtime functions. Thank you.

With the implementation of CL 321715 , ASan in Go can detect the error memory access to heap objects and global objects. For stack objects, as we discussed in the golang-dev mailing list https://groups.google.com/g/golang-dev/c/ACKO07YeeW8/m/B93kNorLBgAJ, we currently have no plans to add support for it. In this way, support ASan in Go is complete.

Welcome to review these patches. Thank you.

Hi @zhangfannie

With all respect, I find these two statements:

For stack objects, as we discussed in the golang-dev mailing list https://groups.google.com/g/golang-dev/c/ACKO07YeeW8/m/B93kNorLBgAJ, we currently have no plans to add support for it.

In this way, support ASan in Go is complete.

to be mutually contradictory.

Without stack objects being supported, ASan for Go is definitely not "complete". "ASan for heap and global objects" can be called as complete, sure -- but I still maintain that users shouldn't care on where their objects are allocated, especially giving that escape analysis can move objects from heap to stack easily.

As for plans to add support for stack objects, here is what @dvyukov wrote in the mailing list:

Either way, I would consider stack only after heap is implemented and
deployed. There is little point in planning/deciding re stack before
that.

I read this as "let's do heap objects first, then decide on stack objects support".

@andreybokhanko

Thank you for correct my expression and "ASan for heap and global objects is complete" is more accurate.

As for plans to add support for stack objects, here is what I wrote in the mailing list:

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.

One problem is the compatibility of abi. Go passes a value, it is also equal to pass the memory, we can use unsafe.Pointer to access the parameter adress and return adress. So far, I am not very clear about the definition of Go calling convention. I am not sure it is passing by value, or passing a memory region shared by caller and callee.

For example, in the following case, if ASan in Go wants to detect this error memory acces to stack objects, we need to insert redzone around the pass variables(a, b) and return variable (ret), which will break the current abi.

package main

import "fmt"
import "unsafe"

//go:noinline
func add(x int, y int) (ret int) {
	*(* int)(unsafe.Pointer((uintptr(unsafe.Pointer(&ret)) - 1*unsafe.Sizeof(ret)))) = 123    // Is it an error?
	*(* int)(unsafe.Pointer((uintptr(unsafe.Pointer(&ret)) - 2*unsafe.Sizeof(ret)))) = 543    // Is it an error?

	ret = x + y
	return
}

func main() {
	fmt.Println(add(42, 13))
}

At present, we do not have a good idea to support it. There should be 3 kinds of stack memory need to be considered, including local variables, argument variables and return variables.

If there is no clear calling conversion definition at the moment, maybe this can be a part of the discussion in regabi. @cherrymui

Change https://golang.org/cl/368834 mentions this issue: doc/go1.18: mention new -asan option

Change https://golang.org/cl/375256 mentions this issue: runtime: add address sanitizer support for riscv64

Change https://go.dev/cl/393315 mentions this issue: cmd/compile: set conversions to unsafe.Pointer as an escaping operation when -asan is enabled

Change https://go.dev/cl/401775 mentions this issue: cmd/compile: enable Asan check for global variables

Change https://go.dev/cl/403851 mentions this issue: cmd/compile: enable Asan check for global variables

Change https://go.dev/cl/404214 mentions this issue: cmd/compile: enable Asan check for global variables

Is there anything else need to be done for this? Thanks.

@cherrymui The implementation mentioned in the proposal is complete, we can close it. Thanks for the reminder.

@zhangfannie thanks for the getting this done!

Change https://go.dev/cl/414516 mentions this issue: cmd/compile: drop "buildcfg" from no instrument packages

Change https://go.dev/cl/408814 mentions this issue: runtime: add address sanitizer support for ppc64le