From c7ba5749d44a65fde2daf114c16fb0272d82d73b Mon Sep 17 00:00:00 2001
From: Zac Medico <zmedico@gmail.com>
Date: Fri, 24 Aug 2018 20:21:50 -0700
Subject: [PATCH] Add lock sanity checks to Save() methods

I have experienced "layer not known" corruption triggered by concurrent
buildah/skopeo processes, and hopefully lock sanity checks will help to
prevent this kind of problem.

Signed-off-by: Zac Medico <zmedico@gmail.com>
---
 containers.go    |  7 +++++++
 images.go        |  7 +++++++
 layers.go        |  7 +++++++
 lockfile.go      |  3 +++
 lockfile_unix.go | 12 ++++++++++--
 5 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/containers.go b/containers.go
index ebc1e99a0..f87ea15be 100644
--- a/containers.go
+++ b/containers.go
@@ -192,6 +192,9 @@ func (r *containerStore) Load() error {
 }
 
 func (r *containerStore) Save() error {
+	if !r.Locked() {
+		return errors.New("container store is not locked")
+	}
 	rpath := r.containerspath()
 	if err := os.MkdirAll(filepath.Dir(rpath), 0700); err != nil {
 		return err
@@ -560,3 +563,7 @@ func (r *containerStore) IsReadWrite() bool {
 func (r *containerStore) TouchedSince(when time.Time) bool {
 	return r.lockfile.TouchedSince(when)
 }
+
+func (r *containerStore) Locked() bool {
+	return r.lockfile.Locked()
+}
diff --git a/images.go b/images.go
index 80fae6dce..b10501b08 100644
--- a/images.go
+++ b/images.go
@@ -219,6 +219,9 @@ func (r *imageStore) Save() error {
 	if !r.IsReadWrite() {
 		return errors.Wrapf(ErrStoreIsReadOnly, "not allowed to modify the image store at %q", r.imagespath())
 	}
+	if !r.Locked() {
+		return errors.New("image store is not locked")
+	}
 	rpath := r.imagespath()
 	if err := os.MkdirAll(filepath.Dir(rpath), 0700); err != nil {
 		return err
@@ -701,3 +704,7 @@ func (r *imageStore) IsReadWrite() bool {
 func (r *imageStore) TouchedSince(when time.Time) bool {
 	return r.lockfile.TouchedSince(when)
 }
+
+func (r *imageStore) Locked() bool {
+	return r.lockfile.Locked()
+}
diff --git a/layers.go b/layers.go
index c5f926273..9989bd6be 100644
--- a/layers.go
+++ b/layers.go
@@ -371,6 +371,9 @@ func (r *layerStore) Save() error {
 	if !r.IsReadWrite() {
 		return errors.Wrapf(ErrStoreIsReadOnly, "not allowed to modify the layer store at %q", r.layerspath())
 	}
+	if !r.Locked() {
+		return errors.New("layer store is not locked")
+	}
 	rpath := r.layerspath()
 	if err := os.MkdirAll(filepath.Dir(rpath), 0700); err != nil {
 		return err
@@ -1181,3 +1184,7 @@ func (r *layerStore) IsReadWrite() bool {
 func (r *layerStore) TouchedSince(when time.Time) bool {
 	return r.lockfile.TouchedSince(when)
 }
+
+func (r *layerStore) Locked() bool {
+	return r.lockfile.Locked()
+}
diff --git a/lockfile.go b/lockfile.go
index c1aa482f8..9f6a18144 100644
--- a/lockfile.go
+++ b/lockfile.go
@@ -28,6 +28,9 @@ type Locker interface {
 
 	// IsReadWrite() checks if the lock file is read-write
 	IsReadWrite() bool
+
+	// Locked() checks if lock is locked
+	Locked() bool
 }
 
 var (
diff --git a/lockfile_unix.go b/lockfile_unix.go
index 679259234..31e39c02e 100644
--- a/lockfile_unix.go
+++ b/lockfile_unix.go
@@ -25,9 +25,9 @@ func getLockFile(path string, ro bool) (Locker, error) {
 	}
 	unix.CloseOnExec(fd)
 	if ro {
-		return &lockfile{file: path, fd: uintptr(fd), lw: stringid.GenerateRandomID(), locktype: unix.F_RDLCK}, nil
+		return &lockfile{file: path, fd: uintptr(fd), lw: stringid.GenerateRandomID(), locktype: unix.F_RDLCK, locked: false}, nil
 	}
-	return &lockfile{file: path, fd: uintptr(fd), lw: stringid.GenerateRandomID(), locktype: unix.F_WRLCK}, nil
+	return &lockfile{file: path, fd: uintptr(fd), lw: stringid.GenerateRandomID(), locktype: unix.F_WRLCK, locked: false}, nil
 }
 
 type lockfile struct {
@@ -36,6 +36,7 @@ type lockfile struct {
 	fd       uintptr
 	lw       string
 	locktype int16
+	locked   bool
 }
 
 // Lock locks the lock file
@@ -48,6 +49,7 @@ func (l *lockfile) Lock() {
 		Pid:    int32(os.Getpid()),
 	}
 	l.mu.Lock()
+	l.locked = true
 	for unix.FcntlFlock(l.fd, unix.F_SETLKW, &lk) != nil {
 		time.Sleep(10 * time.Millisecond)
 	}
@@ -65,9 +67,15 @@ func (l *lockfile) Unlock() {
 	for unix.FcntlFlock(l.fd, unix.F_SETLKW, &lk) != nil {
 		time.Sleep(10 * time.Millisecond)
 	}
+	l.locked = false
 	l.mu.Unlock()
 }
 
+// Check if lock is locked
+func (l *lockfile) Locked() bool {
+	return l.locked
+}
+
 // Touch updates the lock file with the UID of the user
 func (l *lockfile) Touch() error {
 	l.lw = stringid.GenerateRandomID()
