Commit a105d685 authored by Miklos Szeredi's avatar Miklos Szeredi

ovl: fix remove/copy-up race

ovl_remove_and_whiteout() needs to check if upper dentry exists or not
after having locked upper parent directory.

Previously we used a "type" value computed before locking the upper parent
directory, which is susceptible to racing with copy-up.

There's a similar check in ovl_check_empty_and_clear().  This one is not
actually racy, since copy-up doesn't change the "emptyness" property of a
directory.  Add a comment to this effect, and check the existence of upper
dentry locally to make the code cleaner.
Signed-off-by: default avatarMiklos Szeredi <mszeredi@suse.cz>
parent ef94b186
...@@ -284,8 +284,7 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry, ...@@ -284,8 +284,7 @@ static struct dentry *ovl_clear_empty(struct dentry *dentry,
return ERR_PTR(err); return ERR_PTR(err);
} }
static struct dentry *ovl_check_empty_and_clear(struct dentry *dentry, static struct dentry *ovl_check_empty_and_clear(struct dentry *dentry)
enum ovl_path_type type)
{ {
int err; int err;
struct dentry *ret = NULL; struct dentry *ret = NULL;
...@@ -294,8 +293,17 @@ static struct dentry *ovl_check_empty_and_clear(struct dentry *dentry, ...@@ -294,8 +293,17 @@ static struct dentry *ovl_check_empty_and_clear(struct dentry *dentry,
err = ovl_check_empty_dir(dentry, &list); err = ovl_check_empty_dir(dentry, &list);
if (err) if (err)
ret = ERR_PTR(err); ret = ERR_PTR(err);
else if (type == OVL_PATH_MERGE) else {
ret = ovl_clear_empty(dentry, &list); /*
* If no upperdentry then skip clearing whiteouts.
*
* Can race with copy-up, since we don't hold the upperdir
* mutex. Doesn't matter, since copy-up can't create a
* non-empty directory from an empty one.
*/
if (ovl_dentry_upper(dentry))
ret = ovl_clear_empty(dentry, &list);
}
ovl_cache_free(&list); ovl_cache_free(&list);
...@@ -487,8 +495,7 @@ static int ovl_link(struct dentry *old, struct inode *newdir, ...@@ -487,8 +495,7 @@ static int ovl_link(struct dentry *old, struct inode *newdir,
return err; return err;
} }
static int ovl_remove_and_whiteout(struct dentry *dentry, static int ovl_remove_and_whiteout(struct dentry *dentry, bool is_dir)
enum ovl_path_type type, bool is_dir)
{ {
struct dentry *workdir = ovl_workdir(dentry); struct dentry *workdir = ovl_workdir(dentry);
struct inode *wdir = workdir->d_inode; struct inode *wdir = workdir->d_inode;
...@@ -500,7 +507,7 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, ...@@ -500,7 +507,7 @@ static int ovl_remove_and_whiteout(struct dentry *dentry,
int err; int err;
if (is_dir) { if (is_dir) {
opaquedir = ovl_check_empty_and_clear(dentry, type); opaquedir = ovl_check_empty_and_clear(dentry);
err = PTR_ERR(opaquedir); err = PTR_ERR(opaquedir);
if (IS_ERR(opaquedir)) if (IS_ERR(opaquedir))
goto out; goto out;
...@@ -515,9 +522,10 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, ...@@ -515,9 +522,10 @@ static int ovl_remove_and_whiteout(struct dentry *dentry,
if (IS_ERR(whiteout)) if (IS_ERR(whiteout))
goto out_unlock; goto out_unlock;
if (type == OVL_PATH_LOWER) { upper = ovl_dentry_upper(dentry);
if (!upper) {
upper = lookup_one_len(dentry->d_name.name, upperdir, upper = lookup_one_len(dentry->d_name.name, upperdir,
dentry->d_name.len); dentry->d_name.len);
err = PTR_ERR(upper); err = PTR_ERR(upper);
if (IS_ERR(upper)) if (IS_ERR(upper))
goto kill_whiteout; goto kill_whiteout;
...@@ -529,7 +537,6 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, ...@@ -529,7 +537,6 @@ static int ovl_remove_and_whiteout(struct dentry *dentry,
} else { } else {
int flags = 0; int flags = 0;
upper = ovl_dentry_upper(dentry);
if (opaquedir) if (opaquedir)
upper = opaquedir; upper = opaquedir;
err = -ESTALE; err = -ESTALE;
...@@ -648,7 +655,7 @@ static int ovl_do_remove(struct dentry *dentry, bool is_dir) ...@@ -648,7 +655,7 @@ static int ovl_do_remove(struct dentry *dentry, bool is_dir)
cap_raise(override_cred->cap_effective, CAP_CHOWN); cap_raise(override_cred->cap_effective, CAP_CHOWN);
old_cred = override_creds(override_cred); old_cred = override_creds(override_cred);
err = ovl_remove_and_whiteout(dentry, type, is_dir); err = ovl_remove_and_whiteout(dentry, is_dir);
revert_creds(old_cred); revert_creds(old_cred);
put_cred(override_cred); put_cred(override_cred);
...@@ -781,7 +788,7 @@ static int ovl_rename2(struct inode *olddir, struct dentry *old, ...@@ -781,7 +788,7 @@ static int ovl_rename2(struct inode *olddir, struct dentry *old,
} }
if (overwrite && (new_type == OVL_PATH_LOWER || new_type == OVL_PATH_MERGE) && new_is_dir) { if (overwrite && (new_type == OVL_PATH_LOWER || new_type == OVL_PATH_MERGE) && new_is_dir) {
opaquedir = ovl_check_empty_and_clear(new, new_type); opaquedir = ovl_check_empty_and_clear(new);
err = PTR_ERR(opaquedir); err = PTR_ERR(opaquedir);
if (IS_ERR(opaquedir)) { if (IS_ERR(opaquedir)) {
opaquedir = NULL; opaquedir = NULL;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment