public inbox for linux-erofs@ozlabs.org
 help / color / mirror / Atom feed
* [PATCH] erofs: fix the out-of-bounds nameoff handling for trailing dirents
@ 2026-04-16  6:03 Gao Xiang
  2026-04-16  6:35 ` [PATCH v2] " Gao Xiang
  0 siblings, 1 reply; 8+ messages in thread
From: Gao Xiang @ 2026-04-16  6:03 UTC (permalink / raw)
  To: linux-erofs; +Cc: LKML, Gao Xiang, Yuhao Jiang, Junrui Luo, stable

Currently we already have boundary-checks for nameoffs, but the trailing
dirents are special since the namelens are calculated with strnlen()
with unchecked nameoffs.

If a crafted EROFS has a trailing dirent with nameoff >= maxsize,
maxsize - nameoff can underflow, causing strnlen() to read past the
directory block.

Fixes: 3aa8ec716e52 ("staging: erofs: add directory operations")
Fixes: 33bac912840f ("staging: erofs: keep corrupted fs from crashing kernel in erofs_readdir()")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Reported-by: Junrui Luo <moonafterrain@outlook.com>
Closes: https://lore.kernel.org/r/A0FD7E0F-7558-49B0-8BC8-EB1ECDB2479A@outlook.com
Cc: stable@vger.kernel.org
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
 fs/erofs/dir.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
index e5132575b9d3..ecfac9b49322 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;
-		}
+		/* a corrupted entry is found (including negative namelen) */
+		if (!clamp_t(unsigned int, 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)
-- 
2.43.5



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2] erofs: fix the out-of-bounds nameoff handling for trailing dirents
  2026-04-16  6:03 [PATCH] erofs: fix the out-of-bounds nameoff handling for trailing dirents Gao Xiang
@ 2026-04-16  6:35 ` Gao Xiang
  2026-04-16  9:44   ` [PATCH v3] " Gao Xiang
  0 siblings, 1 reply; 8+ messages in thread
From: Gao Xiang @ 2026-04-16  6:35 UTC (permalink / raw)
  To: linux-erofs; +Cc: LKML, Gao Xiang, Yuhao Jiang, Junrui Luo, stable

Currently we already have boundary-checks for nameoffs, but the trailing
dirents are special since the namelens are calculated with strnlen()
with unchecked nameoffs.

If a crafted EROFS has a trailing dirent with nameoff >= maxsize,
maxsize - nameoff can underflow, causing strnlen() to read past the
directory block.

Fixes: 3aa8ec716e52 ("staging: erofs: add directory operations")
Fixes: 33bac912840f ("staging: erofs: keep corrupted fs from crashing kernel in erofs_readdir()")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Reported-by: Junrui Luo <moonafterrain@outlook.com>
Closes: https://lore.kernel.org/r/A0FD7E0F-7558-49B0-8BC8-EB1ECDB2479A@outlook.com
Cc: stable@vger.kernel.org
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
v2:
 - should use in_range32() instead.

 fs/erofs/dir.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
index e5132575b9d3..c7717149c5ed 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;
-		}
+		/* a corrupted entry is found (including negative namelen) */
+		if (!in_range32(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)
-- 
2.43.5



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3] erofs: fix the out-of-bounds nameoff handling for trailing dirents
  2026-04-16  6:35 ` [PATCH v2] " Gao Xiang
@ 2026-04-16  9:44   ` Gao Xiang
  2026-04-21  7:26     ` Chao Yu
  0 siblings, 1 reply; 8+ messages in thread
From: Gao Xiang @ 2026-04-16  9:44 UTC (permalink / raw)
  To: linux-erofs; +Cc: LKML, Gao Xiang, Yuhao Jiang, Junrui Luo, stable

Currently we already have boundary-checks for nameoffs, but the trailing
dirents are special since the namelens are calculated with strnlen()
with unchecked nameoffs.

If a crafted EROFS has a trailing dirent with nameoff >= maxsize,
maxsize - nameoff can underflow, causing strnlen() to read past the
directory block.

nameoff0 should also be verified to be a multiple of
`sizeof(struct erofs_dirent)` as well [1].

[1] https://sashiko.dev/#/patchset/20260416063511.3173774-1-hsiangkao%40linux.alibaba.com
Fixes: 3aa8ec716e52 ("staging: erofs: add directory operations")
Fixes: 33bac912840f ("staging: erofs: keep corrupted fs from crashing kernel in erofs_readdir()")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Reported-by: Junrui Luo <moonafterrain@outlook.com>
Closes: https://lore.kernel.org/r/A0FD7E0F-7558-49B0-8BC8-EB1ECDB2479A@outlook.com
Cc: stable@vger.kernel.org
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
v3:
 - Disallow unaligned nameoff0 to avoid petential oob reads as well.

 fs/erofs/dir.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
index e5132575b9d3..d074fded1577 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;
-		}
+		/* a corrupted entry is found (including negative namelen) */
+		if (!in_range32(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)
@@ -88,7 +90,8 @@ static int erofs_readdir(struct file *f, struct dir_context *ctx)
 		}
 
 		nameoff = le16_to_cpu(de->nameoff);
-		if (nameoff < sizeof(struct erofs_dirent) || nameoff >= bsz) {
+		if (nameoff < sizeof(struct erofs_dirent) || nameoff >= bsz ||
+		    (nameoff % sizeof(struct erofs_dirent))) {
 			erofs_err(sb, "invalid de[0].nameoff %u @ nid %llu",
 				  nameoff, EROFS_I(dir)->nid);
 			err = -EFSCORRUPTED;
-- 
2.43.5



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] erofs: fix the out-of-bounds nameoff handling for trailing dirents
  2026-04-16  9:44   ` [PATCH v3] " Gao Xiang
@ 2026-04-21  7:26     ` Chao Yu
  2026-04-21  7:38       ` Gao Xiang
  2026-04-21  7:59       ` [PATCH v4] " Gao Xiang
  0 siblings, 2 replies; 8+ messages in thread
From: Chao Yu @ 2026-04-21  7:26 UTC (permalink / raw)
  To: Gao Xiang, linux-erofs; +Cc: chao, LKML, Yuhao Jiang, Junrui Luo, stable

On 4/16/2026 5:44 PM, Gao Xiang wrote:
> Currently we already have boundary-checks for nameoffs, but the trailing
> dirents are special since the namelens are calculated with strnlen()
> with unchecked nameoffs.
> 
> If a crafted EROFS has a trailing dirent with nameoff >= maxsize,
> maxsize - nameoff can underflow, causing strnlen() to read past the
> directory block.
> 
> nameoff0 should also be verified to be a multiple of
> `sizeof(struct erofs_dirent)` as well [1].
> 
> [1] https://sashiko.dev/#/patchset/20260416063511.3173774-1-hsiangkao%40linux.alibaba.com
> Fixes: 3aa8ec716e52 ("staging: erofs: add directory operations")
> Fixes: 33bac912840f ("staging: erofs: keep corrupted fs from crashing kernel in erofs_readdir()")
> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> Reported-by: Junrui Luo <moonafterrain@outlook.com>
> Closes: https://lore.kernel.org/r/A0FD7E0F-7558-49B0-8BC8-EB1ECDB2479A@outlook.com
> Cc: stable@vger.kernel.org
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
> v3:
>   - Disallow unaligned nameoff0 to avoid petential oob reads as well.
> 
>   fs/erofs/dir.c | 29 ++++++++++++++++-------------
>   1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
> index e5132575b9d3..d074fded1577 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;
> -		}
> +		/* a corrupted entry is found (including negative namelen) */
> +		if (!in_range32(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)
> @@ -88,7 +90,8 @@ static int erofs_readdir(struct file *f, struct dir_context *ctx)
>   		}
>   
>   		nameoff = le16_to_cpu(de->nameoff);
> -		if (nameoff < sizeof(struct erofs_dirent) || nameoff >= bsz) {

You mean?

if (!nameoff || nameoff >= bsz || nameoff % sizeof(struct erofs_dirent))

Thanks,

> +		if (nameoff < sizeof(struct erofs_dirent) || nameoff >= bsz ||
> +		    (nameoff % sizeof(struct erofs_dirent))) {
>   			erofs_err(sb, "invalid de[0].nameoff %u @ nid %llu",
>   				  nameoff, EROFS_I(dir)->nid);
>   			err = -EFSCORRUPTED;



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] erofs: fix the out-of-bounds nameoff handling for trailing dirents
  2026-04-21  7:26     ` Chao Yu
@ 2026-04-21  7:38       ` Gao Xiang
  2026-04-21  8:31         ` Chao Yu
  2026-04-21  7:59       ` [PATCH v4] " Gao Xiang
  1 sibling, 1 reply; 8+ messages in thread
From: Gao Xiang @ 2026-04-21  7:38 UTC (permalink / raw)
  To: Chao Yu, linux-erofs; +Cc: LKML, Yuhao Jiang, Junrui Luo, stable



On 2026/4/21 15:26, Chao Yu wrote:
> On 4/16/2026 5:44 PM, Gao Xiang wrote:
>> Currently we already have boundary-checks for nameoffs, but the trailing
>> dirents are special since the namelens are calculated with strnlen()
>> with unchecked nameoffs.
>>
>> If a crafted EROFS has a trailing dirent with nameoff >= maxsize,
>> maxsize - nameoff can underflow, causing strnlen() to read past the
>> directory block.
>>
>> nameoff0 should also be verified to be a multiple of
>> `sizeof(struct erofs_dirent)` as well [1].
>>
>> [1] https://sashiko.dev/#/patchset/20260416063511.3173774-1-hsiangkao%40linux.alibaba.com
>> Fixes: 3aa8ec716e52 ("staging: erofs: add directory operations")
>> Fixes: 33bac912840f ("staging: erofs: keep corrupted fs from crashing kernel in erofs_readdir()")
>> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
>> Reported-by: Junrui Luo <moonafterrain@outlook.com>
>> Closes: https://lore.kernel.org/r/A0FD7E0F-7558-49B0-8BC8-EB1ECDB2479A@outlook.com
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
>> ---
>> v3:
>>   - Disallow unaligned nameoff0 to avoid petential oob reads as well.
>>
>>   fs/erofs/dir.c | 29 ++++++++++++++++-------------
>>   1 file changed, 16 insertions(+), 13 deletions(-)
>>
>> diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
>> index e5132575b9d3..d074fded1577 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;
>> -        }
>> +        /* a corrupted entry is found (including negative namelen) */
>> +        if (!in_range32(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)
>> @@ -88,7 +90,8 @@ static int erofs_readdir(struct file *f, struct dir_context *ctx)
>>           }
>>           nameoff = le16_to_cpu(de->nameoff);
>> -        if (nameoff < sizeof(struct erofs_dirent) || nameoff >= bsz) {
> 
> You mean?
> 
> if (!nameoff || nameoff >= bsz || nameoff % sizeof(struct erofs_dirent))

The explanation can be seen as:
https://sashiko.dev/#/patchset/20260416063511.3173774-1-hsiangkao%40linux.alibaba.com

But I think `nameoff < sizeof(struct erofs_dirent)` is also fine?
I could also switch to your suggested version.

Thanks,
Gao Xiang



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4] erofs: fix the out-of-bounds nameoff handling for trailing dirents
  2026-04-21  7:26     ` Chao Yu
  2026-04-21  7:38       ` Gao Xiang
