public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
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.

  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