public inbox for linux-kernel-mentees@lists.linux-foundation.org
 help / color / mirror / Atom feed
From: George Anthony Vernon <contact@gvernon.com>
To: slava@dubeyko.com, glaubitz@physik.fu-berlin.de,
	frank.li@vivo.com, linux-fsdevel@vger.kernel.org,
	skhan@linuxfoundation.org
Cc: George Anthony Vernon <contact@gvernon.com>,
	linux-kernel@vger.kernel.org,
	linux-kernel-mentees@lists.linux.dev,
	penguin-kernel@i-love.sakura.ne.jp
Subject: [PATCH v2 0/2] hfs: Validate CNIDs read from filesystem
Date: Tue,  4 Nov 2025 01:47:35 +0000	[thread overview]
Message-ID: <20251104014738.131872-2-contact@gvernon.com> (raw)
In-Reply-To: <d2b28f73-49c8-4e30-9913-01702da4dfe4@I-love.SAKURA.ne.jp>

Changes from V1->V2:
- is_valid_cnid prototype now takes u32 and u8 types
- Add fsck advice in dmesg
- Replace make_bad_inode calls in hfs_read_inode with gotos
- Reuse same check in hfs_write_inode
- Disallow HFS_POR_CNID, HFS_BAD_CNID, and HFS_EXCH_CNID
- Add Tetsuo's patch to mine and make it a patch series

This patch series contains two patches which work together to prevent
bad catalog records from being read. Previously it was possible for a
malformed HFS filesystem image to reach a BUG() in writeback. I propose
to fix this by verifying that CNIDs are allowed ones when constructing
an inode in hfs_read_inode(). Tetsuo Handa's additional check in
hfs_fill_super makes sure the root inode's CNID is correct, handling an
edge case where the records' directory CNID > 15.

I think we should continue returning BUG() in hfs_write_inode() because
it is a filesystem implementation error if we ever allow an inode to be
constructed from a bad CNID. Now we properly guard our reads in
hfs_read_inode(), records with bad CNIDs should not get so far as to
initialise inodes which are queued for writeback.

I'm suggesting to disallow HFS_POR_CNID because there is no 'real'
record with that CNID or corresponding file, it doesn't make sense to
present an inode for it to the VFS when it exists only as a dummy
reference in our internal btree. Similarly I'm suggesting to disallow
HFS_BAD_CNID and HFS_EXCH_CNID because not only are they for internal
use, but we also do not implement bad blocks or exchange file
functionality in the Linux driver.

Thank you to Slava and Tetsuo for the feedback on V1.

George Anthony Vernon (2):
  hfs: Validate CNIDs in hfs_read_inode
  hfs: Update sanity check of the root record

 fs/hfs/inode.c | 67 +++++++++++++++++++++++++++++++++++++++-----------
 fs/hfs/super.c |  2 +-
 2 files changed, 54 insertions(+), 15 deletions(-)

-- 
2.50.1


  reply	other threads:[~2025-11-04  1:48 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-03  2:45 [PATCH] hfs: Validate CNIDs in hfs_read_inode George Anthony Vernon
2025-10-03 22:40 ` Viacheslav Dubeyko
2025-10-04  1:25   ` George Anthony Vernon
2025-10-07 13:40     ` Viacheslav Dubeyko
2025-10-09 12:57       ` Tetsuo Handa
2025-10-29  3:20         ` George Anthony Vernon
2025-10-29 10:06           ` Tetsuo Handa
2025-11-04  1:47             ` George Anthony Vernon [this message]
2025-11-04  1:47             ` [PATCH v2 1/2] " George Anthony Vernon
2025-11-04 22:34               ` Viacheslav Dubeyko
2025-11-11  0:00                 ` George Anthony Vernon
2025-11-11  0:48                   ` Viacheslav Dubeyko
2025-11-24 22:33                     ` George Anthony Vernon
2025-11-25 19:02                       ` Viacheslav Dubeyko
2025-11-11 14:39               ` Tetsuo Handa
2025-11-11 22:42                 ` Viacheslav Dubeyko
2025-11-24 23:46                   ` George Anthony Vernon
2025-11-25 19:15                     ` Viacheslav Dubeyko
2025-11-30 10:07                     ` Tetsuo Handa
2026-01-06 10:21                       ` Tetsuo Handa
2025-11-04  1:47             ` [PATCH v2 2/2] hfs: Update sanity check of the root record George Anthony Vernon
2025-11-04 23:01               ` Viacheslav Dubeyko
2025-11-10 23:03                 ` George Anthony Vernon
2025-11-10 23:34                   ` Viacheslav Dubeyko
2025-11-11  0:23                     ` George Anthony Vernon
2025-11-11  0:34                       ` Viacheslav Dubeyko
2025-11-24 22:56                         ` George Anthony Vernon
2025-11-11 14:26                       ` Tetsuo Handa
2025-11-11 22:56                         ` Viacheslav Dubeyko
2025-11-14 14:18                           ` Tetsuo Handa
2025-11-14 21:00                             ` 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=20251104014738.131872-2-contact@gvernon.com \
    --to=contact@gvernon.com \
    --cc=frank.li@vivo.com \
    --cc=glaubitz@physik.fu-berlin.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel-mentees@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=skhan@linuxfoundation.org \
    --cc=slava@dubeyko.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