Skip to content

crypto: fix infinite recursion in Secp256k1 string formatting (#5707)#5709

Merged
erikgrinaker merged 1 commit intorc0/v0.34.1from
erik/backport-5707
Nov 24, 2020
Merged

crypto: fix infinite recursion in Secp256k1 string formatting (#5707)#5709
erikgrinaker merged 1 commit intorc0/v0.34.1from
erik/backport-5707

Conversation

@erikgrinaker
Copy link
Contributor

@erikgrinaker erikgrinaker commented Nov 24, 2020

This caused stack overflow panics in E2E tests, e.g.:

2020-11-24T02:37:17.6085640Z �[35mvalidator04    |�[0m runtime: goroutine stack exceeds 1000000000-byte limit
2020-11-24T02:37:17.6087818Z �[35mvalidator04    |�[0m runtime: sp=0xc0234b23c0 stack=[0xc0234b2000, 0xc0434b2000]
2020-11-24T02:37:17.6088920Z �[35mvalidator04    |�[0m fatal error: stack overflow
2020-11-24T02:37:17.6089776Z �[35mvalidator04    |�[0m
2020-11-24T02:37:17.6090569Z �[35mvalidator04    |�[0m runtime stack:
2020-11-24T02:37:17.6091677Z �[35mvalidator04    |�[0m runtime.throw(0x12dc476, 0xe)
2020-11-24T02:37:17.6093123Z �[35mvalidator04    |�[0m 	/usr/local/go/src/runtime/panic.go:1116 +0x72
2020-11-24T02:37:17.6094320Z �[35mvalidator04    |�[0m runtime.newstack()
2020-11-24T02:37:17.6095374Z �[35mvalidator04    |�[0m 	/usr/local/go/src/runtime/stack.go:1067 +0x78d
2020-11-24T02:37:17.6096381Z �[35mvalidator04    |�[0m runtime.morestack()
2020-11-24T02:37:17.6097657Z �[35mvalidator04    |�[0m 	/usr/local/go/src/runtime/asm_amd64.s:449 +0x8f
2020-11-24T02:37:17.6098505Z �[35mvalidator04    |�[0m
2020-11-24T02:37:17.6099328Z �[35mvalidator04    |�[0m goroutine 88 [running]:
2020-11-24T02:37:17.6100470Z �[35mvalidator04    |�[0m runtime.heapBitsSetType(0xc009565380, 0x20, 0x18, 0x1137e00)
2020-11-24T02:37:17.6101961Z �[35mvalidator04    |�[0m 	/usr/local/go/src/runtime/mbitmap.go:911 +0xaa5 fp=0xc0234b23d0 sp=0xc0234b23c8 pc=0x432625
2020-11-24T02:37:17.6103906Z �[35mvalidator04    |�[0m runtime.mallocgc(0x20, 0x1137e00, 0x117b601, 0x11e9240)
2020-11-24T02:37:17.6105179Z �[35mvalidator04    |�[0m 	/usr/local/go/src/runtime/malloc.go:1090 +0x5a5 fp=0xc0234b2470 sp=0xc0234b23d0 pc=0x428b25
2020-11-24T02:37:17.6106540Z �[35mvalidator04    |�[0m runtime.convTslice(0xc002743710, 0x21, 0x21, 0xc0234b24e8)
2020-11-24T02:37:17.6107861Z �[35mvalidator04    |�[0m 	/usr/local/go/src/runtime/iface.go:385 +0x59 fp=0xc0234b24a0 sp=0xc0234b2470 pc=0x426379
2020-11-24T02:37:17.6109315Z �[35mvalidator04    |�[0m github.com/tendermint/tendermint/crypto/secp256k1.PubKey.String(...)
2020-11-24T02:37:17.6151692Z �[35mvalidator04    |�[0m 	/src/tendermint/crypto/secp256k1/secp256k1.go:161
2020-11-24T02:37:17.6153872Z �[35mvalidator04    |�[0m github.com/tendermint/tendermint/crypto/secp256k1.(*PubKey).String(0xc009565360, 0x11e9240, 0xc009565360)
2020-11-24T02:37:17.6157421Z �[35mvalidator04    |�[0m 	<autogenerated>:1 +0x65 fp=0xc0234b24f8 sp=0xc0234b24a0 pc=0x656965
2020-11-24T02:37:17.6159134Z �[35mvalidator04    |�[0m fmt.(*pp).handleMethods(0xc00956c680, 0x58, 0xc0234b2801)
2020-11-24T02:37:17.6161462Z �[35mvalidator04    |�[0m 	/usr/local/go/src/fmt/print.go:630 +0x30a fp=0xc0234b2768 sp=0xc0234b24f8 pc=0x518b8a
[...]
2020-11-24T02:37:17.6649685Z �[35mvalidator04    |�[0m 	/usr/local/go/src/fmt/print.go:630 +0x30a fp=0xc0234b7f48 sp=0xc0234b7cd8 pc=0x518b8a
2020-11-24T02:37:17.6651177Z �[35mvalidator04    |�[0m created by github.com/tendermint/tendermint/node.startStateSync
2020-11-24T02:37:17.6652521Z �[35mvalidator04    |�[0m 	/src/tendermint/node/node.go:587 +0x150

Description

Please add a description of the changes that this PR introduces and the files that
are the most critical to review.

Closes: #XXX

@erikgrinaker
crypto: fix infinite recursion in Secp256k1 string formatting (#5707)
dde657f 
This caused stack overflow panics in E2E tests, e.g.:

```
2020-11-24T02:37:17.6085640Z �[35mvalidator04    |�[0m runtime: goroutine stack exceeds 1000000000-byte limit
2020-11-24T02:37:17.6087818Z �[35mvalidator04    |�[0m runtime: sp=0xc0234b23c0 stack=[0xc0234b2000, 0xc0434b2000]
2020-11-24T02:37:17.6088920Z �[35mvalidator04    |�[0m fatal error: stack overflow
2020-11-24T02:37:17.6089776Z �[35mvalidator04    |�[0m
2020-11-24T02:37:17.6090569Z �[35mvalidator04    |�[0m runtime stack:
2020-11-24T02:37:17.6091677Z �[35mvalidator04    |�[0m runtime.throw(0x12dc476, 0xe)
2020-11-24T02:37:17.6093123Z �[35mvalidator04    |�[0m 	/usr/local/go/src/runtime/panic.go:1116 +0x72
2020-11-24T02:37:17.6094320Z �[35mvalidator04    |�[0m runtime.newstack()
2020-11-24T02:37:17.6095374Z �[35mvalidator04    |�[0m 	/usr/local/go/src/runtime/stack.go:1067 +0x78d
2020-11-24T02:37:17.6096381Z �[35mvalidator04    |�[0m runtime.morestack()
2020-11-24T02:37:17.6097657Z �[35mvalidator04    |�[0m 	/usr/local/go/src/runtime/asm_amd64.s:449 +0x8f
2020-11-24T02:37:17.6098505Z �[35mvalidator04    |�[0m
2020-11-24T02:37:17.6099328Z �[35mvalidator04    |�[0m goroutine 88 [running]:
2020-11-24T02:37:17.6100470Z �[35mvalidator04    |�[0m runtime.heapBitsSetType(0xc009565380, 0x20, 0x18, 0x1137e00)
2020-11-24T02:37:17.6101961Z �[35mvalidator04    |�[0m 	/usr/local/go/src/runtime/mbitmap.go:911 +0xaa5 fp=0xc0234b23d0 sp=0xc0234b23c8 pc=0x432625
2020-11-24T02:37:17.6103906Z �[35mvalidator04    |�[0m runtime.mallocgc(0x20, 0x1137e00, 0x117b601, 0x11e9240)
2020-11-24T02:37:17.6105179Z �[35mvalidator04    |�[0m 	/usr/local/go/src/runtime/malloc.go:1090 +0x5a5 fp=0xc0234b2470 sp=0xc0234b23d0 pc=0x428b25
2020-11-24T02:37:17.6106540Z �[35mvalidator04    |�[0m runtime.convTslice(0xc002743710, 0x21, 0x21, 0xc0234b24e8)
2020-11-24T02:37:17.6107861Z �[35mvalidator04    |�[0m 	/usr/local/go/src/runtime/iface.go:385 +0x59 fp=0xc0234b24a0 sp=0xc0234b2470 pc=0x426379
2020-11-24T02:37:17.6109315Z �[35mvalidator04    |�[0m github.com/tendermint/tendermint/crypto/secp256k1.PubKey.String(...)
2020-11-24T02:37:17.6151692Z �[35mvalidator04    |�[0m 	/src/tendermint/crypto/secp256k1/secp256k1.go:161
2020-11-24T02:37:17.6153872Z �[35mvalidator04    |�[0m github.com/tendermint/tendermint/crypto/secp256k1.(*PubKey).String(0xc009565360, 0x11e9240, 0xc009565360)
2020-11-24T02:37:17.6157421Z �[35mvalidator04    |�[0m 	<autogenerated>:1 +0x65 fp=0xc0234b24f8 sp=0xc0234b24a0 pc=0x656965
2020-11-24T02:37:17.6159134Z �[35mvalidator04    |�[0m fmt.(*pp).handleMethods(0xc00956c680, 0x58, 0xc0234b2801)
2020-11-24T02:37:17.6161462Z �[35mvalidator04    |�[0m 	/usr/local/go/src/fmt/print.go:630 +0x30a fp=0xc0234b2768 sp=0xc0234b24f8 pc=0x518b8a
[...]
2020-11-24T02:37:17.6649685Z �[35mvalidator04    |�[0m 	/usr/local/go/src/fmt/print.go:630 +0x30a fp=0xc0234b7f48 sp=0xc0234b7cd8 pc=0x518b8a
2020-11-24T02:37:17.6651177Z �[35mvalidator04    |�[0m created by github.com/tendermint/tendermint/node.startStateSync
2020-11-24T02:37:17.6652521Z �[35mvalidator04    |�[0m 	/src/tendermint/node/node.go:587 +0x150

```
@erikgrinaker erikgrinaker added S:automerge Automatically merge PR when requirements pass S:backport labels Nov 24, 2020
@erikgrinaker erikgrinaker self-assigned this Nov 24, 2020
@codecov
Copy link

codecov bot commented Nov 24, 2020

Codecov Report

❗ No coverage uploaded for pull request base (rc0/v0.34.1@182fa32). Click here to learn what that means.
The diff coverage is n/a.

@@              Coverage Diff               @@
##             rc0/v0.34.1    #5709   +/-   ##
==============================================
  Coverage               ?   60.50%           
==============================================
  Files                  ?      263           
  Lines                  ?    23816           
  Branches               ?        0           
==============================================
  Hits                   ?    14411           
  Misses                 ?     7922           
  Partials               ?     1483

@erikgrinaker erikgrinaker merged commit 15b7037 into rc0/v0.34.1 Nov 24, 2020
@erikgrinaker erikgrinaker deleted the erik/backport-5707 branch November 24, 2020 13:01
@cmwaters cmwaters mentioned this pull request Feb 22, 2022
Merged
This was referenced Apr 22, 2022
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ebuchman ebuchman Awaiting requested review from ebuchman

@tac0turtle tac0turtle Awaiting requested review from tac0turtle

@tessr tessr Awaiting requested review from tessr

1 more reviewer

@melekes melekes melekes approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@erikgrinaker erikgrinaker

Labels

S:automerge Automatically merge PR when requirements pass

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@erikgrinaker @melekes