Re: [PATCH] hfsplus: Verify inode mode when loading from disk

[Viacheslav Dubeyko <slava@dubeyko.com>]

On Sat, 2025-11-15 at 00:35 +0900, Tetsuo Handa wrote:
> Ping?
> 

You checks the inode->i_mode in the patch. And logical place for this
check is hfsplus_get_perms() method [1] because all mode checks are
made there before assigning to inode->i_mode. So, you simply need to
add this check there and to check the return value of
hfsplus_get_perms() in hfsplus_cat_read_inode().

static void hfsplus_get_perms(struct inode *inode,
		struct hfsplus_perm *perms, int dir)
{
	struct hfsplus_sb_info *sbi = HFSPLUS_SB(inode->i_sb);
	u16 mode;

	mode = be16_to_cpu(perms->mode);

<skipped>

	if (dir) {
		mode = mode ? (mode & S_IALLUGO) : (S_IRWXUGO & ~(sbi-
>umask));
		mode |= S_IFDIR;
	} else if (!mode)
		mode = S_IFREG | ((S_IRUGO|S_IWUGO) & ~(sbi->umask));

<-- You simply can implement your check here.

	inode->i_mode = mode;

<skipped>
}

It simple modification from my point of view. And I don't quite follow
to your difficulties to move your check into hfsplus_get_perms().

Thanks,
Slava.

[1]
https://elixir.bootlin.com/linux/v6.18-rc5/source/fs/hfsplus/inode.c#L183

> Now that BFS got
> https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git/commit/?h=vfs.fixes&id=34ab4c75588c07cca12884f2bf6b0347c7a13872
>  ,
> HFS+ became the last filesystem that has not come to an answer.
> 
> On 2025/10/08 20:21, Tetsuo Handa wrote:
> > > As far as I can see, we operate by inode->i_mode here. But if
> > > inode
> > > mode has been corrupted on disk, then we assigned wrong value
> > > before.
> > > And HFS+ has hfsplus_get_perms() method that assigns perms->mode
> > > to
> > > inode->i_mode. So, I think we need to rework hfsplus_get_perms()
> > > for
> > > checking the correctness of inode mode before assigning it to
> > > inode->
> > > i_mode.
> > 
> > Then, can you give us an authoritative explanation, with historical
> > part
> > fully understood?