[v1] fs/ntfs3: Add sanity check for file name

[PATCH] fs/ntfs3: Add sanity check for file name

Posted by Lizhi Xu 6 months, 2 weeks ago

The length of the file name should be smaller than the directory entry size.

Reported-by: syzbot+598057afa0f49e62bd23@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=598057afa0f49e62bd23
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
 fs/ntfs3/dir.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/ntfs3/dir.c b/fs/ntfs3/dir.c
index b6da80c69ca6..b31bc9cbfa35 100644
--- a/fs/ntfs3/dir.c
+++ b/fs/ntfs3/dir.c
@@ -304,6 +304,9 @@ static inline bool ntfs_dir_emit(struct ntfs_sb_info *sbi,
 	if (sbi->options->nohidden && (fname->dup.fa & FILE_ATTRIBUTE_HIDDEN))
 		return true;
 
+	if (fname->name_len > le16_to_cpu(e->size) - sizeof(struct NTFS_DE))
+		return true;
+
 	name_len = ntfs_utf16_to_nls(sbi, fname->name, fname->name_len, name,
 				     PATH_MAX);
 	if (name_len <= 0) {
-- 
2.43.0

Re: [PATCH] fs/ntfs3: Add sanity check for file name

Posted by Al Viro 6 months, 2 weeks ago

On Fri, Jun 06, 2025 at 11:51:24AM +0800, Lizhi Xu wrote:
> The length of the file name should be smaller than the directory entry size.
> 
> Reported-by: syzbot+598057afa0f49e62bd23@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=598057afa0f49e62bd23
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> ---
>  fs/ntfs3/dir.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/ntfs3/dir.c b/fs/ntfs3/dir.c
> index b6da80c69ca6..b31bc9cbfa35 100644
> --- a/fs/ntfs3/dir.c
> +++ b/fs/ntfs3/dir.c
> @@ -304,6 +304,9 @@ static inline bool ntfs_dir_emit(struct ntfs_sb_info *sbi,
>  	if (sbi->options->nohidden && (fname->dup.fa & FILE_ATTRIBUTE_HIDDEN))
>  		return true;
>  
> +	if (fname->name_len > le16_to_cpu(e->size) - sizeof(struct NTFS_DE))
> +		return true;

And if e->size happens to be e.g. 0?  Note that (unsigned short)0 - sizeof(whatever)
ends up being a large unsigned.

unsigned short gets promoted to int.  sizeof is size_t - whatever it is,
it's an unsigned integer type, with rank no lower than that of int.

Since we have the entire range of unsigned short representable by int on all
architectures we care about, we get unsigned short promoted to int (preserving
the value) and then to size_t (value taken modulo range of size_t, i.e.
the original unsigned short value preserved).  Incidentally, even on a target
where sizeof(unsigned short) == sizeof(int) we'd still get an unsigned result -
unsigned short would be promoted to unsigned int, and mix of two unsigned
integer types gets converted to whichever has the higher rank.

IOW, comparison in
	if (fname->name_len > le16_to_cpu(e->size) - sizeof(struct NTFS_DE))
is going to be an unsigned one.  AFAICS, fname->name_len is u8, so just
turn that check into
	if (sizeof(struct NTFS_DE) + fname->name_len > le16_to_cpu(e->size))
and be done with that - comparison is, again, unsigned, but there's no
possibility of wraparounds in that variant.

[PATCH V2] fs/ntfs3: Add sanity check for file name

Posted by Lizhi Xu 6 months, 2 weeks ago

The length of the file name should be smaller than the directory entry size.

Reported-by: syzbot+598057afa0f49e62bd23@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=598057afa0f49e62bd23
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
V1 -> V2: move sizeof to left

 fs/ntfs3/dir.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/ntfs3/dir.c b/fs/ntfs3/dir.c
index b6da80c69ca6..b31bc9cbfa35 100644
--- a/fs/ntfs3/dir.c
+++ b/fs/ntfs3/dir.c
@@ -304,6 +304,9 @@ static inline bool ntfs_dir_emit(struct ntfs_sb_info *sbi,
 	if (sbi->options->nohidden && (fname->dup.fa & FILE_ATTRIBUTE_HIDDEN))
 		return true;
 
+	if (fname->name_len + sizeof(struct NTFS_DE) > le16_to_cpu(e->size))
+		return true;
+
 	name_len = ntfs_utf16_to_nls(sbi, fname->name, fname->name_len, name,
 				     PATH_MAX);
 	if (name_len <= 0) {
-- 
2.43.0