• Thomas Gleixner's avatar
    debugobject: Prevent init race with static objects · 63a75969
    Thomas Gleixner authored
    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: 3ac7fe5a ("infrastructure to debug (dynamic) objects")
    Reported-by: syzbot+5093ba19745994288b53@syzkaller.appspotmail.com
    Debugged-by: default avatarSchspa Shi <schspa@gmail.com>
    Signed-off-by: default avatarThomas Gleixner <tglx@linutronix.de>
    Reviewed-by: default avatarStephen 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
    63a75969
debugobjects.c 36 KB