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.

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

x/crypto/blake2b: supportSSE4 detection wrong, SIGILL on Atom D2700 #18113

Closed
ghost opened this issue Nov 30, 2016 · 5 comments
Closed

x/crypto/blake2b: supportSSE4 detection wrong, SIGILL on Atom D2700 #18113

ghost opened this issue Nov 30, 2016 · 5 comments

Comments

@ghost
Copy link

ghost commented Nov 30, 2016

% go version
go version go1.7.3 linux/amd64

% go env
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH=""
GORACE=""
GOROOT="/usr/lib/go"
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build635952482=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"

I am using Arch Linux x86-64 with 4.8.10-1-arch Linux kernel. I am using some code that runs blake2b, but it fails with the following error:

SIGILL: illegal instruction
PC=0x56d15a m=7

goroutine 22 [running]:
golang.org/x/crypto/blake2b.hashBlocksSSE4(0xc4200f2040, 0x0, 0xc4200f2058, 0x80, 0x80, 0xc420038be8)
        /tmp/.../aur-govpn/src/govpn-7.0/src/golang.org/x/crypto/blake2b/blake2b_amd64.s:145 +0x7a fp=0xc420038b60 sp=0xc420038b30

seems that it is trying to call SSE4.1-related assembler instruction, but host CPU does not support it.

% cat /proc/cpuinfo
processor       : 0
vendor_id       : GenuineIntel
cpu family      : 6
model           : 54
model name      : Intel(R) Atom(TM) CPU D2700   @ 2.13GHz
stepping        : 1
microcode       : 0x103
cpu MHz         : 2127.966
cache size      : 512 KB
physical id     : 0
siblings        : 4
core id         : 0
cpu cores       : 2
apicid          : 0
initial apicid  : 0
fpu             : yes
fpu_exception   : yes
cpuid level     : 10
wp              : yes
flags           : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx lm constant_tsc arch_perfmon pebs bts nopl
+nonstop_tsc aperfmperf eagerfpu pni dtes64 monitor ds_cpl tm2 ssse3 cx16 xtpr pdcm movbe lahf_lm dtherm arat
bugs            :
bogomips        : 4257.26
clflush size    : 64
cache_alignment : 64
address sizes   : 36 bits physical, 48 bits virtual
[...]

supportSSE4 assembler code has the following line:

SHRL $15, CX  // Bit 15 indicates SSE4 support

but Wikipedia says that SSE4.1 is indicated by 19-bit: https://en.wikipedia.org/wiki/SSE4#SSE4.1

I have never programmed in assembler, so possibly this is false assumption.

@bradfitz bradfitz changed the title Invalid golang.org/x/crypto/blake2b.supportSSE4 detection for Atom D2700 x/crypto/blake2b: supportSSE4 detection wrong for Atom D2700 Nov 30, 2016
@bradfitz bradfitz changed the title x/crypto/blake2b: supportSSE4 detection wrong for Atom D2700 x/crypto/blake2b: supportSSE4 detection wrong, SIGILL on Atom D2700 Nov 30, 2016
@bradfitz bradfitz added this to the Unreleased milestone Nov 30, 2016
@bradfitz
Copy link
Contributor

/cc @agl

@agl agl self-assigned this Nov 30, 2016
@agl
Copy link
Contributor

agl commented Dec 1, 2016

Thanks for the report. Does simply changing the 15 to 19 (as in this fix) work for you?

@gopherbot
Copy link

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

@ghost
Copy link
Author

ghost commented Dec 1, 2016

Yes, 19-bit solves the problem (it correctly detects SSS4 absence and does not try to use that optimized instructions).

@ghost
Copy link
Author

ghost commented Dec 1, 2016

Thank you!

@golang golang locked and limited conversation to collaborators Dec 1, 2017
c-expert-zigbee pushed a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 28, 2022
I missed this in the initial review but this will cause blake2b to
SIGILL on, at least, some Atom chips.

Fixes golang/go#18113.

Change-Id: I0c16319df71f3ce4a38411ad4c4e9d3871fd9533
Reviewed-on: https://go-review.googlesource.com/33830
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
c-expert-zigbee added a commit to c-expert-zigbee/crypto_go that referenced this issue Mar 29, 2022
I missed this in the initial review but this will cause blake2b to
SIGILL on, at least, some Atom chips.

Fixes golang/go#18113.

Change-Id: I0c16319df71f3ce4a38411ad4c4e9d3871fd9533
Reviewed-on: https://go-review.googlesource.com/33830
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@rsc rsc unassigned agl Jun 23, 2022
LewiGoddard pushed a commit to LewiGoddard/crypto that referenced this issue Feb 16, 2023
I missed this in the initial review but this will cause blake2b to
SIGILL on, at least, some Atom chips.

Fixes golang/go#18113.

Change-Id: I0c16319df71f3ce4a38411ad4c4e9d3871fd9533
Reviewed-on: https://go-review.googlesource.com/33830
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
BiiChris pushed a commit to BiiChris/crypto that referenced this issue Sep 15, 2023
I missed this in the initial review but this will cause blake2b to
SIGILL on, at least, some Atom chips.

Fixes golang/go#18113.

Change-Id: I0c16319df71f3ce4a38411ad4c4e9d3871fd9533
Reviewed-on: https://go-review.googlesource.com/33830
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants