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

Fix a race around SQLite DB config validation #17902

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 26 additions & 15 deletions libpod/sqlite_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,31 +333,38 @@ func (s *SQLiteState) ValidateDBConfig(runtime *Runtime) (defErr error) {
runtimeGraphDriver = storeOpts.GraphDriverName
}

row := s.conn.QueryRow("SELECT Os, StaticDir, TmpDir, GraphRoot, RunRoot, GraphDriver, VolumeDir FROM DBConfig;")
// We have to do this in a transaction to ensure mutual exclusion.
// Otherwise we have a race - multiple processes can be checking the
// row's existence simultaneously, both try to create it, second one to
// get the transaction lock gets an error.
// TODO: The transaction isn't strictly necessary, and there's a (small)
// chance it's a perf hit. If it is, we can move it entirely within the
// `errors.Is()` block below, with extra validation to ensure the row
// still does not exist (and, if it does, to retry this function).
tx, err := s.conn.Begin()
if err != nil {
return fmt.Errorf("beginning database validation transaction: %w", err)
}
defer func() {
if defErr != nil {
if err := tx.Rollback(); err != nil {
logrus.Errorf("Rolling back transaction to validate database: %v", err)
}
}
}()

row := tx.QueryRow("SELECT Os, StaticDir, TmpDir, GraphRoot, RunRoot, GraphDriver, VolumeDir FROM DBConfig;")

if err := row.Scan(&os, &staticDir, &tmpDir, &graphRoot, &runRoot, &graphDriver, &volumePath); err != nil {
if errors.Is(err, sql.ErrNoRows) {
// Need to create runtime config info in DB
tx, err := s.conn.Begin()
if err != nil {
return fmt.Errorf("beginning DB config transaction: %w", err)
}
defer func() {
if defErr != nil {
if err := tx.Rollback(); err != nil {
logrus.Errorf("Rolling back transaction to create DB config: %v", err)
}
}
}()

if _, err := tx.Exec(createRow, 1, schemaVersion, runtimeOS,
runtimeStaticDir, runtimeTmpDir, runtimeGraphRoot,
runtimeRunRoot, runtimeGraphDriver, runtimeVolumePath); err != nil {
return fmt.Errorf("adding DB config row: %w", err)
}

if err := tx.Commit(); err != nil {
return fmt.Errorf("committing DB config transaction: %w", err)
return fmt.Errorf("committing write of database validation row: %w", err)
}

return nil
Expand All @@ -366,6 +373,10 @@ func (s *SQLiteState) ValidateDBConfig(runtime *Runtime) (defErr error) {
return fmt.Errorf("retrieving DB config: %w", err)
}

if err := tx.Commit(); err != nil {
vrothberg marked this conversation as resolved.
Show resolved Hide resolved
return fmt.Errorf("committing database validation row: %w", err)
}

checkField := func(fieldName, dbVal, ourVal string) error {
if dbVal != ourVal {
return fmt.Errorf("database %s %q does not match our %s %q: %w", fieldName, dbVal, fieldName, ourVal, define.ErrDBBadConfig)
Expand Down