Skip to content

Data race in ConnectTo round-robin address selection #752

@queelius

Description

@queelius

Bug Description

The ConnectTo option creates a DialContext closure that reads and writes a round-robin counter (cm.n) without any synchronization:

tr.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
    if cm, ok := connectTo[addr]; ok {
        cm.n = (cm.n + 1) % len(cm.addrs)  // unsynchronized read-modify-write
        addr = cm.addrs[cm.n]
    }
    return dial(ctx, network, addr)
}

DialContext is called concurrently by Go's HTTP transport, so concurrent requests to the same target host will race on cm.n. This can corrupt the counter value and potentially index out of bounds.

Reproduction

Running this with -race reliably triggers the detector:

func TestConnectTo_ConcurrentDialRace(t *testing.T) {
    // Set up multiple backend servers, create attacker with ConnectTo,
    // then fire 20 concurrent hits
    var wg sync.WaitGroup
    for i := 0; i < 20; i++ {
        wg.Add(1)
        go func() {
            defer wg.Done()
            atk.hit(tr, a)
        }()
    }
    wg.Wait()
}

Output:

WARNING: DATA RACE
Read at 0x00c00007e0b8 by goroutine 34:
  ...ConnectTo.func4.1()
      vegeta/lib/attack.go:309
Previous write at 0x00c00007e0b8 by goroutine 33:
  ...ConnectTo.func4.1()
      vegeta/lib/attack.go:309

Fix

Add a sync.Mutex per round-robin group and lock around the counter update.

PR forthcoming.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions