Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/crypto/sha3: TestKeccakKats failing on freebsd-amd64-race due to invalid unsafe pointer conversion #35173

Closed
bcmills opened this issue Oct 26, 2019 · 2 comments
Closed

x/crypto/sha3: TestKeccakKats failing on freebsd-amd64-race due to invalid unsafe pointer conversion #35173

bcmills opened this issue Oct 26, 2019 · 2 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@bcmills
Copy link
Contributor

bcmills commented Oct 26, 2019

From the freebsd-amd64-race builder (https://build.golang.org/log/dbfde792a0d9d30fa34b24190b0023988ef2e63d):

--- FAIL: TestKeccakKats (1.95s)
panic: runtime error: unsafe pointer conversion [recovered]
	panic: runtime error: unsafe pointer conversion

goroutine 20 [running]:
testing.tRunner.func1(0xc0000ba100)
	/tmp/workdir/go/src/testing/testing.go:881 +0x69f
panic(0x6473e0, 0xc00009aac0)
	/tmp/workdir/go/src/runtime/panic.go:915 +0x370
golang.org/x/crypto/sha3.xorInUnaligned(0xc000104a80, 0xc000104b69, 0x90, 0xa8)
	/tmp/workdir/gopath/src/golang.org/x/crypto/sha3/xor_unaligned.go:13 +0x60
golang.org/x/crypto/sha3.(*state).permute(0xc000104a80)
	/tmp/workdir/gopath/src/golang.org/x/crypto/sha3/sha3.go:84 +0x1d3
golang.org/x/crypto/sha3.(*state).padAndPermute(0xc000104a80, 0xc000036706)
	/tmp/workdir/gopath/src/golang.org/x/crypto/sha3/sha3.go:117 +0x347
golang.org/x/crypto/sha3.(*state).Read(0xc000104a80, 0xc0000d45c0, 0x1c, 0x1c, 0x453700, 0xc0000818d0, 0x8)
	/tmp/workdir/gopath/src/golang.org/x/crypto/sha3/sha3.go:163 +0x2e0
golang.org/x/crypto/sha3.(*state).Sum(0xc0001048c0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
	/tmp/workdir/gopath/src/golang.org/x/crypto/sha3/sha3.go:190 +0x233
golang.org/x/crypto/sha3.TestKeccakKats.func1(0x661d91, 0x9)
	/tmp/workdir/gopath/src/golang.org/x/crypto/sha3/sha3_test.go:116 +0x5a9
golang.org/x/crypto/sha3.testUnalignedAndGeneric(0xc0000ba100, 0xc00008df00)
	/tmp/workdir/gopath/src/golang.org/x/crypto/sha3/sha3_test.go:84 +0x14f
golang.org/x/crypto/sha3.TestKeccakKats(0xc0000ba100)
	/tmp/workdir/gopath/src/golang.org/x/crypto/sha3/sha3_test.go:93 +0x5c
testing.tRunner(0xc0000ba100, 0x66bd20)
	/tmp/workdir/go/src/testing/testing.go:916 +0x1a9
created by testing.(*T).Run
	/tmp/workdir/go/src/testing/testing.go:967 +0x652
FAIL	golang.org/x/crypto/sha3	1.966s

CC @FiloSottile @mknyszek
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 26, 2019
@gopherbot gopherbot added this to the Unreleased milestone Oct 26, 2019
@bcmills
Copy link
Contributor Author

bcmills commented Oct 26, 2019

Looks like the wild reads there are conditional, and the punned slice type is of uint64 so the collector won't try to chase pointers through it.

I'm not sure whether that's actually unsafe in practice, but it does potentially produce an out-of-object slice.

@bcmills bcmills self-assigned this Oct 28, 2019
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/203837 mentions this issue: sha3: align (*state).storage

@albrow albrow mentioned this issue Mar 4, 2020
Closed
@bwesterb bwesterb mentioned this issue Mar 4, 2020
Closed
bwesterb added a commit to bwesterb/circl that referenced this issue Mar 4, 2020
@bwesterb
Align internal SHAKE buffer when using it as []uint64
97c94ab 
The Go runtime assumes that a pointer to a type is aligned on the size
of that type.  We cast a [N]byte to []uint64, which breaks this
assumption.  Thus allocate the original [N]byte as a [N/8]uint64 instead.

Fixes cloudflare#89

Cf. the same issue and patch in upstream:

  patch https://go-review.googlesource.com/c/crypto/+/203837/
  issue golang/go#35173
@bwesterb bwesterb mentioned this issue Mar 4, 2020
Merged
@kriskwiatkowski kriskwiatkowski mentioned this issue Mar 4, 2020
Open
armfazh pushed a commit to cloudflare/circl that referenced this issue Mar 4, 2020
@bwesterb @armfazh
Align internal SHAKE buffer when using it as []uint64
0c5a078 
The Go runtime assumes that a pointer to a type is aligned on the size
of that type.  We cast a [N]byte to []uint64, which breaks this
assumption.  Thus allocate the original [N]byte as a [N/8]uint64 instead.

Fixes #89

Cf. the same issue and patch in upstream:

  patch https://go-review.googlesource.com/c/crypto/+/203837/
  issue golang/go#35173
@golang golang locked and limited conversation to collaborators Nov 4, 2020
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 28, 2022
@bradfitz
sha3: align (*state).storage
35d7367 
Even on platforms that allow unaligned reads, the Go runtime assumes
that a pointer to a given type has the alignment required by that
type.

Fixes golang/go#35173
Updates golang/go#34972
Updates golang/go#34964

Change-Id: I90361e096e59162e42ebde2914985af92f777ece
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/203837
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
@bradfitz
sha3: align (*state).storage
2b99290 
Even on platforms that allow unaligned reads, the Go runtime assumes
that a pointer to a given type has the alignment required by that
type.

Fixes golang/go#35173
Updates golang/go#34972
Updates golang/go#34964

Change-Id: I90361e096e59162e42ebde2914985af92f777ece
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/203837
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
@bradfitz
sha3: align (*state).storage
439bd5c 
Even on platforms that allow unaligned reads, the Go runtime assumes
that a pointer to a given type has the alignment required by that
type.

Fixes golang/go#35173
Updates golang/go#34972
Updates golang/go#34964

Change-Id: I90361e096e59162e42ebde2914985af92f777ece
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/203837
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
@rsc rsc unassigned bcmills Jun 23, 2022
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
@bradfitz
sha3: align (*state).storage
34d8ed4 
Even on platforms that allow unaligned reads, the Go runtime assumes
that a pointer to a given type has the alignment required by that
type.

Fixes golang/go#35173
Updates golang/go#34972
Updates golang/go#34964

Change-Id: I90361e096e59162e42ebde2914985af92f777ece
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/203837
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
@bradfitz
sha3: align (*state).storage
057f033 
Even on platforms that allow unaligned reads, the Go runtime assumes
that a pointer to a given type has the alignment required by that
type.

Fixes golang/go#35173
Updates golang/go#34972
Updates golang/go#34964

Change-Id: I90361e096e59162e42ebde2914985af92f777ece
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/203837
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
desdeel2d0m added a commit to desdeel2d0m/crypto that referenced this issue Jul 1, 2024
@desdeel2d0m @bradfitz
sha3: align (*state).storage
2cf1922 
Even on platforms that allow unaligned reads, the Go runtime assumes
that a pointer to a given type has the alignment required by that
type.

Fixes golang/go#35173
Updates golang/go#34972
Updates golang/go#34964

Change-Id: I90361e096e59162e42ebde2914985af92f777ece
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/203837
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
2 participants
@bcmills @gopherbot