@ 2026-04-21  7:59       ` Gao Xiang
  2026-04-21  8:32         ` Chao Yu
  1 sibling, 1 reply; 8+ messages in thread
From: Gao Xiang @ 2026-04-21  7:59 UTC (permalink / raw)
  To: linux-erofs, Chao Yu
  Cc: LKML, oliver.yang, Gao Xiang, Yuhao Jiang, Junrui Luo, stable

Currently we already have boundary-checks for nameoffs, but the trailing
dirents are special since the namelens are calculated with strnlen()
with unchecked nameoffs.

If a crafted EROFS has a trailing dirent with nameoff >= maxsize,
maxsize - nameoff can underflow, causing strnlen() to read past the
directory block.

nameoff0 should also be verified to be a multiple of
`sizeof(struct erofs_dirent)` as well [1].

[1] https://sashiko.dev/#/patchset/20260416063511.3173774-1-hsiangkao%40linux.alibaba.com

Fixes: 3aa8ec716e52 ("staging: erofs: add directory operations")
Fixes: 33bac912840f ("staging: erofs: keep corrupted fs from crashing kernel in erofs_readdir()")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Reported-by: Junrui Luo <moonafterrain@outlook.com>
Closes: https://lore.kernel.org/r/A0FD7E0F-7558-49B0-8BC8-EB1ECDB2479A@outlook.com
Cc: stable@vger.kernel.org
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
---
v4:
 - switch to `if (!nameoff || nameoff >= bsz || (nameoff % sizeof(*de)))`
   as suggested by Chao.

 fs/erofs/dir.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
index e5132575b9d3..4aa52a5f204a 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;
-		}
+		/* a corrupted entry is found (including negative namelen) */
+		if (!in_range32(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)
@@ -88,7 +90,7 @@ static int erofs_readdir(struct file *f, struct dir_context *ctx)
 		}
 
 		nameoff = le16_to_cpu(de->nameoff);
