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@xxxxxxx>님이 작성:
>
>    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@xxxxxxxxxx>님이 작성:
> >> 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@xxxxxxxxxx>
> >> ---
> >>   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
> >>
> >>
>