From: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
To: "glaubitz@physik.fu-berlin.de" <glaubitz@physik.fu-berlin.de>,
"contact@gvernon.com" <contact@gvernon.com>,
"penguin-kernel@I-love.SAKURA.ne.jp"
<penguin-kernel@I-love.SAKURA.ne.jp>,
"slava@dubeyko.com" <slava@dubeyko.com>,
"frank.li@vivo.com" <frank.li@vivo.com>
Cc: "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"syzbot+97e301b4b82ae803d21b@syzkaller.appspotmail.com"
<syzbot+97e301b4b82ae803d21b@syzkaller.appspotmail.com>
Subject: RE: [PATCH v4] hfs: Validate CNIDs in hfs_read_inode
Date: Mon, 16 Mar 2026 21:50:14 +0000 [thread overview]
Message-ID: <4f6b37028269c7c4a40a58b57cd546a93dcbd7c2.camel@ibm.com> (raw)
In-Reply-To: <b332e546-e93f-4b79-a11c-695058d61fb1@I-love.SAKURA.ne.jp>
On Sat, 2026-03-14 at 15:35 +0900, Tetsuo Handa wrote:
> On 2026/03/14 3:40, Viacheslav Dubeyko wrote:
> > On Fri, 2026-03-13 at 20:03 +0900, Tetsuo Handa wrote:
> > > On 2026/03/13 8:13, Viacheslav Dubeyko wrote:
> > > > > And even after both changes are applied, my patch still makes sense
> > > > > because mount() operation still succeeds for cnid >= 16. :-)
> > > >
> > > > I don't follow how it could happen. Please, take a look here [1]:
> > >
> > > Do you remember that you said
> > >
> > > "Why do not localize the all checks in hfs_read_inode()?"
> > >
> >
> > If we still have not correct implementation of hfs_read_inode() flow, then we
> > can make it right. The patch is under review yet and it can be changed.
> >
>
> Did you read relevant commit in my patch's description? What you have been
> failing to understand is that the value filled in by hfs_bnode_read() can be
> corrupted, for the value is given from relevant offset of the file system image.
>
> > > > We request to find exactly HFS_ROOT_CNID. If root folder has another CNID, then
> > > > we simply cannot find the record for root folder in Catalog File. So, we never
> > > > will call hfs_iget() if we cannot find the record.
>
> Your assumption is absolutely wrong. We request to find HFS_ROOT_CNID, but
> there is no guarantee that the CNID value (the value of rec.dir.DirID if
> rec.type is HFS_CDR_DIR, or the value of rec->file.FlNum if rec->type is
> HFS_CDR_FIL) which is filled in by hfs_bnode_read() is actually HFS_ROOT_CNID,
> for the CNID value is nothing but 32bits read from relevant offset of the
> file system image.
>
> Since we are passing wrong CNID to hfs_iget(), we can't fix this problem
> by updating hfs_read_inode() flow. The correct fix for this report is
> "don't call hfs_iget() if CNID is wrong". Your patch can be applies as
> a further improvement rather than a fix for this report.
It means that, maybe, we need to add the logic of checking the extracted record
by hfs_cat_find_brec(). But I cannot imagine that this logic can extract the
record with incorrect CNID. Because, it is the main goal of hfs_cat_find_brec()
logic to extract the record that contains requested CNID. And if we requested
the HFS_ROOT_CNID, then this logic should return the record with exactly
requested CNID or return the error code if such record has not been found.
Thanks,
Slava.
next prev parent reply other threads:[~2026-03-16 21:50 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-11 21:13 [PATCH v4] hfs: Validate CNIDs in hfs_read_inode George Anthony Vernon
2026-03-12 10:45 ` Tetsuo Handa
2026-03-12 23:13 ` Viacheslav Dubeyko
2026-03-13 11:03 ` Tetsuo Handa
2026-03-13 18:40 ` Viacheslav Dubeyko
2026-03-14 6:35 ` Tetsuo Handa
2026-03-16 21:50 ` Viacheslav Dubeyko [this message]
2026-03-18 0:37 ` George Anthony Vernon
2026-03-18 2:02 ` Viacheslav Dubeyko
2026-03-18 22:49 ` George Anthony Vernon
2026-03-19 9:57 ` Tetsuo Handa
2026-03-19 12:26 ` George Vernon
2026-03-20 0:32 ` Tetsuo Handa
2026-03-18 10:42 ` Tetsuo Handa
2026-03-18 0:10 ` George Anthony Vernon
2026-03-18 8:16 ` Tetsuo Handa
2026-03-12 23:07 ` Viacheslav Dubeyko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4f6b37028269c7c4a40a58b57cd546a93dcbd7c2.camel@ibm.com \
--to=slava.dubeyko@ibm.com \
--cc=contact@gvernon.com \
--cc=frank.li@vivo.com \
--cc=glaubitz@physik.fu-berlin.de \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=penguin-kernel@I-love.SAKURA.ne.jp \
--cc=slava@dubeyko.com \
--cc=syzbot+97e301b4b82ae803d21b@syzkaller.appspotmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox