Re: [PATCH] Fix kobject uevent string handling errors

[Kay Sievers]

 On Sat, 2007-08-25 at 00:17 -0400, Mathieu Desnoyers wrote:
> Fix kobject uevent string handling errors

> - add warnings when add_uevent_var. Proper handling of its return values should
>   really be done by the callers, but they aren't, so things currently
>   fail silently.

Right, I added the checks to kobject_uevent.c now. There are still
checks missing in:
  arch/powerpc/kernel/vio.c
  block/genhd.c
  drivers/base/class.c
  drivers/base/core.c
  drivers/base/platform.c
  drivers/eisa/eisa-bus.c
  drivers/ide/ide.c
  drivers/scsi/scsi_sysfs.c
  drivers/spi/spi.c

We should fix them with a separate patch, and mark add_uevent_var() as
__must_check.

> --- linux-2.6-lttng.orig/lib/kobject_uevent.c	2007-08-25 00:07:41.000000000 -0400
> +++ linux-2.6-lttng/lib/kobject_uevent.c	2007-08-25 00:12:23.000000000 -0400
> @@ -247,8 +247,12 @@ int add_uevent_var(struct kobj_uevent_en
>  	va_list args;
>  	int len;
>  
> -	if (env->envp_idx >= ARRAY_SIZE(env->envp))
> +	if (env->envp_idx >= ARRAY_SIZE(env->envp)) {
> +		printk("add_uevent_var: too small array size %u %u\n",
> +			env->envp_idx, ARRAY_SIZE(env->envp));
> +		WARN_ON(1);
>  		return -ENOMEM;
> +	}

I added a warning to the patch.

>  	va_start(args, format);
>  	len = vsnprintf(&env->buf[env->buflen],
> @@ -256,8 +260,12 @@ int add_uevent_var(struct kobj_uevent_en
>  			format, args);
>  	va_end(args);
>  
> -	if (len + 1 >= (sizeof(env->buf) - env->buflen))
> +	if (len >= (sizeof(env->buf) - env->buflen)) {

This is already in Greg's tree.

> +		printk("add_uevent_var: failed vsnprintf %d %u\n",
> +			len, (sizeof(env->buf) - env->buflen));
> +		WARN_ON(1);
>  		return -ENOMEM;
> +	}

I added a warning to the patch.

>  	env->envp[env->envp_idx++] = &env->buf[env->buflen];
>  	env->buflen += len + 1;
> Index: linux-2.6-lttng/drivers/firmware/dmi-id.c
> ===================================================================
> --- linux-2.6-lttng.orig/drivers/firmware/dmi-id.c	2007-08-25 00:07:24.000000000 -0400
> +++ linux-2.6-lttng/drivers/firmware/dmi-id.c	2007-08-25 00:07:58.000000000 -0400
> @@ -152,9 +152,10 @@ static int dmi_dev_uevent(struct device 
>  	if (add_uevent_var(env, "MODALIAS="))
>  		return -ENOMEM;
>  	len = get_modalias(&env->buf[env->buflen - 1],
> -			   sizeof(env->buf) - env->buflen);
> -	if (len >= (sizeof(env->buf) - env->buflen))
> +			   sizeof(env->buf) - (env->buflen - 1));
> +	if (len >= (sizeof(env->buf) - (env->buflen - 1)))
>  		return -ENOMEM;
> +	env->buflen += len + 1;

The increment for the trailing '\0' is already done in add_uevent_var(),
so this change is not needed, I think.


Greg,
the attached file replaces the one in your patch tree. It contains the
printk warning and a few checks for return values.

Thanks,
Kay

Attachment: driver-core-change-add_uevent_var-to-use-a-struct.patch
Description: application/mbox