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

[Hongling Zeng <zhongling0719@126.com>]

   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
>>>>
>>>>