[PATCH] ovl: clean up dir on casefold mismatch

[PATCH] ovl: clean up dir on casefold mismatch

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

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@vger.kernel.org
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 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;
-- 
2.43.0

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

[Tao Cui]

在 2026/9/7 12:55, Tao Cui 写道:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> 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@vger.kernel.org
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> ---
>  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.

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

[Amir Goldstein]

On Mon, Sep 7, 2026 at 7:38 AM Tao Cui <cui.tao@linux.dev> wrote:
>
>
>
> 在 2026/9/7 12:55, Tao Cui 写道:
> > From: Tao Cui <cuitao@kylinos.cn>
> >
> > 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.

Hi Tao,

This is a case of:

Changes to the underlying filesystems while part of a mounted overlay
filesystem are not allowed.  If the underlying filesystem is changed,
the behavior of the overlay is undefined, though it will not result in
a crash or deadlock.

I am not sympathetic to adding code to gracefully cleanup after
this error.

Assuming that you are utilizing LLM to help you with writing overlayfs
patches please refer LLM to this commandment.

Thanks,
Amir.

> > 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@vger.kernel.org
> > Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> > ---
> >  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.
>
>
>