Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

usedVolumes (called in volume remove and prune) can be raced by container creation, and other sources of racyness in volumes #3132

Closed
apostasie opened this issue Jun 20, 2024 · 0 comments · Fixed by #3133
Labels
bug Something isn't working

Comments

@apostasie
Copy link
Contributor

Description

Volume Store does have a lock mechanism preventing race conditions between volume creation and volume removal / pruning.

However, calls to usedVolumes, which will decide which volumes are safe to remove, are not protected.
From a casual reading of the source, it seems to me that container creation or removal could race a call to usedVolumes, resulting in removal or pruning to either fail to remove volumes that should have been (this case is ok IMHO), but also possibly removing volumes that should not be as they are NOW in use (this one is probably really bad).

Solution could be that we enforce the volume lock mechanism so that container creation has to get an exclusive lock, and then we also put usedVolumes inside the locked function.

That would definitely solve the bad scenario:

  • usedVolume check -> container is created with volume -> volume destroy

Trying to lock as well on container remove or prune is unlikely to bring joy - but again, in that case, it is not too bad (we just fail to remove stuff that could have been).

Furthermore, other calls to volStore.Get with the intent of creating volumes dependent on results are also susceptible to the same problem.

Since we now changed the behavior of Create to align with docker, I will fix the simplest / most egregious cases ("get" then "create") and overall revamp Get which is not safe.

But for the rest (locking volStore on container create):
Is this reading overall accurate?
Thoughts?

Steps to reproduce the issue

na

Describe the results you received and expected

na

What version of nerdctl are you using?

main

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

None

Host information

No response

@apostasie apostasie added the kind/unconfirmed-bug-claim Unconfirmed bug claim label Jun 20, 2024
@AkihiroSuda AkihiroSuda added bug Something isn't working and removed kind/unconfirmed-bug-claim Unconfirmed bug claim labels Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants