1. Patchew
  2. linux
  3. View series

[PATCH v2] ovl: Fix uninit-value in ovl_fill_real

Qing Wang posted 1 patch 1 week, 4 days ago 
fs/overlayfs/readdir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v2] ovl: Fix uninit-value in ovl_fill_real
Posted by Qing Wang 1 week, 4 days ago 
Syzbot reported a KMSAN uninit-value issue in ovl_fill_real.

This iusse's call chain is:
__do_sys_getdents64()
    -> iterate_dir()
        ...
            -> ext4_readdir()
                -> fscrypt_fname_alloc_buffer() // alloc
                -> fscrypt_fname_disk_to_usr // write without tail '\0'
                -> dir_emit()
                    -> ovl_fill_real() // read by strcmp()

The string is used to store the decrypted directory entry name for an
encrypted inode. As shown in the call chain, fscrypt_fname_disk_to_usr()
write it wthout null-terminate. However, ovl_fill_real() uses strcmp() to
compare the name against "..", which assumes a null-terminated string and
may trigger a KMSAN uninit-value warning when the buffer tail contains
uninit data.

Reported-by: syzbot+d130f98b2c265fae5297@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=d130f98b2c265fae5297
Fixes: 4edb83bb1041 ("ovl: constant d_ino for non-merge dirs")
Signed-off-by: Qing Wang <wangqing7171@gmail.com>
---
 fs/overlayfs/readdir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
