Re: usb hotplug $DEVICE empty

Re: usb hotplug $DEVICE empty

[Klaus Dittrich]

I found out why $DEVICE did not show of in /sbin/hotplug
any more. (2.6.10-rc1-bk5)

Depending on the setting of CONFIG_USB_DEVICEFS
either the environment variables DEVICE or PRODUCT
which should be passed to /sbin/hotplug become overwritten
by the environment variable SEQNUM.

This happens in kobject_hotplug() of lib/kobject_uevent.c.

The here called hotplug_ops->hotplug function usb_hotplug()
advances both, the index to envp and the pointer into buffer
but the calling function gets no notice of that.

So to add SEQNUM later on it advances index/pointer to
envp/buffer from where it has stopped before calling
usb_hotplug(), thus overwriting the first entry that 
usb_hotplug() has added before.


-- 
Klaus

[PATCH] kobject hotplug: don't let SEQNUM overwrite other vars (was Re: usb hotplug $DEVICE empty)

[Roland Dreier]

I think this trivial patch should fix this for you.  (Greg, not sure
if you have this already -- it's not in Linus's tree yet in any case)

Prevent SEQNUM from overwriting kset-specific hotplug environment vars.

Signed-off-by: Roland Dreier <roland@topspin.com>

Index: linux-bk/lib/kobject_uevent.c
===================================================================
--- linux-bk.orig/lib/kobject_uevent.c	2004-10-28 21:20:10.000000000 -0700
+++ linux-bk/lib/kobject_uevent.c	2004-10-28 21:21:10.000000000 -0700
@@ -258,6 +258,13 @@
 	envp [i++] = scratch;
 	scratch += sprintf(scratch, "SUBSYSTEM=%s", name) + 1;
 
+	spin_lock(&sequence_lock);
+	seq = ++hotplug_seqnum;
+	spin_unlock(&sequence_lock);
+
+	envp [i++] = scratch;
+	scratch += sprintf(scratch, "SEQNUM=%lld", (long long)seq) + 1;
+
 	if (hotplug_ops->hotplug) {
 		/* have the kset specific function add its stuff */
 		retval = hotplug_ops->hotplug (kset, kobj,
@@ -270,13 +277,6 @@
 		}
 	}
 
-	spin_lock(&sequence_lock);
-	seq = ++hotplug_seqnum;
-	spin_unlock(&sequence_lock);
-
-	envp [i++] = scratch;
-	scratch += sprintf(scratch, "SEQNUM=%lld", (long long)seq) + 1;
-
 	pr_debug ("%s: %s %s seq=%lld %s %s %s %s %s\n",
 		  __FUNCTION__, argv[0], argv[1], (long long)seq,
 		  envp[0], envp[1], envp[2], envp[3], envp[4]);

Re: [PATCH] kobject hotplug: don't let SEQNUM overwrite other vars (was Re: usb hotplug $DEVICE empty)

[Greg KH]

On Thu, Oct 28, 2004 at 09:24:15PM -0700, Roland Dreier wrote:
> I think this trivial patch should fix this for you.  (Greg, not sure
> if you have this already -- it's not in Linus's tree yet in any case)

No, people are still working on getting this right.

> Prevent SEQNUM from overwriting kset-specific hotplug environment vars.

No, this puts back the problem where if the hotplug() subsystem call
fails, we have already incremented the seqnum without emitting a call
with that number.

Now I know userspace needs to handle this properly anyway, but we might
as well get the kernel right, and not do stuff to make userspace unhappy
if we can obviously help it.

I'll work on fixing this up properly tomorrow,

thanks,

greg k-h

Re: [PATCH] kobject hotplug: don't let SEQNUM overwrite other vars

[Roland Dreier]

    Greg> No, this puts back the problem where if the hotplug()
    Greg> subsystem call fails, we have already incremented the seqnum
    Greg> without emitting a call with that number.

    Greg> Now I know userspace needs to handle this properly anyway,
    Greg> but we might as well get the kernel right, and not do stuff
    Greg> to make userspace unhappy if we can obviously help it.

Got it... I had remembered you saying gaps in the sequence numbers
were OK in the past.

 - R.