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 .
