debugobjects: insulate non-fixup logic related to static obj from fixup callbacks

When activating a static object we need make sure that the object is
tracked in the object tracker.  If it is a non-static object then the
activation is illegal.

In previous implementation, each subsystem need take care of this in
their fixup callbacks.  Actually we can put it into debugobjects core.
Thus we can save duplicated code, and have *pure* fixup callbacks.

To achieve this, a new callback "is_static_object" is introduced to let
the type specific code decide whether a object is static or not.  If
yes, we take it into object tracker, otherwise give warning and invoke
fixup callback.

This change has paassed debugobjects selftest, and I also do some test
with all debugobjects supports enabled.

At last, I have a concern about the fixups that can it change the object
which is in incorrect state on fixup? Because the 'addr' may not point
to any valid object if a non-static object is not tracked.  Then Change
such object can overwrite someone's memory and cause unexpected
behaviour.  For example, the timer_fixup_activate bind timer to function
stub_timer.

Link: http://lkml.kernel.org/r/1462576157-14539-1-git-send-email-changbin.du@intel.com
[changbin.du@intel.com: improve code comments where invoke the new is_static_object callback]
  Link: http://lkml.kernel.org/r/1462777431-8171-1-git-send-email-changbin.du@intel.com
Signed-off-by: Du, Changbin <changbin.du@intel.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Josh Triplett <josh@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tejun Heo <tj@kernel.org>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

1 files changed, 32 insertions, 17 deletions

diff --git a/lib/debugobjects.c b/lib/debugobjects.c
index 2f07c8c697b8..a8e12601eb37 100644
--- a/lib/debugobjects.c
+++ b/lib/debugobjects.c
@@ -431,14 +431,21 @@ int debug_object_activate(void *addr, struct debug_obj_descr *descr)
 
         raw_spin_unlock_irqrestore(&db->lock, flags);
         /*
-         * This happens when a static object is activated. We
-         * let the type specific code decide whether this is
-         * true or not.
+         * We are here when a static object is activated. We
+         * let the type specific code confirm whether this is
+         * true or not. if true, we just make sure that the
+         * static object is tracked in the object tracker. If
+         * not, this must be a bug, so we try to fix it up.
          */
-        if (debug_object_fixup(descr->fixup_activate, addr,
-                           ODEBUG_STATE_NOTAVAILABLE)) {
+        if (descr->is_static_object && descr->is_static_object(addr)) {
+                /* track this static object */
+                debug_object_init(addr, descr);
+                debug_object_activate(addr, descr);
+        } else {
                 debug_print_object(&o, "activate");
-                return -EINVAL;
+                ret = debug_object_fixup(descr->fixup_activate, addr,
+                                        ODEBUG_STATE_NOTAVAILABLE);
+                return ret ? 0 : -EINVAL;
         }
         return 0;
 }
@@ -602,12 +609,18 @@ void debug_object_assert_init(void *addr, struct debug_obj_descr *descr)
 
                 raw_spin_unlock_irqrestore(&db->lock, flags);
                 /*
-                 * Maybe the object is static.  Let the type specific
-                 * code decide what to do.
+                 * Maybe the object is static, and we let the type specific
+                 * code confirm. Track this static object if true, else invoke
+                 * fixup.
                  */
-                if (debug_object_fixup(descr->fixup_assert_init, addr,
-                                       ODEBUG_STATE_NOTAVAILABLE))
+                if (descr->is_static_object && descr->is_static_object(addr)) {
+                        /* Track this static object */
+                        debug_object_init(addr, descr);
+                } else {
                         debug_print_object(&o, "assert_init");
+                        debug_object_fixup(descr->fixup_assert_init, addr,
+                                           ODEBUG_STATE_NOTAVAILABLE);
+                }
                 return;
         }
 
@@ -792,6 +805,13 @@ struct self_test {
 
 static __initdata struct debug_obj_descr descr_type_test;
 
+static bool __init is_static_object(void *addr)
+{
+        struct self_test *obj = addr;
+
+        return obj->static_init;
+}
+
 /*
  * fixup_init is called when:
  * - an active object is initialized
@@ -813,7 +833,7 @@ static bool __init fixup_init(void *addr, enum debug_obj_state state)
 /*
  * fixup_activate is called when:
  * - an active object is activated
- * - an unknown object is activated (might be a statically initialized object)
+ * - an unknown non-static object is activated
  */
 static bool __init fixup_activate(void *addr, enum debug_obj_state state)
 {
@@ -821,13 +841,7 @@ static bool __init fixup_activate(void *addr, enum debug_obj_state state)
 
         switch (state) {
         case ODEBUG_STATE_NOTAVAILABLE:
-                if (obj->static_init == 1) {
-                        debug_object_init(obj, &descr_type_test);
-                        debug_object_activate(obj, &descr_type_test);
-                        return false;
-                }
                 return true;
-
         case ODEBUG_STATE_ACTIVE:
                 debug_object_deactivate(obj, &descr_type_test);
                 debug_object_activate(obj, &descr_type_test);
@@ -916,6 +930,7 @@ out:
 
 static __initdata struct debug_obj_descr descr_type_test = {
         .name                   = "selftest",
+        .is_static_object       = is_static_object,
         .fixup_init             = fixup_init,
         .fixup_activate         = fixup_activate,
         .fixup_destroy          = fixup_destroy,