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

Nil pointer dereference in beacon transaction during early EstimateGas #3669

Closed
kostko opened this issue Feb 2, 2021 · 2 comments · Fixed by #3672
Closed

Nil pointer dereference in beacon transaction during early EstimateGas #3669

kostko opened this issue Feb 2, 2021 · 2 comments · Fixed by #3672
Assignees
@Yawning
Labels
c:beacon Category: random beacon service c:bug Category: bug

Comments

@kostko
Copy link
Member

kostko commented Feb 2, 2021 •
edited

There is a possible nil pointer dereference in the beacon transaction processing when EstimateGas is called before the beacon backend is initialized.

Backtrace of this happening, discovered by daily long-term tests after forcibly restarting a validator node:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x113c06b]

goroutine 153 [running]:
github.com/oasisprotocol/oasis-core/go/consensus/tendermint/apps/beacon.(*beaconApplication).ExecuteTx(0xc0001ff8c0, 0xc00f042b40, 0xc00f69d040, 0xc00f69d1c0, 0x4)
	github.com/oasisprotocol/oasis-core/go/consensus/tendermint/apps/beacon/beacon.go:88 +0x1eb
github.com/oasisprotocol/oasis-core/go/consensus/tendermint/abci.(*abciMux).processTx(0xc001332000, 0xc00f042b40, 0xc00f69d040, 0x537, 0x537, 0xc00f042b40)
	github.com/oasisprotocol/oasis-core/go/consensus/tendermint/abci/mux.go:553 +0x282
github.com/oasisprotocol/oasis-core/go/consensus/tendermint/abci.(*abciMux).EstimateGas(0xc001332000, 0xbcabe806d09f48cb, 0x375136c35cd4c96b, 0xb1dcf0f3433028f2, 0x8c1cce7d0180581e, 0xc00f69d040, 0x0, 0x0, 0x0)
	github.com/oasisprotocol/oasis-core/go/consensus/tendermint/abci/mux.go:603 +0x206
github.com/oasisprotocol/oasis-core/go/consensus/tendermint/abci.(*ApplicationServer).EstimateGas(...)
	github.com/oasisprotocol/oasis-core/go/consensus/tendermint/abci/mux.go:177
github.com/oasisprotocol/oasis-core/go/consensus/tendermint/full.(*fullService).EstimateGas(0xc00136fa00, 0x201ee20, 0xc00f69d140, 0xc00f6ed950, 0x8c1cce7d0180581e, 0xe372c053df, 0xc00f042a50)
	github.com/oasisprotocol/oasis-core/go/consensus/tendermint/full/full.go:526 +0x6c
github.com/oasisprotocol/oasis-core/go/consensus/api.(*submissionManager).EstimateGasAndSetFee(0xc0001975e0, 0x201ee20, 0xc00f69d140, 0x201f7a0, 0xc0013e67c0, 0xc00f69d040, 0x0, 0x76692c1fcfb8c886)
	github.com/oasisprotocol/oasis-core/go/consensus/api/submission.go:86 +0xed
github.com/oasisprotocol/oasis-core/go/consensus/api.(*submissionManager).signAndSubmitTx(0xc0001975e0, 0x201ee20, 0xc00f69d140, 0x201f7a0, 0xc0013e67c0, 0xc00f69d040, 0xbcbe7c, 0xc00f6cbf80)
	github.com/oasisprotocol/oasis-core/go/consensus/api/submission.go:136 +0x388
github.com/oasisprotocol/oasis-core/go/consensus/api.(*submissionManager).SignAndSubmitTx.func1(0xc01008d720, 0xc01008d720)
	github.com/oasisprotocol/oasis-core/go/consensus/api/submission.go:171 +0x57`
github.com/cenkalti/backoff/v4.RetryNotifyWithTimer(0xc00fa0bb28, 0x2003ae0, 0xc01008d720, 0x0, 0x200e620, 0xc00056ec88, 0x0, 0x0)
	github.com/cenkalti/backoff/v4@v4.1.0/retry.go:55 +0x102
github.com/cenkalti/backoff/v4.RetryNotify(...)
	github.com/cenkalti/backoff/v4@v4.1.0/retry.go:34
github.com/cenkalti/backoff/v4.Retry(...)
	github.com/cenkalti/backoff/v4@v4.1.0/retry.go:28
github.com/oasisprotocol/oasis-core/go/consensus/api.(*submissionManager).SignAndSubmitTx(0xc0001975e0, 0x201ee20, 0xc00f69d140, 0x201f7a0, 0xc0013e67c0, 0xc00f69d040, 0x0, 0x0)
	github.com/oasisprotocol/oasis-core/go/consensus/api/submission.go:170 +0x185
github.com/oasisprotocol/oasis-core/go/consensus/api.SignAndSubmitTx(0x201ee20, 0xc00f69d140, 0x2045e00, 0xc00136fa00, 0x201f7a0, 0xc0013e67c0, 0xc00f69d040, 0x28, 0x7fdde1af72f0)
	github.com/oasisprotocol/oasis-core/go/consensus/api/submission.go:196 +0x82
github.com/oasisprotocol/oasis-core/go/worker/beacon.(*Worker).retrySubmitTx.func1(0x1b05080, 0xc00f6ecba0)
	github.com/oasisprotocol/oasis-core/go/worker/beacon/worker.go:599 +0x214
github.com/oasisprotocol/oasis-core/go/worker/beacon.(*Worker).retrySubmitTx(0xc010c7e000, 0xc00f69d040)
	github.com/oasisprotocol/oasis-core/go/worker/beacon/worker.go:610 +0x134
github.com/oasisprotocol/oasis-core/go/worker/beacon.(*Worker).recoverPersistedState(0xc010c7e000)
	github.com/oasisprotocol/oasis-core/go/worker/beacon/worker.go:502 +0x7aa
github.com/oasisprotocol/oasis-core/go/worker/beacon.(*Worker).worker(0xc010c7e000)
	github.com/oasisprotocol/oasis-core/go/worker/beacon/worker.go:102 +0x13b
created by github.com/oasisprotocol/oasis-core/go/worker/beacon.(*Worker).Start
	github.com/oasisprotocol/oasis-core/go/worker/beacon/worker.go:61 +0x5a
@kostko kostko added c:bug Category: bug c:beacon Category: random beacon service labels Feb 2, 2021
@kostko kostko mentioned this issue Feb 2, 2021
Closed
1 task
@Yawning
Copy link
Contributor

Yawning commented Feb 2, 2021

Wut. It will initialize itself in BeginBlock as well. Why isn't that sufficient?

@kostko
Copy link
Member Author

kostko commented Feb 2, 2021

Oh I missed the part in BeginBlock, in that case the problem is that an EstimateGas request can come in before any block has been processed.

@kostko kostko changed the title Beacon backend only initialized in InitChain Nil pointer dereference in beacon transaction during early EstimateGas Feb 2, 2021
@kostko kostko linked a pull request Feb 2, 2021 that will close this issue
go/consensus/tendermint/abci/mux: Reject early EstimateGas calls #3672
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Yawning Yawning

Labels
c:beacon Category: random beacon service c:bug Category: bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

go/consensus/tendermint/abci/mux: Reject early EstimateGas calls oasisprotocol/oasis-core
2 participants
@kostko @Yawning