index 160960bb0ad0..9ea3cd11dd93 100644
--- a/fs/overlayfs/readdir.c
+++ b/fs/overlayfs/readdir.c
@@ -755,7 +755,7 @@ static bool ovl_fill_real(struct dir_context *ctx, const char *name,
 	struct dir_context *orig_ctx = rdt->orig_ctx;
 	bool res;
 
-	if (rdt->parent_ino && strcmp(name, "..") == 0) {
+	if (rdt->parent_ino && namelen == 2 && !strncmp(name, "..", namelen)) {
 		ino = rdt->parent_ino;
 	} else if (rdt->cache) {
 		struct ovl_cache_entry *p;
-- 
2.34.1
Re: [PATCH v2] ovl: Fix uninit-value in ovl_fill_real
Posted by Miklos Szeredi 1 week, 4 days ago 
On Tue, 27 Jan 2026 at 11:52, Qing Wang <wangqing7171@gmail.com> wrote:
>
> Syzbot reported a KMSAN uninit-value issue in ovl_fill_real.
>
> This iusse's call chain is:
> __do_sys_getdents64()
>     -> iterate_dir()
>         ...
>             -> ext4_readdir()
>                 -> fscrypt_fname_alloc_buffer() // alloc
>                 -> fscrypt_fname_disk_to_usr // write without tail '\0'
>                 -> dir_emit()
>                     -> ovl_fill_real() // read by strcmp()
>
> The string is used to store the decrypted directory entry name for an
> encrypted inode. As shown in the call chain, fscrypt_fname_disk_to_usr()
> write it wthout null-terminate. However, ovl_fill_real() uses strcmp() to
> compare the name against "..", which assumes a null-terminated string and
> may trigger a KMSAN uninit-value warning when the buffer tail contains
> uninit data.
>
> Reported-by: syzbot+d130f98b2c265fae5297@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=d130f98b2c265fae5297
> Fixes: 4edb83bb1041 ("ovl: constant d_ino for non-merge dirs")
> Signed-off-by: Qing Wang <wangqing7171@gmail.com>
> ---
>  fs/overlayfs/readdir.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index 160960bb0ad0..9ea3cd11dd93 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -755,7 +755,7 @@ static bool ovl_fill_real(struct dir_context *ctx, const char *name,
>         struct dir_context *orig_ctx = rdt->orig_ctx;
>         bool res;
>
> -       if (rdt->parent_ino && strcmp(name, "..") == 0) {
> +       if (rdt->parent_ino && namelen == 2 && !strncmp(name, "..", namelen)) {

Yeah, and also the str... functions only make sense on null terminated
strings, so that needs to be memcmp.

Thanks,
Miklos
Re: [PATCH v2] ovl: Fix uninit-value in ovl_fill_real
Posted by Qing Wang 1 week, 3 days ago 
On Tue, 27 Jan 2026 at 19:19, Miklos Szeredi <miklos@szeredi.hu> wrote:
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index 160960bb0ad0..9ea3cd11dd93 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -755,7 +755,7 @@ static bool ovl_fill_real(struct dir_context *ctx, const char *name,
>         struct dir_context *orig_ctx = rdt->orig_ctx;
>         bool res;
>
> -       if (rdt->parent_ino && strcmp(name, "..") == 0) {
> +       if (rdt->parent_ino && namelen == 2 && !strncmp(name, "..", namelen)) {
> 
> Yeah, and also the str... functions only make sense on null terminated
> strings, so that needs to be memcmp.

Thanks for your idea, but I think strncmp is same as memncmp in this case.
strncmp dons't use null terminated.

```
int strncmp(const char *cs, const char *ct, size_t count)
{
	unsigned char c1, c2;

	while (count) {
		c1 = *cs++;
		c2 = *ct++;
		if (c1 != c2)
			return c1 < c2 ? -1 : 1;
		if (!c1)
			break;
		count--;
	}
	return 0;
}
```

What do you think?

--
Best Regards,
Qing
Re: [PATCH v2] ovl: Fix uninit-value in ovl_fill_real
Posted by Amir Goldstein 1 week, 3 days ago 
On Wed, Jan 28, 2026 at 3:42 AM Qing Wang <wangqing7171@gmail.com> wrote:
>
> On Tue, 27 Jan 2026 at 19:19, Miklos Szeredi <miklos@szeredi.hu> wrote:
> > diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> > index 160960bb0ad0..9ea3cd11dd93 100644
> > --- a/fs/overlayfs/readdir.c
> > +++ b/fs/overlayfs/readdir.c
> > @@ -755,7 +755,7 @@ static bool ovl_fill_real(struct dir_context *ctx, const char *name,
> >         struct dir_context *orig_ctx = rdt->orig_ctx;
> >         bool res;
> >
> > -       if (rdt->parent_ino && strcmp(name, "..") == 0) {
> > +       if (rdt->parent_ino && namelen == 2 && !strncmp(name, "..", namelen)) {
> >
> > Yeah, and also the str... functions only make sense on null terminated
> > strings, so that needs to be memcmp.
>
> Thanks for your idea, but I think strncmp is same as memncmp in this case.
> strncmp dons't use null terminated.
>
> ```
> int strncmp(const char *cs, const char *ct, size_t count)
> {
>         unsigned char c1, c2;
>
>         while (count) {
>                 c1 = *cs++;
>                 c2 = *ct++;
>                 if (c1 != c2)
>                         return c1 < c2 ? -1 : 1;
>                 if (!c1)
>                         break;
>                 count--;
>         }
>         return 0;
> }
> ```
>
> What do you think?

I think this discussion is not worth spending our time.

After namelen == 2, it really does not matter if we use strncmp or memcmp
IMO. I will post a followup patch to add name_is_dotdot() helper and use
those helpers in overlayfs readdir code instead of all the many open coded
variants that it has now, which may be safe, but still ugly.

In the meanwhile I'd rather apply v2 as is so it could be picked by stable.

Thanks,
Amir.
Re: [PATCH v2] ovl: Fix uninit-value in ovl_fill_real
Posted by Miklos Szeredi 1 week, 3 days ago 
On Wed, 28 Jan 2026 at 11:20, Amir Goldstein <amir73il@gmail.com> wrote:

> In the meanwhile I'd rather apply v2 as is so it could be picked by stable.

ACK.

Thanks,
Miklos
Re: [PATCH v2] ovl: Fix uninit-value in ovl_fill_real
Posted by Amir Goldstein 1 week, 4 days ago 
On Tue, Jan 27, 2026 at 11:52 AM Qing Wang <wangqing7171@gmail.com> wrote:
>
> Syzbot reported a KMSAN uninit-value issue in ovl_fill_real.
>
> This iusse's call chain is:
> __do_sys_getdents64()
>     -> iterate_dir()
>         ...
>             -> ext4_readdir()
>                 -> fscrypt_fname_alloc_buffer() // alloc
>                 -> fscrypt_fname_disk_to_usr // write without tail '\0'
>                 -> dir_emit()
>                     -> ovl_fill_real() // read by strcmp()
>
> The string is used to store the decrypted directory entry name for an
> encrypted inode. As shown in the call chain, fscrypt_fname_disk_to_usr()
> write it wthout null-terminate. However, ovl_fill_real() uses strcmp() to

typo: without

> compare the name against "..", which assumes a null-terminated string and
> may trigger a KMSAN uninit-value warning when the buffer tail contains
> uninit data.
>
> Reported-by: syzbot+d130f98b2c265fae5297@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=d130f98b2c265fae5297
> Fixes: 4edb83bb1041 ("ovl: constant d_ino for non-merge dirs")
> Signed-off-by: Qing Wang <wangqing7171@gmail.com>

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

> ---
>  fs/overlayfs/readdir.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index 160960bb0ad0..9ea3cd11dd93 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -755,7 +755,7 @@ static bool ovl_fill_real(struct dir_context *ctx, const char *name,
>         struct dir_context *orig_ctx = rdt->orig_ctx;
>         bool res;
>
> -       if (rdt->parent_ino && strcmp(name, "..") == 0) {
> +       if (rdt->parent_ino && namelen == 2 && !strncmp(name, "..", namelen)) {
>                 ino = rdt->parent_ino;
>         } else if (rdt->cache) {
>                 struct ovl_cache_entry *p;
> --
> 2.34.1
>

Eric,

Considering that fscrypt_fname_alloc_buffer() anyway allocates the byte for NUL
termination and that decrypted names are zero padded (right?).

How about reinforcing this fix with:

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index a9a4432d12ba1..97e00af49bb9f 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -276,6 +276,7 @@ int fscrypt_fname_disk_to_usr(const struct inode *inode,
        if (fscrypt_is_dot_dotdot(&qname)) {
                oname->name[0] = '.';
                oname->name[iname->len - 1] = '.';
+               oname->name[iname->len] = 0;
                oname->len = iname->len;
                return 0;
        }

Thanks,
Amir.
Re: [PATCH v2] ovl: Fix uninit-value in ovl_fill_real
Posted by Eric Biggers 1 week, 3 days ago 
On Tue, Jan 27, 2026 at 12:09:47PM +0100, Amir Goldstein wrote:
> Eric,
> 
> Considering that fscrypt_fname_alloc_buffer() anyway allocates the byte for NUL
> termination and that decrypted names are zero padded (right?).

Only when the length of the original filename isn't already a multiple
of the padding amount, as configured by FSCRYPT_POLICY_FLAGS_PAD_*.

> How about reinforcing this fix with:
> 
> diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
> index a9a4432d12ba1..97e00af49bb9f 100644
> --- a/fs/crypto/fname.c
> +++ b/fs/crypto/fname.c
> @@ -276,6 +276,7 @@ int fscrypt_fname_disk_to_usr(const struct inode *inode,
>         if (fscrypt_is_dot_dotdot(&qname)) {
>                 oname->name[0] = '.';
>                 oname->name[iname->len - 1] = '.';
> +               oname->name[iname->len] = 0;
>                 oname->len = iname->len;
>                 return 0;

Do you propose to do the same for all callers of dir_emit() in all
filesystems?  It seems not NUL-terminating the name is normal.  Just
usually the name is in the pagecache rather than slab memory, which
makes KMSAN not usually notice the out-of-bounds read in overlayfs.

- Eric

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.