search

containerd / nerdctl

contaiNERD CTL - Docker-compatible CLI for containerd, with support for Compose, Rootless, eStargz, OCIcrypt, IPFS, ...
Apache License 2.0
7.63k stars 565 forks source link

ZstdLayerConvertFunc unsafe to use concurrently #3078

Closed apostasie closed 3 weeks ago

apostasie commented 3 weeks ago

Description

convert zstd is not safe to use concurrently, as there is no guarantee that the temporary uncompressed resource is still there by the time we read it.

https://github.com/containerd/nerdctl/blob/main/pkg/imgutil/converter/zstd.go#L54

Furthermore, using uncompress.LayerConvertFunc seems to create useless churn (and is at the root of the problem), as we will read and write twice instead of once.

From a quick read, it seems to me that we should instead just avoid the containerd Uncompress helper altogether and just get a DecompressStream directly and roll with it:


    readerAt, err := cs.ReaderAt(ctx, desc)
    sr := io.NewSectionReader(readerAt, 0, desc.Size)
    newR, err := compression.DecompressStream(sr)

Thoughts?

Steps to reproduce the issue

Use convert zstd concurrently.

Describe the results you received and expected

Fail because the temporary uncompressed layer has already been deleted by the concurrent execution.

Should not fail.

What version of nerdctl are you using?

1.7.6

Are you using a variant of nerdctl? (e.g., Rancher Desktop)

None

Host information

No response