Re: [PATCH] ovl: clean up dir on casefold mismatch

[Tao Cui]

在 2026/9/7 12:55, Tao Cui 写道:
> From: Tao Cui <cuitao@xxxxxxxxxx>
> 
> In ovl_create_real(), the S_IFDIR case checks after a successful
> ovl_do_mkdir() that the new directory inherited the expected casefold
> flag.  On mismatch, err is set to -EINVAL but the directory that was
> just created in the workdir or in the upper layer is left behind.
> 
> The mismatch is reachable: ofs->casefold is fixed at mount time, but
> the casefold flag of the index dir can diverge.  With index=on, a
> pre-existing "index" directory with +F is accepted at mount, and every
> temp mkdir for an index entry inside it fails the check and leaks one
> directory.  Measured on an ext4 casefold upper: 100 directory renames
> left 100 "#nnnn" entries in the index dir, one per failed mkdir,
> growing without bound.
> 
> Clean up the created directory with ovl_cleanup_locked() before
> returning the error.  All callers of ovl_create_real() arrive with the
> parent inode locked (via start_creating()), so the locked variant must
> be used; ovl_cleanup() would deadlock on inode_lock().
> 
> Fixes: dfc7da402ccc9 ("ovl: Check for casefold consistency when creating new dentries")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Tao Cui <cuitao@xxxxxxxxxx>
> ---
>  fs/overlayfs/dir.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
> index 7beb0af26498..4e451bbd8f97 100644
> --- a/fs/overlayfs/dir.c
> +++ b/fs/overlayfs/dir.c
> @@ -188,6 +188,7 @@ struct dentry *ovl_create_real(struct ovl_fs *ofs, struct dentry *parent,
>  			if (!err && ofs->casefold != ovl_dentry_casefolded(newdentry)) {
>  				pr_warn_ratelimited("wrong inherited casefold (%pd2)\n",
>  						    newdentry);
> +				ovl_cleanup_locked(ofs, dir, newdentry);
>  				err = -EINVAL;
>  			}
>  			break;

Sashiko pointed out that the casefold check can crash on a negative
dentry (https://sashiko.dev/#/patchset/20260907045517.1347518-1-cui.tao%40linux.dev):

ovl_dentry_casefolded() evaluates IS_CASEFOLDED(d_inode()), which is a
plain dereference, so a negative dentry would crash before we even get
to the WARN_ON(!newdentry->d_inode) check below.  And if it somehow
survived that, ovl_cleanup_locked() would rmdir a negative dentry.

The NULL deref itself is pre-existing (dfc7da402ccc9), but the cleanup
call makes it worse, so I'll fix the ordering in v2:

+			if (!err && d_is_positive(newdentry) &&
+			    ofs->casefold != ovl_dentry_casefolded(newdentry)) {

Negative dentries stay on the existing WARN_ON path.