containers / ocicrypt

Encryption libraries for Encrypted OCI Container images
Apache License 2.0
149 stars 33 forks source link

Fix a possible race with concurrent encryption with the same config #93

Closed mtrmac closed 1 year ago

mtrmac commented 1 year ago

append() can allocate arrays with cap(s) > len(s), and future append() calls would then just write to the free slots; doing that from multiple goroutines would race.

Fixes #92, which contains a demonstration.

(I didn’t test this in practice.)

mtrmac commented 1 year ago

A full self-contained demonstration:

package main

import (
    "fmt"
    "sync"
    "time"
)

func goroutine(wg *sync.WaitGroup, sharedSlice []int, i int) {
    defer wg.Done()

    privateSlice := append(sharedSlice, i)
    oldValue := privateSlice[len(privateSlice)-1]
    // This is already slow enough that the "full" value is frequently overwritten by another goroutine
    fmt.Printf("goroutine %d: immediate %d; full %#v@%v\n", i, oldValue, privateSlice, cap(privateSlice))

    time.Sleep(100 * time.Millisecond) // Give ample time for other goroutines to run

    newValue := privateSlice[len(privateSlice)-1]
    if newValue != i {
        panic(fmt.Sprintf("goroutine %d: immediate %d, after sleep %d; full %#v@%v", i, oldValue, newValue, privateSlice, cap(privateSlice)))
    }
}

func main() {
    sharedSlice := []int{0, 0, 0, 0}
    // Creates a backing array with extra capacity. (Alternatively, this could use the 3-argument version of make(),
    // but this demonstrates that this happens completely naturally just using append().)
    sharedSlice = append(sharedSlice, 0, 0)
    fmt.Printf("shared: %#v@%d\n", sharedSlice, cap(sharedSlice))

    wg := sync.WaitGroup{}
    for i := 0; i < 10; i++ {
        wg.Add(1)
        go goroutine(&wg, sharedSlice, i)
    }
    wg.Wait()
}

One output:

shared: []int{0, 0, 0, 0, 0, 0}@8
goroutine 9: immediate 9; full []int{0, 0, 0, 0, 0, 0, 4}@8
goroutine 7: immediate 7; full []int{0, 0, 0, 0, 0, 0, 7}@8
goroutine 8: immediate 8; full []int{0, 0, 0, 0, 0, 0, 8}@8
goroutine 4: immediate 4; full []int{0, 0, 0, 0, 0, 0, 2}@8
goroutine 2: immediate 2; full []int{0, 0, 0, 0, 0, 0, 2}@8
goroutine 5: immediate 5; full []int{0, 0, 0, 0, 0, 0, 5}@8
goroutine 3: immediate 3; full []int{0, 0, 0, 0, 0, 0, 3}@8
goroutine 0: immediate 0; full []int{0, 0, 0, 0, 0, 0, 0}@8
goroutine 6: immediate 6; full []int{0, 0, 0, 0, 0, 0, 0}@8
goroutine 1: immediate 1; full []int{0, 0, 0, 0, 0, 0, 1}@8
panic: goroutine 4: immediate 4, after sleep 1; full []int{0, 0, 0, 0, 0, 0, 1}@8
…
stefanberger commented 1 year ago

Thanks. Would have never found it.