linux-kernel - Re: [PATCH v4] hfs: update sanity check of 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: <98938e56-b404-4748-94bd-75c88415fafe@I-love.SAKURA.ne.jp>
Date: Sat, 2 Aug 2025 06:52:01 +0900
From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To: Viacheslav Dubeyko <Slava.Dubeyko@....com>,
        "leocstone@...il.com" <leocstone@...il.com>,
        "jack@...e.cz" <jack@...e.cz>,
        "willy@...radead.org" <willy@...radead.org>,
        "brauner@...nel.org" <brauner@...nel.org>
Cc: "glaubitz@...sik.fu-berlin.de" <glaubitz@...sik.fu-berlin.de>,
        "frank.li@...o.com" <frank.li@...o.com>,
        "slava@...eyko.com" <slava@...eyko.com>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        Alexander Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH v4] hfs: update sanity check of the root record

On 2025/08/02 3:26, Viacheslav Dubeyko wrote:
> On Fri, 2025-08-01 at 06:12 +0900, Tetsuo Handa wrote:
>> On 2025/08/01 3:03, Viacheslav Dubeyko wrote:
>>> On Thu, 2025-07-31 at 07:02 +0900, Tetsuo Handa wrote:
>>>> On 2025/07/31 4:24, Viacheslav Dubeyko wrote:
>>>>> If we considering case HFS_CDR_DIR in hfs_read_inode(), then we know that it
>>>>> could be HFS_POR_CNID, HFS_ROOT_CNID, or >= HFS_FIRSTUSER_CNID. Do you mean that
>>>>> HFS_POR_CNID could be a problem in hfs_write_inode()?
>>>>
>>>> Yes. Passing one of 1, 5 or 15 instead of 2 from hfs_fill_super() triggers BUG()
>>>> in hfs_write_inode(). We *MUST* validate at hfs_fill_super(), or hfs_read_inode()
>>>> shall have to also reject 1, 5 and 15 (and as a result only accept 2).
>>>
>>> The fix should be in hfs_read_inode(). Currently, suggested solution hides the
>>> issue but not fix the problem.
>>
>> Not fixing this problem might be hiding other issues, by hitting BUG() before
>> other issues shows up.
>>
> 
> I am not going to start a philosophical discussion. We simply need to fix the
> bug. The suggested patch doesn't fix the issue.

What is your issue?

My issue (what syzbot is reporting) is that the kernel crashes if the inode number
of the record retrieved as a result of hfs_cat_find_brec(HFS_ROOT_CNID) is not
HFS_ROOT_CNID. My suggested patch does fix my issue.

> Please, don't use hardcoded value. I already shared the point that we must use
> the declared constants.
> 
> This function is incorrect and it cannot work for folders and files at the same
> time.

I already shared that I don't plan to try writing such function
( http://lkml.kernel.org/r/38d8f48e-47c3-4d67-9caa-498f3b47004f@I-love.SAKURA.ne.jp ).

Please show us your patch that solves your issue.