I was working on #78633 and got very puzzled by theses 5 optimizations triggers:
runtime/set_vma_name_linux.go:29:14: known value of v89: 5
# crypto/internal/fips140/sha256
crypto/internal/fips140/sha256/sha256.go:105:7: known value of v968: 64
# crypto/internal/fips140/sha512
crypto/internal/fips140/sha512/sha512.go:188:7: known value of v1526: 128
# crypto/md5
crypto/md5/md5.go:93:7: known value of v579: 64
# crypto/sha1
crypto/sha1/sha1.go:82:7: known value of v616: 64
The code looks like this:
target = (And64 x (Const32 [-1]))
As a side effect of how my pass is written it finds the constant folding between the Const32 and the And64.
However this is illegal, it should never happen.
Here is the buggy snippet of code:
cap := v.Args[0]
x, delta := isConstDelta(cap)
if x != nil {
// slicemask(x + y)
// if x is larger than -y (y is negative), then slicemask is -1.
lim := ft.limits[x.ID]
if lim.umin > uint64(-delta) {
if cap.Op == OpAdd64 {
v.reset(OpConst64)
} else {
v.reset(OpConst32)
}
if b.Func.pass.debug > 0 {
b.Func.Warnl(v.Pos, "Proved slicemask not needed")
}
v.AuxInt = -1
}
break
}
lim := ft.limits[cap.ID]
if lim.umin > 0 {
if cap.Type.Size() == 8 {cap.Op == OpAdd64 tries to use a Const64 if we are compiling on a 64bits machine.
However in these cases sadly the cap is not Add64 it is Sub64.
The correct way to write this is seen in the similar (limits) code bellow: if cap.Type.Size() == 8 {
On some architectures (s390x is the only one that comes to my mind) loading a 32 bits constant leaves garbage in the upper 32 bits of the register.
On others (like amd64) the upper half is zeroed out.
This means we then perform an And64 between the shift amount and this half garbage half all ones register before adding it to the pointer.
We might get a value shorter out of the And.
The 5 cases in the std are safe since the reslice amount is constant and fits in 32 bits.
So at worst for the std the reslice amount clears out the garbage from the upper half zeroing it as expected.
I don't see why this would work if you reslices >= 1<<32 elements tho.
Since then the slice amount will be truncated by the half zero (or garbage) register.
This means you won't advance the pointer inside the slice as far as you should.
IMO this is extremely unlikely;
- you need a Sub rather than Add argument to a Slicemas
- that Slicemask must be proved in bounds
- you need to slice by an index that doesn't fit in an uint32
- you need to be running architectures that sext 32bits constants to 64bits (amd64, s390x, ...) (ok that one is likely)
Here is a repro (easy version that uses lots of ram):
package main
import (
"encoding/binary"
"fmt"
)
//go:noinline
func bug(s []byte) []byte {
if len(s) < 1<<34+8 {
panic("too short")
}
return s[min(1<<34, len(s)):]
}
func main() {
s := make([]byte, 1<<34+8)
sc := s
for i := uint64(0); len(sc) >= 8; i += 8 {
binary.LittleEndian.PutUint64(sc, i)
sc = sc[8:]
}
if v := binary.LittleEndian.Uint64(bug(s)); v != 1<<34 {
panic(fmt.Sprintf("got %d, want %d", v, 1<<34))
}
}panics due to the missadvanced pointer
Running with -gcflags=-N fixes the issue.
This was reported to security@golang.org but was judged fixable in the open.
I was working on #78633 and got very puzzled by theses 5 optimizations triggers:
The code looks like this:
As a side effect of how my pass is written it finds the constant folding between the Const32 and the And64.
However this is illegal, it should never happen.
Here is the buggy snippet of code:
cap.Op == OpAdd64tries to use a Const64 if we are compiling on a 64bits machine.However in these cases sadly the cap is not Add64 it is Sub64.
The correct way to write this is seen in the similar (limits) code bellow: if cap.Type.Size() == 8 {
On some architectures (s390x is the only one that comes to my mind) loading a 32 bits constant leaves garbage in the upper 32 bits of the register.
On others (like amd64) the upper half is zeroed out.
This means we then perform an And64 between the shift amount and this half garbage half all ones register before adding it to the pointer.
We might get a value shorter out of the And.
The 5 cases in the std are safe since the reslice amount is constant and fits in 32 bits.
So at worst for the std the reslice amount clears out the garbage from the upper half zeroing it as expected.
I don't see why this would work if you reslices >= 1<<32 elements tho.
Since then the slice amount will be truncated by the half zero (or garbage) register.
This means you won't advance the pointer inside the slice as far as you should.
IMO this is extremely unlikely;
Here is a repro (easy version that uses lots of ram):
panics due to the missadvanced pointer
Running with -gcflags=-N fixes the issue.
This was reported to security@golang.org but was judged fixable in the open.