Hacktoberfest 2026: los issues que los mantenedores marcaron para octubre, abiertos y aptos para principiantes. Explorar issues de Hacktoberfest

bufio.CachedConn: data race on the cached buffer between Read/ReaderReplaceable and Close

Abierto
  • ¿Algo no está bien?
#112 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
55/100
Tipo de issue
Error
Claridad
Bastante claro
Estado de actividad
Activo
Stack tecnológico
go
Área
networking

Línea de trabajo

Start in common/bufio/cache.go with CachedConn.Read, ReaderReplaceable, Close, and the corresponding CachedPacketConn methods. Reproduce the reported concurrent paths with the race detector, including the routing and DNS-over-TCP entry points named in the issue. Done means the cache has one consistent ownership path, without races or double release.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

sing v0.8.10 (via sing-box v1.13.12), go 1.26, -race

CachedConn.buffer is read and cleared without synchronization in Read and ReaderReplaceable, while Close (usually called from another goroutine) reads and clears it too. Close guards itself with taken, but Read / ReaderReplaceable don't check or set taken:

// common/bufio/cache.go
func (c *CachedConn) Read(p []byte) (n int, err error) {
	if c.buffer != nil {              // read
		...
		c.buffer.DecRef()
		c.buffer.Release()
		c.buffer = nil                // write
	}
	return c.Conn.Read(p)
}

func (c *CachedConn) ReaderReplaceable() bool {
	return c.buffer == nil            // read
}

func (c *CachedConn) Close() error {
	if c.taken.CompareAndSwap(false, true) {
		if buffer := c.buffer; buffer != nil {   // read (races with Read)
			c.buffer = nil
			buffer.DecRef()
			buffer.Release()          // can release a buffer Read already released
		}
	}
	return c.Conn.Close()
}

CachedPacketConn.ReadPacket / Close have the same pattern.

Besides the race report, the worst case is the pooled buffer being released twice, so it can come back from buf.New to another connection while still referenced.

Where it shows up in sing-box 1.13.12 (both found with -race):

  1. Routing a sniffed TCP connection: route.(*Router).routeConnection wraps it in bufio.NewCachedConn; ConnectionManager.connectionCopy then calls ReaderReplaceable (via N.UnwrapCountReader in bufio.CopyWithCounters) on one goroutine, while the other copy direction calls Close (route/conn.go:262 vs :271).
  2. Hijacked DNS over TCP: protocol/dns.HandleStreamDNSRequest reads the next query from the conn (handle.go:24) while the exchange goroutine of the previous query calls conn.Close() on error (handle.go:46).

When the wrapper can be unwrapped before copying, ReadCached (which does use taken) avoids it, but callers such as a ConnectionTracker that wraps the conn force the unsynchronized Read path.

Suggested fix: make every consumer of the cache take it through the same atomic flag, e.g.

func (c *CachedConn) Read(p []byte) (n int, err error) {
	if !c.taken.Load() {
		c.mu.Lock() // or take the buffer once with CompareAndSwap and keep it reader-local
		...
	}
	return c.Conn.Read(p)
}

or have Read take the buffer once (CompareAndSwap(false, true)), move it into a reader-owned field, and never touch it from Close after that. ReaderReplaceable would then read an atomic. Happy to send a PR if the approach sounds right.

Lenguaje dominante
Go
Estrellas
131
Forks
116
Merge medio
1 h 42 min
PR fusionados (30 d)
1

Preparar el entorno

Este proyecto no incluye contenedor de desarrollo, Dockerfile ni guía de contribución, así que la configuración corre por tu cuenta: empieza por su README y consulta nuestra guía para la primera contribución para los pasos generales.

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de SagerNet/sing

Todos los issues de SagerNet/sing

Issues similares

Más issues de Go

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.