linux-kernel - Re: [syzbot] [kernel?] KCSAN: data-race in kobject_put / kobject_uevent

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] [day] [month] [year] [list]

Message-ID: <bef0612c-df79-4478-8704-68d07cff46b5@I-love.SAKURA.ne.jp>
Date: Thu, 18 Jul 2024 22:34:23 +0900
From: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To: Greg KH <gregkh@...uxfoundation.org>, Marco Elver <elver@...gle.com>
Cc: syzbot <syzbot+0ac8e4da6d5cfcc7743e@...kaller.appspotmail.com>,
        linux-kernel@...r.kernel.org, rafael@...nel.org,
        syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] [kernel?] KCSAN: data-race in kobject_put /
 kobject_uevent_env (2)

On 2024/07/18 20:09, Greg KH wrote:
> On Thu, Jul 18, 2024 at 12:38:05PM +0200, Marco Elver wrote:
>> The two racing accesses here (on bitfield variables
>> kobj->state_remove_uevent_sent, kobj->state_initialized) are in the
>> same bitfield. There's no guarantee (by the compiler) that while the
>> bitfield is being updated the bit at kobj->state_initialized will
>> remain non-zero, and therefore the WARN in kobject_put() could be
>> triggered. This appears benign, unless of course someone set
>> panic_on_warn.
>>
>> More generally, if the bitfield is updated concurrently, it's very
>> likely that one of the updates would be lost.
>>
>> Just my initial observation.
> 
> Thanks for the review, I'll try to carve out some time next week to
> knock up a patch for this...

Unless it is proven that these

	unsigned int state_initialized:1;
	unsigned int state_in_sysfs:1;
	unsigned int state_add_uevent_sent:1;
	unsigned int state_remove_uevent_sent:1;
	unsigned int uevent_suppress:1;

fields are never modified concurrently, just marking this "unsigned int" as
__data_racy is wrong.

When we hit unprotected bitmask modification bug at
https://lkml.kernel.org/r/201409192053.IHJ35462.JLOMOSOFFVtQFH@I-love.SAKURA.ne.jp ,
we fixed the race by converting to test_bit()/set_bit()/clear_bit().