debugobject: Prevent init race with static objects
authorThomas Gleixner <tglx@linutronix.de>
Wed, 12 Apr 2023 07:54:39 +0000 (09:54 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 11 May 2023 14:03:16 +0000 (23:03 +0900)
commitfc2b20c0921f33906d6c54ecaca7b46f2721c493
tree190a72e588365d6505bdde8635e56e68a0c740b5
parent1d1735c6fbfdf6e272e7d7cba67dfb75529fd83e
debugobject: Prevent init race with static objects

[ Upstream commit 63a759694eed61025713b3e14dd827c8548daadc ]

Statically initialized objects are usually not initialized via the init()
function of the subsystem. They are special cased and the subsystem
provides a function to validate whether an object which is not yet tracked
by debugobjects is statically initialized. This means the object is started
to be tracked on first use, e.g. activation.

This works perfectly fine, unless there are two concurrent operations on
that object. Schspa decoded the problem:

T0                      T1

debug_object_assert_init(addr)
  lock_hash_bucket()
  obj = lookup_object(addr);
  if (!obj) {
   unlock_hash_bucket();
- > preemption
            lock_subsytem_object(addr);
      activate_object(addr)
      lock_hash_bucket();
      obj = lookup_object(addr);
      if (!obj) {
     unlock_hash_bucket();
if (is_static_object(addr))
   init_and_track(addr);
      lock_hash_bucket();
      obj = lookup_object(addr);
      obj->state = ACTIVATED;
      unlock_hash_bucket();

    subsys function modifies content of addr,
    so static object detection does
    not longer work.

    unlock_subsytem_object(addr);

        if (is_static_object(addr)) <- Fails

  debugobject emits a warning and invokes the fixup function which
  reinitializes the already active object in the worst case.

This race exists forever, but was never observed until mod_timer() got a
debug_object_assert_init() added which is outside of the timer base lock
held section right at the beginning of the function to cover the lockless
early exit points too.

Rework the code so that the lookup, the static object check and the
tracking object association happens atomically under the hash bucket
lock. This prevents the issue completely as all callers are serialized on
the hash bucket lock and therefore cannot observe inconsistent state.

Fixes: 3ac7fe5a4aab ("infrastructure to debug (dynamic) objects")
Reported-by: syzbot+5093ba19745994288b53@syzkaller.appspotmail.com
Debugged-by: Schspa Shi <schspa@gmail.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Link: https://syzkaller.appspot.com/bug?id=22c8a5938eab640d1c6bcc0e3dc7be519d878462
Link: https://lore.kernel.org/lkml/20230303161906.831686-1-schspa@gmail.com
Link: https://lore.kernel.org/r/87zg7dzgao.ffs@tglx
Signed-off-by: Sasha Levin <sashal@kernel.org>
lib/debugobjects.c