linux-kernel - Re: [PATCH] hfs: Sanity check the root record

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241126093313.2t7nu67e6cjvbe7b@quack3>
Date: Tue, 26 Nov 2024 10:33:13 +0100
From: Jan Kara <jack@...e.cz>
To: Leo Stone <leocstone@...il.com>
Cc: syzbot+2db3c7526ba68f4ea776@...kaller.appspotmail.com,
	brauner@...nel.org, quic_jjohnson@...cinc.com, jack@...e.cz,
	viro@...iv.linux.org.uk, sandeen@...hat.com,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	syzkaller-bugs@...glegroups.com, shuah@...nel.org,
	anupnewsmail@...il.com,
	linux-kernel-mentees@...ts.linuxfoundation.org
Subject: Re: [PATCH] hfs: Sanity check the root record

On Sat 23-11-24 11:49:47, Leo Stone wrote:
> In the syzbot reproducer, the hfs_cat_rec for the root dir has type
> HFS_CDR_FIL after being read with hfs_bnode_read() in hfs_super_fill().
> This indicates it should be used as an hfs_cat_file, which is 102 bytes.
> Only the first 70 bytes of that struct are initialized, however,
> because the entrylength passed into hfs_bnode_read() is still the length of
> a directory record. This causes uninitialized values to be used later on,
> when the hfs_cat_rec union is treated as the larger hfs_cat_file struct.
> 
> Add a check to make sure the retrieved record has the correct type
> for the root directory (HFS_CDR_DIR).

This certainly won't hurt but shouldn't we also add some stricter checks
for entry length so that we know we've loaded enough data to have full info
about the root dir?

								Honza

> 
> Reported-by: syzbot+2db3c7526ba68f4ea776@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=2db3c7526ba68f4ea776
> Tested-by: syzbot+2db3c7526ba68f4ea776@...kaller.appspotmail.com
> Signed-off-by: Leo Stone <leocstone@...il.com>
> ---
>  fs/hfs/super.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/hfs/super.c b/fs/hfs/super.c
> index 3bee9b5dba5e..02d78992eefd 100644
> --- a/fs/hfs/super.c
> +++ b/fs/hfs/super.c
> @@ -354,6 +354,8 @@ static int hfs_fill_super(struct super_block *sb, struct fs_context *fc)
>  			goto bail_hfs_find;
>  		}
>  		hfs_bnode_read(fd.bnode, &rec, fd.entryoffset, fd.entrylength);
> +		if (rec.type != HFS_CDR_DIR)
> +			res = -EIO;
>  	}
>  	if (res)
>  		goto bail_hfs_find;
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR