linux-kernel - Re: [PATCH v3] hfs: remove BUG() from hfs_release_folio()/hfs_test_inode()/hfs_write

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: <e6f71dcc-80e0-4cfe-91cf-8adb4d4effb7@I-love.SAKURA.ne.jp>
Date: Sat, 26 Jul 2025 07:22:04 +0900
From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To: Viacheslav Dubeyko <Slava.Dubeyko@....com>,
        "willy@...radead.org" <willy@...radead.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>
Subject: Re: [PATCH v3] hfs: remove BUG() from
 hfs_release_folio()/hfs_test_inode()/hfs_write_inode()

On 2025/07/26 2:42, Viacheslav Dubeyko wrote:
>>>
>>> I don't see any sense to introduce flags here. First of all, please, don't use
>>> hardcoded values but you should use declared constants from hfs.h (for example,
>>> HFS_EXT_CNID instead of 3). Secondly, you can simply compare the i_ino with
>>> constants, for example:
>>
>> This will save a lot of computational power compared to switch().
>>
> 
> Even if you would like to use flags, then the logic must to be simple and
> understandable. You still can use special inline function and do not create a
> mess in hfs_read_inode(). Especially, you can declare the mask one time in
> header, for example, but not to prepare the bad_cnid_list for every function
> call. Currently, the code looks really messy.

No, since this is "static const u16", the compiler will prepare it at build time
than every function call. Also since it is a simple u16, the compiler would
generate simple code like

  test ax, an_imm16_constant_value_determined_at_build_time
  jnz somewhere

which is much faster than

  cmp eax, HFS_EXT_CNID
  je somewhere
  cmp eax, other_cnid_1
  je somewhere
  cmp eax, other_cnid_2
  je somewhere
  cmp eax, other_cnid_3
  je somewhere
  cmp eax, other_cnid_4
  je somewhere

based on switch() in is_inode_id_invalid() shown below.

We can replace "static const u16" with "#define" if you prefer.

> 
>>>
>>> bool is_inode_id_invalid(u64 ino) {
>>>       switch (inode->i_ino) {
>>>       case HFS_EXT_CNID:
>>>       ...
>>>           return true;
>>>
>>>       }
>>>
>>>       return false;
>>> }



> So, 1, 2, 5, 15, etc can be accepted by hfs_read_inode().
> 0, 3, 4, 6, 7, 8, 9, 10, 11, 12, 13, 14 is invalid values for hfs_read_inode().

OK. This list will be useful for hardening, but we can't use this list for fixing
the bug reported at https://syzkaller.appspot.com/bug?extid=97e301b4b82ae803d21b .