Skip to content

Commit

Permalink
Don't blindly reuse state from a previous layer when re-creating it
Browse files Browse the repository at this point in the history
We have reports in the wild of a layer store where two symbolic links
in linkDir point to the same layer. That could only happen when calling
Driver.create with a previously-used layer ID (which happens all the time
because pulls use deterministic layer IDs), without fully deleting
the previous version of the layer (so far, we don't know how that has
happened).

To avoid such situations, don't just leave whatever was in
the layer directory laying around; try to remove any pre-existing
contents, as well as the symbolic link in linkDir, if any.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
  • Loading branch information
mtrmac committed Feb 22, 2022
1 parent e2631cd commit 733574c
Showing 1 changed file with 13 additions and 0 deletions.
13 changes: 13 additions & 0 deletions drivers/overlay/overlay.go
Expand Up @@ -918,6 +918,15 @@ func (d *Driver) create(id, parent string, opts *graphdriver.CreateOpts, disable
d.locker.Lock(id)
defer d.locker.Unlock(id)

if _, err := system.Lstat(dir); err == nil {
logrus.Warnf("Trying to create a layer %#v while directory %q already exists; removing it first", id, dir)
// Don’t just os.RemoveAll(dir) here; removeLocked also removes the link in linkDir,
// so that we can’t end up with two symlinks in linkDir pointing to the same layer.
if err := d.removeLocked(id); err != nil {
return errors.Wrapf(err, "removing a pre-existing layer directory %q", dir)
}
}

if err := idtools.MkdirAllAndChownNew(dir, 0700, idPair); err != nil {
return err
}
Expand Down Expand Up @@ -1154,6 +1163,10 @@ func (d *Driver) Remove(id string) error {
d.locker.Lock(id)
defer d.locker.Unlock(id)

return d.removeLocked(id)
}

func (d *Driver) removeLocked(id string) error {
dir := d.dir(id)
lid, err := ioutil.ReadFile(path.Join(dir, "link"))
if err == nil {
Expand Down

0 comments on commit 733574c

Please sign in to comment.