Skip to contentSkip to conversation

ovl: check dentry positiveness in ovl_cleanup_whiteouts()

PATCH Applied Konstantin Khlebnikov 2 participants 3 messages Nov 16, 2015 – Dec 9, 2015

Subsystems (2)
Stable branch Overlay filesystem
lore
Share & tools
lore

Konstantin Khlebnikov

author
#0 lore raw

PATCH 1/1 ovl: check dentry positiveness in ovl_cleanup_whiteouts()

This patch fixes kernel crash at removing directory which contains whiteouts from lower layers.

Cache of directory content passed as "list" contains entries from all layers, including whiteouts from lower layers. So, lookup in upper dir (moved into work at this stage) will return negative entry. Plus this cache is filled long before and we can race with external removal.

Example:
mkdir -p lower0/dir lower1/dir upper work overlay
touch lower0/dir/a lower0/dir/b
mknod lower1/dir/a c 0 0
mount -t overlay none overlay -o lowerdir=lower1:lower0,upperdir=upper,workdir=work rm -fr overlay/dir

Signed-off-by Konstantin Khlebnikov <[email protected]>
Cc Stable <[email protected]> # 3.18+
---
 fs/overlayfs/readdir.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
fs/overlayfs/readdir.c +2 -1
index 70e9af551600..adcb1398c481 100644
@@ -571,7 +571,8 @@ void ovl_cleanup_whiteouts(struct dentry *upper, struct list_head *list)
 			       (int) PTR_ERR(dentry));
 			continue;
 		}
-		ovl_cleanup(upper->d_inode, dentry);
+		if (dentry->d_inode)
+			ovl_cleanup(upper->d_inode, dentry);
 		dput(dentry);
 	}
 	mutex_unlock(&upper->d_inode->i_mutex);
author
#1 lore raw

Note: kernels starting from 4.0 prints this

[ 72.925147] overlayfs: cleanup of '#ffff88022da16280/a' failed (-2)

instead of crashing, because of this part

fs/overlayfs/dir.c +1 -1
@@ -19,7 +19,7 @@ void ovl_cleanup(struct inode *wdir, struct dentry *wdentry)
        int err;
 
        dget(wdentry);
-       if (S_ISDIR(wdentry->d_inode->i_mode))
+       if (d_is_dir(wdentry))
                err = ovl_do_rmdir(wdir, wdentry);
        else
                err = ovl_do_unlink(wdir, wdentry);

of e36cb0b89ce20b4f8786a57e8a6bc8476f577650
("VFS: (Scripted) Convert S_ISLNK/DIR/REG(dentry->d_inode) to d_is_*(dentry)")

in older kernels crash happens at dereferencing wdentry->d_inode ovl_do_rmdir/unlink calls vfs_unlink/vfs_rmdir which checks positiveness in may_delete(). both returns -ENOENT (-2) in that case.

So, patch is still required: at least for avoiding flood in kernel log.

Quoted 36 lines · Konstantin Khlebnikov

This patch fixes kernel crash at removing directory which contains whiteouts from lower layers.

Cache of directory content passed as "list" contains entries from all layers, including whiteouts from lower layers. So, lookup in upper dir (moved into work at this stage) will return negative entry. Plus this cache is filled long before and we can race with external removal.

Example:
mkdir -p lower0/dir lower1/dir upper work overlay
touch lower0/dir/a lower0/dir/b
mknod lower1/dir/a c 0 0
mount -t overlay none overlay -o lowerdir=lower1:lower0,upperdir=upper,workdir=work rm -fr overlay/dir

Signed-off-by Konstantin Khlebnikov <[email protected]>
Cc Stable <[email protected]> # 3.18+
---
  fs/overlayfs/readdir.c |    3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
fs/overlayfs/readdir.c +2 -1
index 70e9af551600..adcb1398c481 100644
@@ -571,7 +571,8 @@ void ovl_cleanup_whiteouts(struct dentry *upper, struct list_head *list)
  			       (int) PTR_ERR(dentry));
  			continue;
  		}
-		ovl_cleanup(upper->d_inode, dentry);
+		if (dentry->d_inode)
+			ovl_cleanup(upper->d_inode, dentry);
  		dput(dentry);
  	}
  	mutex_unlock(&upper->d_inode->i_mutex);
Signature · 1 line
Konstantin
maintainer Applied
#2 lore raw
In reply to Konstantin Khlebnikov
Quoted 15 lines · Konstantin Khlebnikov

This patch fixes kernel crash at removing directory which contains whiteouts from lower layers.

Cache of directory content passed as "list" contains entries from all layers, including whiteouts from lower layers. So, lookup in upper dir (moved into work at this stage) will return negative entry. Plus this cache is filled long before and we can race with external removal.

Example:
mkdir -p lower0/dir lower1/dir upper work overlay
touch lower0/dir/a lower0/dir/b
mknod lower1/dir/a c 0 0
mount -t overlay none overlay -o lowerdir=lower1:lower0,upperdir=upper,workdir=work rm -fr overlay/dir

Quoted 2 lines
Signed-off-by Konstantin Khlebnikov <[email protected]>
Cc Stable <[email protected]> # 3.18+

Thanks, applied.

Miklos