[PATCH] ntfs: Validate error in ntfs_lookup()

[PATCH] ntfs: Validate error in ntfs_lookup()

[Hongling Zeng]

Check that MREF_ERR returns non-zero before using as error pointer.
This prevents potential ERR_PTR(0) when error code is zero

Fixes: af0db57d4293 ("ntfs: update inode operations")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 fs/ntfs/namei.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
index 10894de519c3..bb075aa97b53 100644
--- a/fs/ntfs/namei.c
+++ b/fs/ntfs/namei.c
@@ -236,7 +236,7 @@ static struct dentry *ntfs_lookup(struct inode *dir_ino, struct dentry *dent,
 	}
 	ntfs_error(vol->sb, "ntfs_lookup_ino_by_name() failed with error code %i.",
 			-MREF_ERR(mref));
-	return ERR_PTR(MREF_ERR(mref));
+	return MREF_ERR(mref) ? ERR_PTR(MREF_ERR(mref)) : NULL;
 handle_name:
 	{
 		struct mft_record *m;
-- 
2.25.1

Re: [PATCH] ntfs: Validate error in ntfs_lookup()

[CharSyam]

Hi, Hongling.

 I don't think returning NULL is the right fallback here. This branch is
already the IS_ERR_MREF(mref) path, and -ENOENT has been handled above as
the negative-dentry case. If MREF_ERR(mref) ever decodes to 0 here, it
should probably remain an error, e.g. -EIO, rather than being converted
to a successful lookup return.

  Also, I do not see a current producer for MREF_ERR(mref) == 0:
ntfs_lookup_inode_by_name() normalizes zero err to -EIO before
ERR_MREF(err).

The Fixes tag also seems wrong, since the same return is
already present in af0db57d4293^.

Thanks.
DaeMyung.

2026년 5월 20일 (수) 오후 8:16, Hongling Zeng <zenghongling@kylinos.cn>님이 작성:
>
> Check that MREF_ERR returns non-zero before using as error pointer.
> This prevents potential ERR_PTR(0) when error code is zero
>
> Fixes: af0db57d4293 ("ntfs: update inode operations")
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>  fs/ntfs/namei.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
> index 10894de519c3..bb075aa97b53 100644
> --- a/fs/ntfs/namei.c
> +++ b/fs/ntfs/namei.c
> @@ -236,7 +236,7 @@ static struct dentry *ntfs_lookup(struct inode *dir_ino, struct dentry *dent,
>         }
>         ntfs_error(vol->sb, "ntfs_lookup_ino_by_name() failed with error code %i.",
>                         -MREF_ERR(mref));
> -       return ERR_PTR(MREF_ERR(mref));
> +       return MREF_ERR(mref) ? ERR_PTR(MREF_ERR(mref)) : NULL;
>  handle_name:
>         {
>                 struct mft_record *m;
> --
> 2.25.1
>
>

Re: [PATCH] ntfs: Validate error in ntfs_lookup()

[Hongling Zeng]

   Hi, DaeMyung.
   Thank you for the detailed review. You are absolutely right.

   After looking at the code more carefully, I agree that:
   1. ntfs_lookup_inode_by_name() already normalizes zero err to -EIO
      before ERR_MREF(err), so MREF_ERR(mref) will never be 0 here.
   2. Even if it were 0, returning NULL would be incorrect since this
      is an error path.

   The correct fix would be:
       int err = MREF_ERR(mref);
       return ERR_PTR(err ?: -EIO);

   This ensures we return a proper error code instead of masking
   the bug as success.
   Should I submit a new version with this fix, or just drop this
   patch entirely since the issue doesn't actually exist in practice?

   Thanks for catching this.
   Best regards,
Hongling


在 2026年05月20日 23:10, CharSyam 写道:
> Hi, Hongling.
>
>   I don't think returning NULL is the right fallback here. This branch is
> already the IS_ERR_MREF(mref) path, and -ENOENT has been handled above as
> the negative-dentry case. If MREF_ERR(mref) ever decodes to 0 here, it
> should probably remain an error, e.g. -EIO, rather than being converted
> to a successful lookup return.
>
>    Also, I do not see a current producer for MREF_ERR(mref) == 0:
> ntfs_lookup_inode_by_name() normalizes zero err to -EIO before
> ERR_MREF(err).
>
> The Fixes tag also seems wrong, since the same return is
> already present in af0db57d4293^.
>
> Thanks.
> DaeMyung.
>
> 2026년 5월 20일 (수) 오후 8:16, Hongling Zeng <zenghongling@kylinos.cn>님이 작성:
>> Check that MREF_ERR returns non-zero before using as error pointer.
>> This prevents potential ERR_PTR(0) when error code is zero
>>
>> Fixes: af0db57d4293 ("ntfs: update inode operations")
>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>> ---
>>   fs/ntfs/namei.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
>> index 10894de519c3..bb075aa97b53 100644
>> --- a/fs/ntfs/namei.c
>> +++ b/fs/ntfs/namei.c
>> @@ -236,7 +236,7 @@ static struct dentry *ntfs_lookup(struct inode *dir_ino, struct dentry *dent,
>>          }
>>          ntfs_error(vol->sb, "ntfs_lookup_ino_by_name() failed with error code %i.",
>>                          -MREF_ERR(mref));
>> -       return ERR_PTR(MREF_ERR(mref));
>> +       return MREF_ERR(mref) ? ERR_PTR(MREF_ERR(mref)) : NULL;
>>   handle_name:
>>          {
>>                  struct mft_record *m;
>> --
>> 2.25.1
>>
>>

Re: [PATCH] ntfs: Validate error in ntfs_lookup()

[CharSyam]

Hi, Hongling.

My impression is that dropping the patch may be the cleaner option here,
unless we can identify a real path that produces ERR_MREF(0).

Thanks.
DaeMyung.

2026년 5월 21일 (목) 오전 10:31, Hongling Zeng <zhongling0719@126.com>님이 작성:
>
>    Hi, DaeMyung.
>    Thank you for the detailed review. You are absolutely right.
>
>    After looking at the code more carefully, I agree that:
>    1. ntfs_lookup_inode_by_name() already normalizes zero err to -EIO
>       before ERR_MREF(err), so MREF_ERR(mref) will never be 0 here.
>    2. Even if it were 0, returning NULL would be incorrect since this
>       is an error path.
>
>    The correct fix would be:
>        int err = MREF_ERR(mref);
>        return ERR_PTR(err ?: -EIO);
>
>    This ensures we return a proper error code instead of masking
>    the bug as success.
>    Should I submit a new version with this fix, or just drop this
>    patch entirely since the issue doesn't actually exist in practice?
>
>    Thanks for catching this.
>    Best regards,
> Hongling
>
>
> 在 2026年05月20日 23:10, CharSyam 写道:
> > Hi, Hongling.
> >
> >   I don't think returning NULL is the right fallback here. This branch is
> > already the IS_ERR_MREF(mref) path, and -ENOENT has been handled above as
> > the negative-dentry case. If MREF_ERR(mref) ever decodes to 0 here, it
> > should probably remain an error, e.g. -EIO, rather than being converted
> > to a successful lookup return.
> >
> >    Also, I do not see a current producer for MREF_ERR(mref) == 0:
> > ntfs_lookup_inode_by_name() normalizes zero err to -EIO before
> > ERR_MREF(err).
> >
> > The Fixes tag also seems wrong, since the same return is
> > already present in af0db57d4293^.
> >
> > Thanks.
> > DaeMyung.
> >
> > 2026년 5월 20일 (수) 오후 8:16, Hongling Zeng <zenghongling@kylinos.cn>님이 작성:
> >> Check that MREF_ERR returns non-zero before using as error pointer.
> >> This prevents potential ERR_PTR(0) when error code is zero
> >>
> >> Fixes: af0db57d4293 ("ntfs: update inode operations")
> >> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> >> ---
> >>   fs/ntfs/namei.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
> >> index 10894de519c3..bb075aa97b53 100644
> >> --- a/fs/ntfs/namei.c
> >> +++ b/fs/ntfs/namei.c
> >> @@ -236,7 +236,7 @@ static struct dentry *ntfs_lookup(struct inode *dir_ino, struct dentry *dent,
> >>          }
> >>          ntfs_error(vol->sb, "ntfs_lookup_ino_by_name() failed with error code %i.",
> >>                          -MREF_ERR(mref));
> >> -       return ERR_PTR(MREF_ERR(mref));
> >> +       return MREF_ERR(mref) ? ERR_PTR(MREF_ERR(mref)) : NULL;
> >>   handle_name:
> >>          {
> >>                  struct mft_record *m;
> >> --
> >> 2.25.1
> >>
> >>
>

Re: [PATCH] ntfs: Validate error in ntfs_lookup()

[Hongling Zeng]

   Hi, DaeMyung.

   I agree. Dropping this patch is the cleaner option since
   the issue doesn't exist in practice. I've verified that
   ntfs_lookup_inode_by_name() normalizes zero err to -EIO,
   so MREF_ERR(mref) can never be 0 here.

   Thank you for the thorough analysis.

   Best regards,
   Hongling


在 2026年05月21日 23:00, CharSyam 写道:
> Hi, Hongling.
>
> My impression is that dropping the patch may be the cleaner option here,
> unless we can identify a real path that produces ERR_MREF(0).
>
> Thanks.
> DaeMyung.
>
> 2026년 5월 21일 (목) 오전 10:31, Hongling Zeng <zhongling0719@126.com>님이 작성:
>>     Hi, DaeMyung.
>>     Thank you for the detailed review. You are absolutely right.
>>
>>     After looking at the code more carefully, I agree that:
>>     1. ntfs_lookup_inode_by_name() already normalizes zero err to -EIO
>>        before ERR_MREF(err), so MREF_ERR(mref) will never be 0 here.
>>     2. Even if it were 0, returning NULL would be incorrect since this
>>        is an error path.
>>
>>     The correct fix would be:
>>         int err = MREF_ERR(mref);
>>         return ERR_PTR(err ?: -EIO);
>>
>>     This ensures we return a proper error code instead of masking
>>     the bug as success.
>>     Should I submit a new version with this fix, or just drop this
>>     patch entirely since the issue doesn't actually exist in practice?
>>
>>     Thanks for catching this.
>>     Best regards,
>> Hongling
>>
>>
>> 在 2026年05月20日 23:10, CharSyam 写道:
>>> Hi, Hongling.
>>>
>>>    I don't think returning NULL is the right fallback here. This branch is
>>> already the IS_ERR_MREF(mref) path, and -ENOENT has been handled above as
>>> the negative-dentry case. If MREF_ERR(mref) ever decodes to 0 here, it
>>> should probably remain an error, e.g. -EIO, rather than being converted
>>> to a successful lookup return.
>>>
>>>     Also, I do not see a current producer for MREF_ERR(mref) == 0:
>>> ntfs_lookup_inode_by_name() normalizes zero err to -EIO before
>>> ERR_MREF(err).
>>>
>>> The Fixes tag also seems wrong, since the same return is
>>> already present in af0db57d4293^.
>>>
>>> Thanks.
>>> DaeMyung.
>>>
>>> 2026년 5월 20일 (수) 오후 8:16, Hongling Zeng <zenghongling@kylinos.cn>님이 작성:
>>>> Check that MREF_ERR returns non-zero before using as error pointer.
>>>> This prevents potential ERR_PTR(0) when error code is zero
>>>>
>>>> Fixes: af0db57d4293 ("ntfs: update inode operations")
>>>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>>>> ---
>>>>    fs/ntfs/namei.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
>>>> index 10894de519c3..bb075aa97b53 100644
>>>> --- a/fs/ntfs/namei.c
>>>> +++ b/fs/ntfs/namei.c
>>>> @@ -236,7 +236,7 @@ static struct dentry *ntfs_lookup(struct inode *dir_ino, struct dentry *dent,
>>>>           }
>>>>           ntfs_error(vol->sb, "ntfs_lookup_ino_by_name() failed with error code %i.",
>>>>                           -MREF_ERR(mref));
>>>> -       return ERR_PTR(MREF_ERR(mref));
>>>> +       return MREF_ERR(mref) ? ERR_PTR(MREF_ERR(mref)) : NULL;
>>>>    handle_name:
>>>>           {
>>>>                   struct mft_record *m;
>>>> --
>>>> 2.25.1
>>>>
>>>>

[PATCH] ntfs: Validate error in ntfs_lookup()

[Hongling Zeng]

Check that MREF_ERR returns non-zero before using as error pointer.
This prevents potential ERR_PTR(0) when error code is zero

Fixes: af0db57d4293 ("ntfs: update inode operations")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 fs/ntfs/namei.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
index 10894de519c3..b72839575fa8 100644
--- a/fs/ntfs/namei.c
+++ b/fs/ntfs/namei.c
@@ -236,7 +236,9 @@ static struct dentry *ntfs_lookup(struct inode *dir_ino, struct dentry *dent,
 	}
 	ntfs_error(vol->sb, "ntfs_lookup_ino_by_name() failed with error code %i.",
 			-MREF_ERR(mref));
-	return ERR_PTR(MREF_ERR(mref));
+	if (MREF_ERR(mref))
+		return ERR_PTR(MREF_ERR(mref));
+	return NULL;
 handle_name:
 	{
 		struct mft_record *m;
-- 
2.25.1