Race condition in applyDiffWithOptions

[mtrmac]

I’m trying to fix storage tests in c/image after 3 years they basically didn’t work at all (containers/image#1729 ), and I’m running into a race condition that reproduces 100% for me.

• storage/layers.go

  Line 2264 in e21971a

  defer uncompressed.Close()

  triggers an uncompressed.Close() (and a buffer deallocation/cleanup) when applyDiffWithOptions returns
• That happens after

  storage/layers.go

  Line 2289 in e21971a

  size, err := r.driver.ApplyDiff(layer.ID, layer.Parent, options)

  returns, essentially after payload is read up to a a tar EOF marker.
• But payload is a read end of a pipe, and the producer is concurrent! The producer has read an EOF marker, correctly propagated it to the consumer, and now the producer may be, in particular, at https://github.com/vbatts/tar-split/blob/fe4605ae8b2a5c0fed8f7852dd460879c03e6943/tar/asm/disassemble.go#L130 . And if uncompressed was already deallocated, this crashes.

Arguably the interface and implementation of NewInputTarStream just don’t match.

Alternatively, c/storage could probably deal with this itself by ensuring uncompressed is not really accessed after the .Close(), using something like https://github.com/containers/image/blob/main/internal/uploadreader/upload_reader.go (which exists to work around a similar problem in net/http).

Cc: @vbatts