linux-kernel - Re: [PATCH] Fix kobject uevent string handling errors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1188051937.2493.21.camel@lov.localdomain>
Date:	Sat, 25 Aug 2007 16:25:37 +0200
From:	Kay Sievers <kay.sievers@...y.org>
To:	Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
Cc:	Greg KH <greg@...ah.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Fix kobject uevent string handling errors

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

Download attachment "driver-core-change-add_uevent_var-to-use-a-struct.patch" of type "application/mbox" (68678 bytes)