[4039440478] Upstream Reporter: Paul Holzinger
Upstream issue status: Open
Upstream description:

There is a major flaw in the podman 5.8 boltdb to sqlite database migration logic. As part of podman 5.8 we added the auto migration on boot from boltdb to sqlite that should trigger automatically on the first podman command but it is not concurrent safe against other parallel podman commands.

I have a few containers startup at boot via quadlet, they get started around the the same time by systemd and indeed one unit successfully logged the migration.

Old database has been renamed to /home/podman/.local/share/containers/storage/libpod/bolt_state.db-old and will no longer be used

However the other units started at the same time where all still using botldb and created the containers there. So when I logged onto the system and run podman ps I noticed podman is still using boltdb and one container was missing despite that service being active. That service is the one which trigger the migration as its content was put into the sqlite db.

There are at least two different failure modes I can see here

1. The one I got that we continue using botldb. Looking at the code it seem quite unlikely but clearly can happen. The boltdb file is created by NewBoltState() but if the code does not see a boltdb file it will default to sqlite so naturally since the migration renames the old boltdb file we should expect all new commands to be sqlite but there is a TOCTOU problem here. https://github.com/containers/podman/blob/06ffb9b3972f3b12634460b08a7636245f93c374/libpod/runtime.go#L312-L326 Process A is first and is doing the migration, Process B checks if botldb file exists, it still does, then Process A just finished migration and renames the boltdb file to _old, then Process B calls NewBoltState() and recreates the boltdb file.
2. The other way which I see as more likely is all following commands do use sqlite but the the parallel started commands created their resources in boltdb and thus are invisible. That happens because the db state created early without a lock here: https://github.com/containers/podman/blob/06ffb9b3972f3b12634460b08a7636245f93c374/libpod/runtime.go#L376-L379 Then we take the lock later here: https://github.com/containers/podman/blob/06ffb9b3972f3b12634460b08a7636245f93c374/libpod/runtime.go#L550-L552 And the actual migration happens as part of of runtime.refresh() here: https://github.com/containers/podman/blob/06ffb9b3972f3b12634460b08a7636245f93c374/libpod/runtime.go#L642-L644 The problem is that the first command does the migration but the other ones that where in the middle, i.e. waiting on the alive lock all still have the boltdb state and thus keep using the wrong one as they do not notice the migration from the other command.

I see two ways to fix this:

The first being some lock over the entire makeRuntime() call, the main problem being it cannot be the alive lock as it depends on some fields we read out of the DB. So we need another lock and hold it for much longer so that will negatively impact concurrent podman commands.

The other way I think is we first need to switch getDBState() to check if sqlite file exists and then always use sqlite lite. That should at least ensure that point 1 (like I did) is not getting forced on boltdb after the migration happens. Then in order to prevent the other commands from not noticing a migration happen they must close the db state again under the alive lock and get a new one with getDBState(), as long as that defaults to sqlite if that db file exists it makes them notice there was a migration and then use the right db backend. In that case there is still the TOCTOU race with getDBState() which causes the boltdb file to be created again but since the sqlite will exists as well and we use that to check that should not longer matter and you just end up with an empty boltdb file which will not cause any problems unless I am overlooking something here.