linux-kernel - Re: [PATCH] libfs: fix implicitly cast in simple_attr_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-next>] [day] [month] [year] [list]

Message-ID:
 <BL0PR03MB41610A9302ADA6A5022A306BADF62@BL0PR03MB4161.namprd03.prod.outlook.com>
Date: Sat, 25 May 2024 19:55:52 +0000
From: Jiasheng Jiang <jiashengjiangcool@...look.com>
To: viro@...iv.linux.org.uk
Cc: brauner@...nel.org,
	jack@...e.cz,
	arnd@...db.de,
	gregkh@...e.de,
	linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Jiasheng Jiang <jiashengjiangcool@...look.com>
Subject: Re: [PATCH] libfs: fix implicitly cast in simple_attr_write_xsigned()

> On Wed, May 15, 2024 at 03:17:25PM +0000, Jiasheng Jiang wrote:
>> Return 0 to indicate failure and return "len" to indicate success.
>> It was hard to distinguish success or failure if "len" equals the error
>> code after the implicit cast.
>> Moreover, eliminating implicit cast is a better practice.
> 
> According to whom?
> 

Programmers can easily overlook implicit casts, leading to unknown
behavior (e.g., this bug).
Converting implicit casts to explicit casts can help prevent future
errors.

> Merits of your ex cathedra claims aside, you do realize that functions
> have calling conventions because they are, well, called, right?
> And changing the value returned in such and such case should be
> accompanied with the corresponding change in the _callers_.
> 
> Al, wondering if somebody had decided to play with LLM...

As the comment shows that "ret = len; /* on success, claim we got the
whole input */", the return value should be checked to determine whether
it equals "len".

Moreover, if "len" is 0, the previous copy_from_user() will fail and
return an error.
Therefore, 0 is an illegal value for "len". Besides, in the linux kernel,
all the callers of simple_attr_write_xsigned() return the return value of
simple_attr_write_xsigned().

-Jiasheng