On Mon, 27 Feb 2023 21:33:03 +0100 Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> On Sun, Feb 26 2023 at 19:55, syzbot wrote:
> > ODEBUG: assert_init not available (active state 0) object: ffffffff8d4fcbc0 object type: timer_list hint: key_gc_timer_func+0x0/0x80 security/keys/gc.c:117
>
> > WARNING: CPU: 1 PID: 10646 at lib/debugobjects.c:512 debug_object_assert_init+0x1f2/0x240 lib/debugobjects.c:899
> > debug_assert_init kernel/time/timer.c:837 [inline]
> > __mod_timer+0x10d/0xf40 kernel/time/timer.c:1020
> > key_reject_and_link+0x3f5/0x6e0 security/keys/key.c:610
> > key_negate_and_link include/linux/key-type.h:187 [inline]
> > complete_request_key security/keys/request_key.c:64 [inline]
> > call_sbin_request_key+0xa7b/0xcd0 security/keys/request_key.c:213
> > construct_key security/keys/request_key.c:244 [inline]
> > construct_key_and_link security/keys/request_key.c:503 [inline]
> > request_key_and_link+0x11e3/0x18e0 security/keys/request_key.c:637
> > __do_sys_request_key security/keys/keyctl.c:222 [inline]
> > __se_sys_request_key+0x271/0x3b0 security/keys/keyctl.c:167
>
> This is odd. The timer object is statically allocated via
> DEFINE_TIMER(). That macro sets
>
> timer.entry.next = TIMER_ENTRY_STATIC
>
> which is used to detect statically allocated timer objects via
> timer_is_static_object() and that checks for:
>
> timer.entry.pprev == NULL && timer.entry.next == TIMER_ENTRY_STATIC
List operations like hlist_add_head() and __hlist_del() make
timer_is_static_object() return false positive result.
>
> The only function which touches key_gc_timer is
>
> key_reject_and_link()
> mod_timer()
> __mod_timer()
> debug_assert_init()
Commit d02e382cef06 ("timers: Silently ignore timers with a NULL function")
added this assert.
> debug_timer_assert_init()
> debug_object_assert_init()
> if (!lookup_object()) {
> if (!check_for_static_object()) <- Invokes timer_is_static_object()
> WARN()
cpu 0 cpu 2
--- ---
mod_timer() mod_timer()
__mod_timer() __mod_timer()
... ...
debug_object_assert_init()
raw_spin_lock_irqsave(&db->lock, flags);
if (!lookup_object()) {
raw_spin_unlock_irqrestore(&db->lock, flags);
raw_spin_lock_irqsave(&db->lock, flags);
if (!lookup_object()) {
raw_spin_unlock_irqrestore(&db->lock, flags);
if (check_for_static_object())
fine;
if (!check_for_static_object())
WARN;
Depending on TIMER_ENTRY_STATIC makes check_for_static_object() fragile as a
static timer is always static regardless if it is enqueued.
The fragility is another explanation.
>
> If this is the first invocation of mod_timer(&key_gc_timer,...) then
> key_gc_timer is corrupted.
>
> If this is not the first invocation of mod_timer(&key_gc_timer,...) then
> the debugobjects hash is corrupted.
>
> Either way neither the timer code nor debugobjects have been changed
> since the 6.2 release and certainly are innocent here.
>
> That smells like a nasty memory corruption issue and the two other
> syzbot reports which arrived in my filtered inbox:
>
> https://lore.kernel.org/all/000000000000d7894b05f5924787@xxxxxxxxxx
> https://lore.kernel.org/all/000000000000840dae05f5a7fb53@xxxxxxxxxx
>
> point to memory corruption as well.
>
> The first one has a C reproducer. Can that be used for bisection?
>
> Thanks,
>
> tglx
>
