Skip to content

Commit cfc9fde

Browse files
Maxim PatlasovMiklos Szeredi
authored andcommitted
ovl: verify upper dentry in ovl_remove_and_whiteout()
The upper dentry may become stale before we call ovl_lock_rename_workdir. For example, someone could (mistakenly or maliciously) manually unlink(2) it directly from upperdir. To ensure it is not stale, let's lookup it after ovl_lock_rename_workdir and and check if it matches the upper dentry. Essentially, it is the same problem and similar solution as in commit 11f3710 ("ovl: verify upper dentry before unlink and rename"). Signed-off-by: Maxim Patlasov <[email protected]> Signed-off-by: Miklos Szeredi <[email protected]> Cc: <[email protected]>
1 parent 07a2daa commit cfc9fde

File tree

1 file changed

+24
-30
lines changed

1 file changed

+24
-30
lines changed

fs/overlayfs/dir.c

Lines changed: 24 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,7 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, bool is_dir)
505505
struct dentry *upper;
506506
struct dentry *opaquedir = NULL;
507507
int err;
508+
int flags = 0;
508509

509510
if (WARN_ON(!workdir))
510511
return -EROFS;
@@ -534,46 +535,39 @@ static int ovl_remove_and_whiteout(struct dentry *dentry, bool is_dir)
534535
if (err)
535536
goto out_dput;
536537

537-
whiteout = ovl_whiteout(workdir, dentry);
538-
err = PTR_ERR(whiteout);
539-
if (IS_ERR(whiteout))
538+
upper = lookup_one_len(dentry->d_name.name, upperdir,
539+
dentry->d_name.len);
540+
err = PTR_ERR(upper);
541+
if (IS_ERR(upper))
540542
goto out_unlock;
541543

542-
upper = ovl_dentry_upper(dentry);
543-
if (!upper) {
544-
upper = lookup_one_len(dentry->d_name.name, upperdir,
545-
dentry->d_name.len);
546-
err = PTR_ERR(upper);
547-
if (IS_ERR(upper))
548-
goto kill_whiteout;
549-
550-
err = ovl_do_rename(wdir, whiteout, udir, upper, 0);
551-
dput(upper);
552-
if (err)
553-
goto kill_whiteout;
554-
} else {
555-
int flags = 0;
544+
err = -ESTALE;
545+
if ((opaquedir && upper != opaquedir) ||
546+
(!opaquedir && ovl_dentry_upper(dentry) &&
547+
upper != ovl_dentry_upper(dentry))) {
548+
goto out_dput_upper;
549+
}
556550

557-
if (opaquedir)
558-
upper = opaquedir;
559-
err = -ESTALE;
560-
if (upper->d_parent != upperdir)
561-
goto kill_whiteout;
551+
whiteout = ovl_whiteout(workdir, dentry);
552+
err = PTR_ERR(whiteout);
553+
if (IS_ERR(whiteout))
554+
goto out_dput_upper;
562555

563-
if (is_dir)
564-
flags |= RENAME_EXCHANGE;
556+
if (d_is_dir(upper))
557+
flags = RENAME_EXCHANGE;
565558

566-
err = ovl_do_rename(wdir, whiteout, udir, upper, flags);
567-
if (err)
568-
goto kill_whiteout;
559+
err = ovl_do_rename(wdir, whiteout, udir, upper, flags);
560+
if (err)
561+
goto kill_whiteout;
562+
if (flags)
563+
ovl_cleanup(wdir, upper);
569564

570-
if (is_dir)
571-
ovl_cleanup(wdir, upper);
572-
}
573565
ovl_dentry_version_inc(dentry->d_parent);
574566
out_d_drop:
575567
d_drop(dentry);
576568
dput(whiteout);
569+
out_dput_upper:
570+
dput(upper);
577571
out_unlock:
578572
unlock_rename(workdir, upperdir);
579573
out_dput:

0 commit comments

Comments
 (0)