[PATCH] kernel-2.6.9.rc4 lib/kobject.c

[PATCH] kernel-2.6.9.rc4 lib/kobject.c

[Andrew]

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 <cmkrnl@speakeasy.net>



--- lib/kobject.c.orig    2004-10-16 20:51:01.450973973 -0400
+++ lib/kobject.c    2004-10-16 21:08:19.961602269 -0400
@@ -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;
        }
    }

Re: [PATCH] kernel-2.6.9.rc4 lib/kobject.c

[Olaf Hering]

 On Sat, Oct 16, Andrew wrote:

> 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 >

-- 
USB is for mice, FireWire is for men!

sUse lINUX ag, nÜRNBERG

Re: [PATCH] kernel-2.6.9.rc4 lib/kobject.c

[Greg KH]

On Sat, Oct 16, 2004 at 11:47:44PM -0400, Andrew wrote:
> 
> --- lib/kobject.c.orig    2004-10-16 20:51:01.450973973 -0400
> +++ lib/kobject.c    2004-10-16 21:08:19.961602269 -0400
> @@ -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 :(

>    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);
> +

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