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 (https://github.com/containers/image/issues/1729 ), and I’m running into a race condition that reproduces 100% for me.

• https://github.com/containers/storage/blob/e21971a94abbc96f51e79c439becc3cb967fc86b/layers.go#L2264 triggers an uncompressed.Close() (and a buffer deallocation/cleanup) when applyDiffWithOptions returns
• That happens after https://github.com/containers/storage/blob/e21971a94abbc96f51e79c439becc3cb967fc86b/layers.go#L2289 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