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