-		if (nameoff < sizeof(struct erofs_dirent) || nameoff >= bsz) {
+		if (!nameoff || nameoff >= bsz || (nameoff % sizeof(*de))) {
 			erofs_err(sb, "invalid de[0].nameoff %u @ nid %llu",
 				  nameoff, EROFS_I(dir)->nid);
 			err = -EFSCORRUPTED;
-- 
2.43.5



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] erofs: fix the out-of-bounds nameoff handling for trailing dirents
  2026-04-21  7:38       ` Gao Xiang
@ 2026-04-21  8:31         ` Chao Yu
  0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2026-04-21  8:31 UTC (permalink / raw)
  To: Gao Xiang, linux-erofs; +Cc: chao, LKML, Yuhao Jiang, Junrui Luo, stable

On 4/21/2026 3:38 PM, Gao Xiang wrote:
> 
> 
> On 2026/4/21 15:26, Chao Yu wrote:
>> On 4/16/2026 5:44 PM, Gao Xiang wrote:
>>> Currently we already have boundary-checks for nameoffs, but the trailing
>>> dirents are special since the namelens are calculated with strnlen()
>>> with unchecked nameoffs.
>>>
>>> If a crafted EROFS has a trailing dirent with nameoff >= maxsize,
>>> maxsize - nameoff can underflow, causing strnlen() to read past the
>>> directory block.
>>>
>>> nameoff0 should also be verified to be a multiple of
>>> `sizeof(struct erofs_dirent)` as well [1].
>>>
>>> [1] https://sashiko.dev/#/patchset/20260416063511.3173774-1-hsiangkao%40linux.alibaba.com
>>> Fixes: 3aa8ec716e52 ("staging: erofs: add directory operations")
>>> Fixes: 33bac912840f ("staging: erofs: keep corrupted fs from crashing kernel in erofs_readdir()")
>>> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
>>> Reported-by: Junrui Luo <moonafterrain@outlook.com>
>>> Closes: https://lore.kernel.org/r/A0FD7E0F-7558-49B0-8BC8-EB1ECDB2479A@outlook.com
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
>>> ---
>>> v3:
>>>    - Disallow unaligned nameoff0 to avoid petential oob reads as well.
>>>
>>>    fs/erofs/dir.c | 29 ++++++++++++++++-------------
>>>    1 file changed, 16 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/fs/erofs/dir.c b/fs/erofs/dir.c
>>> index e5132575b9d3..d074fded1577 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;
>>> -        }
>>> +        /* a corrupted entry is found (including negative namelen) */
>>> +        if (!in_range32(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)
>>> @@ -88,7 +90,8 @@ static int erofs_readdir(struct file *f, struct dir_context *ctx)
>>>            }
>>>            nameoff = le16_to_cpu(de->nameoff);
>>> -        if (nameoff < sizeof(struct erofs_dirent) || nameoff >= bsz) {
>>
>> You mean?
>>
>> if (!nameoff || nameoff >= bsz || nameoff % sizeof(struct erofs_dirent))
> 
> The explanation can be seen as:
> https://sashiko.dev/#/patchset/20260416063511.3173774-1-hsiangkao%40linux.alibaba.com
> 
> But I think `nameoff < sizeof(struct erofs_dirent)` is also fine?

Yes, it's fine to use "nameoff < sizeof(struct erofs_dirent)", it's a minor
cleanup to use "!nameof".

Thanks,

> I could also switch to your suggested version.
> 
> Thanks,
> Gao Xiang
> 



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v4] erofs: fix the out-of-bounds nameoff handling for trailing dirents
  2026-04-21  7:59       ` [PATCH v4] " Gao Xiang
@ 2026-04-21  8:32         ` Chao Yu
  0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2026-04-21  8:32 UTC (permalink / raw)
  To: Gao Xiang, linux-erofs
  Cc: chao, LKML, oliver.yang, Yuhao Jiang, Junrui Luo, stable

On 4/21/2026 3:59 PM, Gao Xiang wrote:
> Currently we already have boundary-checks for nameoffs, but the trailing
> dirents are special since the namelens are calculated with strnlen()
> with unchecked nameoffs.
> 
> If a crafted EROFS has a trailing dirent with nameoff >= maxsize,
> maxsize - nameoff can underflow, causing strnlen() to read past the
> directory block.
> 
> nameoff0 should also be verified to be a multiple of
> `sizeof(struct erofs_dirent)` as well [1].
> 
> [1] https://sashiko.dev/#/patchset/20260416063511.3173774-1-hsiangkao%40linux.alibaba.com
> 
> Fixes: 3aa8ec716e52 ("staging: erofs: add directory operations")
> Fixes: 33bac912840f ("staging: erofs: keep corrupted fs from crashing kernel in erofs_readdir()")
> Reported-by: Yuhao Jiang <danisjiang@gmail.com>
> Reported-by: Junrui Luo <moonafterrain@outlook.com>
> Closes: https://lore.kernel.org/r/A0FD7E0F-7558-49B0-8BC8-EB1ECDB2479A@outlook.com
> Cc: stable@vger.kernel.org
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-04-21  8:32 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-16  6:03 [PATCH] erofs: fix the out-of-bounds nameoff handling for trailing dirents Gao Xiang
2026-04-16  6:35 ` [PATCH v2] " Gao Xiang
2026-04-16  9:44   ` [PATCH v3] " Gao Xiang
2026-04-21  7:26     ` Chao Yu
2026-04-21  7:38       ` Gao Xiang
2026-04-21  8:31         ` Chao Yu
2026-04-21  7:59       ` [PATCH v4] " Gao Xiang
2026-04-21  8:32         ` Chao Yu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox