[PATCH] erofs: validate nameoff for all dirents in erofs_fill_dentries()

[PATCH] erofs: validate nameoff for all dirents in erofs_fill_dentries()

[Junrui Luo]

erofs_readdir() validates de[0].nameoff before calling
erofs_fill_dentries(), but subsequent dirents are used without
validation. The loop computes `maxsize - nameoff` as an unsigned int
to bound strnlen().

If a crafted EROFS image has a dirent with nameoff >= maxsize, the
subtraction underflows, causing strnlen() to read past the block
buffer.

Fix by validating each entry's nameoff at the top of the loop: it
must be >= nameoff0 and <= maxsize.

Cc: stable@vger.kernel.org
Fixes: 3aa8ec716e52 ("staging: erofs: add directory operations")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
 fs/erofs/dir.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
index e5132575b9d3..2efa16fa162f 100644
--- a/fs/erofs/dir.c
+++ b/fs/erofs/dir.c
@@ -19,6 +19,13 @@ static int erofs_fill_dentries(struct inode *dir, struct dir_context *ctx,
 		const char *de_name = (char *)dentry_blk + nameoff;
 		unsigned int de_namelen;
 
+		if (nameoff < nameoff0 || nameoff > maxsize) {
+			erofs_err(dir->i_sb, "bogus dirent @ nid %llu",
+				  EROFS_I(dir)->nid);
+			DBG_BUGON(1);
+			return -EFSCORRUPTED;
+		}
+
 		/* the last dirent in the block? */
 		if (de + 1 >= end)
 			de_namelen = strnlen(de_name, maxsize - nameoff);

---
base-commit: 7aaa8047eafd0bd628065b15757d9b48c5f9c07d
change-id: 20260414-fixes-ae20cd389f52

Best regards,
-- 
Junrui Luo <moonafterrain@outlook.com>

Re: [PATCH] erofs: validate nameoff for all dirents in erofs_fill_dentries()

[Junrui Luo]

Here is a reproducer for the issue fixed by this patch.

Creates a 3-block (12KB) minimal EROFS image:
- Block 0: superblock
- Block 1: two compact inodes (root dir + dummy file)
- Block 2: directory data with 4 dirents

  de[0]: nameoff=48  FT_DIR "."
  de[1]: nameoff=49  FT_DIR ".."
  de[2]: nameoff=51  FT_REG "f1"
  de[3]: nameoff=0xFFFF FT_REG

Reproducible image (base64-encoded gzipped blob):

H4sIALRA3mkCA+3XwQmDQBAF0HW95mAlSyQVpJn0YS8Wpt5z9GwGwkICNqC+BwOfZU//NJMScFXL
vE4132Lyzp82plEVAACcwvsZW3/3zXln1x+H/5esMgAAADik+89V30euF/8jUs3b1qRSyqtXFwAA
ABzKB5GqxlwAMAAA

Trigger

```
  #define _GNU_SOURCE
  #include <fcntl.h>
  #include <stdio.h>
  #include <unistd.h>
  #include <errno.h>
  #include <sys/syscall.h>

  int main(int argc, char *argv[]) {
      const char *p = argc > 1 ? argv[1] : "/mnt";
      char buf[4096];
      int fd = open(p, O_RDONLY | O_DIRECTORY);
      if (fd < 0) { perror("open"); return 1; }

      /* skip to de[3] at byte 36 */
      lseek(fd, 36, SEEK_SET);
      int n = syscall(SYS_getdents64, fd, buf, sizeof(buf));
      printf("getdents64=%d errno=%d\n", n, errno);
      close(fd);
  }
```
	
$ gcc -static -o trigger trigger.c
$ mount -t erofs -o loop image.erofs /mnt
$ ./trigger /mnt

output (kernel 7.0-rc6, CONFIG_KASAN=y):

  BUG: KASAN: slab-out-of-bounds in strnlen+0x74/0x80
  Read of size 1 at addr ffff888008828fff by task trigger/76

Re: [PATCH] erofs: validate nameoff for all dirents in erofs_fill_dentries()

[Gao Xiang]

Hi Junrui,

On 2026/4/14 23:20, Junrui Luo wrote:
> erofs_readdir() validates de[0].nameoff before calling
> erofs_fill_dentries(), but subsequent dirents are used without
> validation. The loop computes `maxsize - nameoff` as an unsigned int
> to bound strnlen().

The issue is true, but I don't think the description is valid.

I think what we missed is to check the last dirent nameoff vs
maxsize.

BTW, please don't "To" too many people (especially Miao Xie
and Greg), basically I think you only need to post to people
according to `./checkpoint.pl` but leave indivudual person
into "Cc" instead.

