Re: [Patch] 2.6.10.rc1.bk6 /lib/kobject_uevent.c buffer issues

[Kay Sievers]

On Fri, Oct 29, 2004 at 06:13:19PM -0500, Greg KH wrote:
> On Fri, Oct 29, 2004 at 11:28:56PM +0200, Kay Sievers wrote:
> > > But there might still be a problem.  With this change, the sequence
> > > number is not sent out the kevent message.  Kay, do you think this is an
> > > issue?  I don't think we can get netlink messages out of order, right?
> > 
> > Right, especially not the events with the same DEVPATH, like "remove"
> > beating an "add". But I'm not sure if the number isn't useful. Whatever
> > we may do with the hotplug over netlink in the future, we will only have
> > /sbin/hotplug for the early boot and it may be nice to know, what events
> > we have already handled...
> > 
> > > I'll hold off on applying this patch until we figure this out...
> > 
> > How about just reserving 20 bytes for the number (u64 will never be
> > more than that), save the pointer to that field, and fill the number in
> > later?
> 
> Ah, something like this instead?  I like it, it's even smaller than the
> previous patch.  Compile tested only...

I like that. How about the following. It will keep the buffer clean from
random chars, cause the kevent does not have the vector and relies on
the '\0' to separate the strings from each other.
I've tested it. The netlink-hotplug message looks like this:

recv(3, "remove@/class/input/mouse2\0ACTION=remove\0DEVPATH=/class/input/mouse2\0SUBSYSTEM=input\0SEQNUM=961                 \0", 1024, 0) = 113


> --------
> 
> --- 1.5/lib/kobject_uevent.c	2004-10-22 17:42:27 -05:00
> +++ edited/kobject_uevent.c	2004-10-29 18:07:50 -05:00
> @@ -182,6 +182,7 @@
>  	char *argv [3];
>  	char **envp = NULL;
>  	char *buffer = NULL;
> +	char *seq_buff;
>  	char *scratch;
>  	int i = 0;
>  	int retval;
> @@ -258,6 +259,11 @@
>  	envp [i++] = scratch;
>  	scratch += sprintf(scratch, "SUBSYSTEM=%s", name) + 1;
>  
> +	/* reserve space for the sequence, 
> +	 * put the real one in after the hotplug call */
> +	envp[i++] = seq_buff = scratch;
> +	scratch += sprintf(scratch, "SEQNUM=12345678901234567890") + 1;

+       scratch += strlen("SEQNUM=12345678901234567890") + 1;

> +
>  	if (hotplug_ops->hotplug) {
>  		/* have the kset specific function add its stuff */
>  		retval = hotplug_ops->hotplug (kset, kobj,
> @@ -273,9 +279,7 @@
>  	spin_lock(&sequence_lock);
>  	seq = ++hotplug_seqnum;
>  	spin_unlock(&sequence_lock);
> -
> -	envp [i++] = scratch;
> -	scratch += sprintf(scratch, "SEQNUM=%lld", (long long)seq) + 1;
> +	sprintf(seq_buff, "SEQNUM=%lld", (long long)seq);

+       sprintf(seq_buff, "SEQNUM=%-20lld", (long long)seq);

>  	pr_debug ("%s: %s %s seq=%lld %s %s %s %s %s\n",
>  		  __FUNCTION__, argv[0], argv[1], (long long)seq,
> 

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/