fix: resolve race conditions, add bounds checks, and remove redundant code

This commit is contained in:
wnqqnw19 2025-12-04 12:27:35 -08:00
parent d8419ffca2
commit 8bd797b6c6
2 changed files with 28 additions and 7 deletions

View file

@ -316,6 +316,11 @@ func (h *handler) runEthPeer(peer *eth.Peer, handler eth.Handler) error {
defer close(dead) defer close(dead)
// If we have any explicit peer required block hashes, request them // If we have any explicit peer required block hashes, request them
// Capture peer ID and logger to avoid race conditions in goroutines
peerID := peer.ID()
peerLogger := peer.Log()
peerAddr := peer.RemoteAddr()
peerName := peer.Name()
for number, hash := range h.requiredBlocks { for number, hash := range h.requiredBlocks {
resCh := make(chan *eth.Response) resCh := make(chan *eth.Response)
@ -323,7 +328,7 @@ func (h *handler) runEthPeer(peer *eth.Peer, handler eth.Handler) error {
if err != nil { if err != nil {
return err return err
} }
go func(number uint64, hash common.Hash, req *eth.Request) { go func(number uint64, hash common.Hash, req *eth.Request, id string, logger log.Logger, addr string, name string) {
// Ensure the request gets cancelled in case of error/drop // Ensure the request gets cancelled in case of error/drop
defer req.Close() defer req.Close()
@ -345,19 +350,19 @@ func (h *handler) runEthPeer(peer *eth.Peer, handler eth.Handler) error {
return return
} }
if headers[0].Number.Uint64() != number || headers[0].Hash() != hash { if headers[0].Number.Uint64() != number || headers[0].Hash() != hash {
peer.Log().Info("Required block mismatch, dropping peer", "number", number, "hash", headers[0].Hash(), "want", hash) logger.Info("Required block mismatch, dropping peer", "number", number, "hash", headers[0].Hash(), "want", hash)
res.Done <- errors.New("required block mismatch") res.Done <- errors.New("required block mismatch")
return return
} }
peer.Log().Debug("Peer required block verified", "number", number, "hash", hash) logger.Debug("Peer required block verified", "number", number, "hash", hash)
res.Done <- nil res.Done <- nil
case <-timeout.C: case <-timeout.C:
peer.Log().Warn("Required block challenge timed out, dropping", "addr", peer.RemoteAddr(), "type", peer.Name()) logger.Warn("Required block challenge timed out, dropping", "addr", addr, "type", name)
h.removePeer(peer.ID()) h.removePeer(id)
case <-dead: case <-dead:
// Peer handler terminated, abort all goroutines // Peer handler terminated, abort all goroutines
} }
}(number, hash, req) }(number, hash, req, peerID, peerLogger, peerAddr, peerName)
} }
// Handle incoming messages until the connection is torn down // Handle incoming messages until the connection is torn down
return handler(peer) return handler(peer)

View file

@ -52,10 +52,26 @@ func (t *stateTracker) releaseState(number uint64, release StateReleaseFunc) {
t.lock.Lock() t.lock.Lock()
defer t.lock.Unlock() defer t.lock.Unlock()
// Validate that the state number is within the expected range
if number < t.oldest {
// This should not happen in normal operation, but handle gracefully
// by just appending the release function without updating the used array
t.releases = append(t.releases, release)
return
}
// Calculate the index and ensure it's within bounds
index := int(number - t.oldest)
if index < 0 || index >= len(t.used) {
// State is outside the tracking window, just append the release function
t.releases = append(t.releases, release)
return
}
// Set the state as used, the corresponding flag is indexed by // Set the state as used, the corresponding flag is indexed by
// the distance between the specified state and the oldest state // the distance between the specified state and the oldest state
// which is still using for trace. // which is still using for trace.
t.used[int(number-t.oldest)] = true t.used[index] = true
// If the oldest state is used up, update the oldest marker by moving // If the oldest state is used up, update the oldest marker by moving
// it to the next state which is not used up. // it to the next state which is not used up.