linux-kernel - Re: [PATCH] fs/adfs: bigdir: Restore EIO errno return when checkbyte mismatch

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: <20250801192200.GR222315@ZenIV>
Date: Fri, 1 Aug 2025 20:22:00 +0100
From: Al Viro <viro@...iv.linux.org.uk>
To: Zhen Ni <zhen.ni@...ystack.cn>
Cc: rmk+kernel@...linux.org.uk, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] fs/adfs: bigdir: Restore EIO errno return when checkbyte
 mismatch

On Fri, Aug 01, 2025 at 05:38:06PM +0800, Zhen Ni wrote:
> Fix smatch warn:
> fs/adfs/dir_fplus.c:146 adfs_fplus_read() warn: missing error code 'ret'

Gyah...  You don't fix $TOOL warnings, you fix underlying bugs - or, in case
of $TOOL generating a false positive, making $TOOL to STFU.  At the very
least you need to tell which one it is.

In this case it was not a false positive, so you need an explanation of
what's wrong with the code in question ("it makes smatch to complain"
is *NOT* it).

What happens here is a failure exit where we do complain into dmesg,
but actually report success to the caller, since the eventual return
value (in 'ret') is 0 left behind by successful adfs_fplus_validate_tail()
in the previous test.

_IF_ that behaviour had been an intentional mitigation strategy,
it needs to be explicitly described as such; however, looking at the
history of that function it seems more likely that this is an analogue
of the bug in previous failure exit fixed in 587065dcac64 ("fs/adfs:
bigdir: Fix an error code in adfs_fplus_read()"), so it's probably
better to return an error, same as we do on the other failure exits.

As it is, commit message fails to explain anything other than "smatch
has been involved in catching this one".  If you want to mention that,
sure, but it's on the same level as "so-and-so told me this place
looks fishy" - by all means, credit the contribution.  Reported-by/
caught-by/actually quoting smatch warning somewhere in the message -
up to you, but that shouldn't be the entirety of commit message ;-)