> 
> If a crafted EROFS image has a dirent with nameoff >= maxsize, the
> subtraction underflows, causing strnlen() to read past the block
> buffer.
> 
> Fix by validating each entry's nameoff at the top of the loop: it
> must be >= nameoff0 and <= maxsize.
> 
> Cc: stable@vger.kernel.org
> Fixes: 3aa8ec716e52 ("staging: erofs: add directory operations")
> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
> ---
>   fs/erofs/dir.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
> index e5132575b9d3..2efa16fa162f 100644
> --- a/fs/erofs/dir.c
> +++ b/fs/erofs/dir.c
> @@ -19,6 +19,13 @@ static int erofs_fill_dentries(struct inode *dir, struct dir_context *ctx,
>   		const char *de_name = (char *)dentry_blk + nameoff;
>   		unsigned int de_namelen;
>   
> +		if (nameoff < nameoff0 || nameoff > maxsize) {
> +			erofs_err(dir->i_sb, "bogus dirent @ nid %llu",
> +				  EROFS_I(dir)->nid);
> +			DBG_BUGON(1);
> +			return -EFSCORRUPTED;
> +		}

I think the only thing we need is the following diff:

[The reason why nameoff < nameoff0 is unneeded, since
  `de_namelen > EROFS_NAME_LEN` ensures the nameoff delta
  won't be negative (so nameoff will increase.)

  and `nameoff + de_namelen > maxsize` implies
  `nameoff > maxsize` so `nameoff > maxsize` is unneeded too.]

diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
index e5132575b9d3..e0666d6da9af 100644
--- a/fs/erofs/dir.c
+++ b/fs/erofs/dir.c
@@ -20,19 +20,18 @@ static int erofs_fill_dentries(struct inode *dir, struct dir_context *ctx,
  		unsigned int de_namelen;

  		/* the last dirent in the block? */
-		if (de + 1 >= end)
+		if (de + 1 >= end) {
+			if (maxsize <= nameoff)
+				goto err_bogus;
  			de_namelen = strnlen(de_name, maxsize - nameoff);
-		else
+		} else {
  			de_namelen = le16_to_cpu(de[1].nameoff) - nameoff;
+		}

  		/* a corrupted entry is found */
  		if (nameoff + de_namelen > maxsize ||
-		    de_namelen > EROFS_NAME_LEN) {
-			erofs_err(dir->i_sb, "bogus dirent @ nid %llu",
-				  EROFS_I(dir)->nid);
-			DBG_BUGON(1);
-			return -EFSCORRUPTED;
-		}
+		    de_namelen > EROFS_NAME_LEN)
+			goto err_bogus;

  		if (!dir_emit(ctx, de_name, de_namelen,
  			      erofs_nid_to_ino64(EROFS_SB(dir->i_sb),
@@ -42,6 +41,10 @@ static int erofs_fill_dentries(struct inode *dir, struct dir_context *ctx,
  		ctx->pos += sizeof(struct erofs_dirent);
  	}
  	return 0;
+err_bogus:
+	erofs_err(dir->i_sb, "bogus dirent @ nid %llu", EROFS_I(dir)->nid);
+	DBG_BUGON(1);
+	return -EFSCORRUPTED;
  }

  static int erofs_readdir(struct file *f, struct dir_context *ctx)


Thanks,
Gao Xiang

> +
>   		/* the last dirent in the block? */
>   		if (de + 1 >= end)
>   			de_namelen = strnlen(de_name, maxsize - nameoff);
> 
> ---
> base-commit: 7aaa8047eafd0bd628065b15757d9b48c5f9c07d
> change-id: 20260414-fixes-ae20cd389f52
> 
> Best regards,

Re: [PATCH] erofs: validate nameoff for all dirents in erofs_fill_dentries()

[Gao Xiang]

On 2026/4/15 00:00, Gao Xiang wrote:
> Hi Junrui,
> 
> On 2026/4/14 23:20, Junrui Luo wrote:
>> erofs_readdir() validates de[0].nameoff before calling
>> erofs_fill_dentries(), but subsequent dirents are used without
>> validation. The loop computes `maxsize - nameoff` as an unsigned int
>> to bound strnlen().
> 
> The issue is true, but I don't think the description is valid.
> 
> I think what we missed is to check the last dirent nameoff vs
> maxsize.
> 
> BTW, please don't "To" too many people (especially Miao Xie
> and Greg), basically I think you only need to post to people
> according to `./checkpoint.pl` but leave indivudual person
> into "Cc" instead.
> 
>>
>> If a crafted EROFS image has a dirent with nameoff >= maxsize, the
>> subtraction underflows, causing strnlen() to read past the block
>> buffer.
>>
>> Fix by validating each entry's nameoff at the top of the loop: it
>> must be >= nameoff0 and <= maxsize.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 3aa8ec716e52 ("staging: erofs: add directory operations")
>> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
>> Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
>> ---
>>   fs/erofs/dir.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
>> index e5132575b9d3..2efa16fa162f 100644
>> --- a/fs/erofs/dir.c
>> +++ b/fs/erofs/dir.c
>> @@ -19,6 +19,13 @@ static int erofs_fill_dentries(struct inode *dir, struct dir_context *ctx,
>>           const char *de_name = (char *)dentry_blk + nameoff;
>>           unsigned int de_namelen;
>> +        if (nameoff < nameoff0 || nameoff > maxsize) {
>> +            erofs_err(dir->i_sb, "bogus dirent @ nid %llu",
>> +                  EROFS_I(dir)->nid);
>> +            DBG_BUGON(1);
>> +            return -EFSCORRUPTED;
>> +        }
> 
> I think the only thing we need is the following diff:
> 
> [The reason why nameoff < nameoff0 is unneeded, since
>   `de_namelen > EROFS_NAME_LEN` ensures the nameoff delta
>   won't be negative (so nameoff will increase.)
> 
>   and `nameoff + de_namelen > maxsize` implies
>   `nameoff > maxsize` so `nameoff > maxsize` is unneeded too.]

A even better diff is as below:

diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
index e5132575b9d3..2b8375c7d523 100644
--- a/fs/erofs/dir.c
+++ b/fs/erofs/dir.c
@@ -19,20 +19,18 @@ static int erofs_fill_dentries(struct inode *dir, struct dir_context *ctx,
  		const char *de_name = (char *)dentry_blk + nameoff;
  		unsigned int de_namelen;

-		/* the last dirent in the block? */
-		if (de + 1 >= end)
-			de_namelen = strnlen(de_name, maxsize - nameoff);
-		else
+		/* non-trailing dirent in the directory block? */
+		if (de + 1 < end)
  			de_namelen = le16_to_cpu(de[1].nameoff) - nameoff;
+		else if (maxsize <= nameoff)
+			goto err_bogus;
+		else
+			de_namelen = strnlen(de_name, maxsize - nameoff);

  		/* a corrupted entry is found */
-		if (nameoff + de_namelen > maxsize ||
-		    de_namelen > EROFS_NAME_LEN) {
-			erofs_err(dir->i_sb, "bogus dirent @ nid %llu",
-				  EROFS_I(dir)->nid);
-			DBG_BUGON(1);
-			return -EFSCORRUPTED;
-		}
+		if (!clamp(de_namelen, 1, EROFS_NAME_LEN) ||
+		    nameoff + de_namelen > maxsize)
+			goto err_bogus;

  		if (!dir_emit(ctx, de_name, de_namelen,
  			      erofs_nid_to_ino64(EROFS_SB(dir->i_sb),
@@ -42,6 +40,10 @@ static int erofs_fill_dentries(struct inode *dir, struct dir_context *ctx,
  		ctx->pos += sizeof(struct erofs_dirent);
  	}
  	return 0;
+err_bogus:
+	erofs_err(dir->i_sb, "bogus dirent @ nid %llu", EROFS_I(dir)->nid);
+	DBG_BUGON(1);
+	return -EFSCORRUPTED;
  }

  static int erofs_readdir(struct file *f, struct dir_context *ctx)