runtime: non-atomic pointer writes in memclr/memmove

[dvyukov]

The following program crashes when run with GODEBUG=invalidptr=1 GOARCH=386:

package main

import (
    "math/rand"
    "runtime"
)

func main() {
    P := 2 * runtime.GOMAXPROCS(0)
    ptrs := make([][]*int, P)
    for i := 0; i < P; i++ {
        i := i
        go func() {
            r := rand.New(rand.NewSource(int64(i)))
            var sink []byte
            p := make([]*int, 4)
            z := make([]*int, 4)
            ptrs[i] = p
            for {
                if r.Intn(20) != 0 {
                    a := p[:1]
                    a[0] = new(int)
                    for j := range a {
                        a[j] = nil
                    }
                    for j := range p {
                        p[j] = new(int)
                    }
                    copy(p[:3], z)
                    for j := range p {
                        p[j] = new(int)
                    }
                    copy(p[1:4], z)
                    for j := range p {
                        p[j] = new(int)
                        copy(p[j:j+1], z)
                    }
                } else {
                    sink = make([]byte, 8<<10)
                }
            }
            _ = sink
        }()
    }
    select {}
}

GODEBUG=invalidptr=1 GOARCH=386 go run memclr.go

runtime: pointer 0x18d90000 to unallocated spanidx=0x548 span.start=0x185d2000 span.limit=0x185d4000 span.state=3
runtime: found in object at *(0x18456000+0x0)
object=0x18456000 k=0xc22b s.start*_PageSize=0x18456000 s.limit=0x18458000 s.sizeclass=2 s.elemsize=16
 *(object+0) = 0x0 <==
 *(object+4) = 0x0
 *(object+8) = 0x18d9b354
 *(object+12) = 0x18d9b3b0
fatal error: found bad pointer in Go heap (incorrect use of unsafe or cgo?)

The problem is that memclr clears 4-byte regions on 386 with 2 separate 2-byte stores:

_3or4:
    MOVW    AX, (DI)
    MOVW    AX, -2(DI)(BX*1)

This exposes corrupted pointer to GC.

Not sure whether it can crash without invalidptr, I would not exclude such possibility. But it's clearly bad and can lead to false retention. Also it makes data race failure modes significantly worse, introducing exploitation possibilities like use-after-free and easy controlled overwrite.

There is similar issue in amd64p32 memclr which clears pointers with byte stores:

TEXT runtime·memclr(SB),NOSPLIT,$0-8
    MOVL    ptr+0(FP), DI
    MOVL    n+4(FP), CX
    MOVQ    CX, BX
    ANDQ    $3, BX
    SHRQ    $2, CX
    MOVQ    $0, AX
    CLD
    REP
    STOSL
    MOVQ    BX, CX
    REP
    STOSB
    RET

Similar issue in 386 memmove which moves pointer with 2 stores:

move_3or4:
    MOVW    (SI), AX
    MOVW    -2(SI)(BX*1), CX
    MOVW    AX, (DI)
    MOVW    CX, -2(DI)(BX*1)
    RET

Similar issue in plan9/386 memclr:

_1or2:
    MOVB    AX, (DI)
    MOVB    AX, -1(DI)(BX*1)

Similar issue in amd64 memclr:

_5through8:
    MOVL    AX, (DI)
    MOVL    AX, -4(DI)(BX*1)

Did not look at arm/ppc. I guess we just need to go through all of them one-by-one.
@rsc @randall77 @RLH @aclements @dr2chase @davecheney @minux @0intro @4ad

[dvyukov]

@ality

[aclements]

Amusingly arm/ppc are probably okay because we tracked this exact issue down in #12552. At the time I thought the other platforms were okay, but you're definitely right about the tails.

[randall77]

I checked all the memclr & memmove code, CL out for review. It was just x86, the other architectures look ok to me.

The only other place we do byte-at-a-time copy is in the reflect.callX functions. Both the source (a newly allocated chunk of memory) and the destination (the local stack) should be ok for non-atomic pointer copy.

[gopherbot]

CL https://golang.org/cl/16668 mentions this issue.

[rsc]

This might be worth fixing for Go 1.5.2, if we release a Go 1.5.2.