Skip to contentSkip to conversation

kernel-2.6.9.rc4 lib/kobject.c

PATCH In review Andrew 3 participants 3 messages Oct 17, 2004

lore
Share & tools
lore

Andrew

author
#0 lore raw

PATCH 1/1 kernel-2.6.9.rc4 lib/kobject.c

There are two conditions where kset_hotplug can exit without the call to call_usermodehelper() after incrementing the sequence_number. This patch eliminates the first by getting the kobj_path before the sequence_number++. It also reduces the likelihood of the second by decrementing the sequence_number (if it can) if the call to kset->hotplug_ops->hotplug() fails.

Some user space programs (rightly or wrongly) are expecting there would be no "gaps" in hotplug event sequence numbers, and hang waiting for the "missing" events.

Signed-off-by Andrew Duggan <[email protected]>
kobject.c 2004-10-16 21:08:19.961602269 -0400 renamed from kobject.c.orig 2004-10-16 20:51:01.450973973 -0400 +11 -4
@@ -177,6 +177,10 @@ static void kset_hotplug(const char *act
   envp [i++] = scratch;
   scratch += sprintf(scratch, "ACTION=%s", action) + 1;
 
+    kobj_path = kobject_get_path(kset, kobj, GFP_KERNEL);
+    if (!kobj_path)
+        goto exit;
+
   spin_lock(&sequence_lock);
   seq = sequence_num++;
   spin_unlock(&sequence_lock);

@@ -184,10 +188,6 @@ static void kset_hotplug(const char *act
   envp [i++] = scratch;
   scratch += sprintf(scratch, "SEQNUM=%ld", seq) + 1;
 
-    kobj_path = kobject_get_path(kset, kobj, GFP_KERNEL);
-    if (!kobj_path)
-        goto exit;
-
   envp [i++] = scratch;
   scratch += sprintf (scratch, "DEVPATH=%s", kobj_path) + 1;
 

@@ -199,6 +199,13 @@ static void kset_hotplug(const char *act
       if (retval) {
           pr_debug ("%s - hotplug() returned %d\n",
                 __FUNCTION__, retval);
+            /* decr sequence_num since no event will happen
+               but only if it is consistent */
+            spin_lock(&sequence_lock);
+            if (sequence_num == seq+1)
+               sequence_num--;
+            spin_unlock(&sequence_lock);
+
           goto exit;
       }
   }
#1 lore raw
Quoted 3 lines · Andrew

Some user space programs (rightly or wrongly) are expecting there would be no "gaps" in hotplug event sequence numbers, and hang waiting for the "missing" events.

Every code that relies on the value of $SEQNUM is just broken. The only valid check is < or >

Signature · 3 lines
USB is for mice, FireWire is for men!

sUse lINUX ag, nÜRNBERG
#2 lore raw
In reply to Andrew

kobject.c 2004-10-16 21:08:19.961602269 -0400 static void kset_hotplug(const char *act · line 183

Quoted 11 lines · Andrew
kobject.c 2004-10-16 21:08:19.961602269 -0400 renamed from kobject.c.orig 2004-10-16 20:51:01.450973973 -0400 +4 -0
@@ -177,6 +177,10 @@ static void kset_hotplug(const char *act
   envp [i++] = scratch;
   scratch += sprintf(scratch, "ACTION=%s", action) + 1;
 
+    kobj_path = kobject_get_path(kset, kobj, GFP_KERNEL);
+    if (!kobj_path)
+        goto exit;
+

Your email client ate the tabs :(

Quoted 15 lines
kobject.c 2004-10-16 21:08:19.961602269 -0400 renamed from kobject.c.orig 2004-10-16 20:51:01.450973973 -0400 +0 -4
   spin_lock(&sequence_lock);
   seq = sequence_num++;
   spin_unlock(&sequence_lock);

@@ -184,10 +188,6 @@ static void kset_hotplug(const char *act
   envp [i++] = scratch;
   scratch += sprintf(scratch, "SEQNUM=%ld", seq) + 1;
 
-    kobj_path = kobject_get_path(kset, kobj, GFP_KERNEL);
-    if (!kobj_path)
-        goto exit;
-
   envp [i++] = scratch;
   scratch += sprintf (scratch, "DEVPATH=%s", kobj_path) + 1;

kobject.c 2004-10-16 21:08:19.961602269 -0400 static void kset_hotplug(const char *act · line 208

Quoted 10 lines
kobject.c 2004-10-16 21:08:19.961602269 -0400 renamed from kobject.c.orig 2004-10-16 20:51:01.450973973 -0400 +7 -0
@@ -199,6 +199,13 @@ static void kset_hotplug(const char *act
       if (retval) {
           pr_debug ("%s - hotplug() returned %d\n",
                 __FUNCTION__, retval);
+            /* decr sequence_num since no event will happen
+               but only if it is consistent */
+            spin_lock(&sequence_lock);
+            if (sequence_num == seq+1)
+               sequence_num--;
+            spin_unlock(&sequence_lock);
+

This could cause the same sequence number to be given to more than one event. That would really mess userspace up. It can handle gaps in sequence numbers, as long as they are constantly incrementing.

Care to redo this to give out the sequence number as the last thing before calling call_usermodehelper()? That should fix the issue, right?

Oh, and this portion of the kernel has been pretty much reworked a lot recently. Check out the -mm kernel release for what it now looks like (and what will be sent to Linus after 2.6.9 is released.)

thanks,

greg k-h