linux-kernel - Re: [PATCH 4/6] drivers/acpi: Use kasprintf

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]

Date:	Thu, 18 Mar 2010 16:25:29 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Julia Lawall <julia@...u.dk>
Cc:	Zhang Rui <rui.zhang@...el.com>, Len Brown <lenb@...nel.org>,
	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
	kernel-janitors@...r.kernel.org
Subject: Re: [PATCH 4/6] drivers/acpi: Use kasprintf

On Wed, 10 Mar 2010 22:16:51 +0100 (CET)
Julia Lawall <julia@...u.dk> wrote:

> From: Julia Lawall <julia@...u.dk>
> 
> kasprintf combines kmalloc and sprintf, and takes care of the size
> calculation itself.
> 
> The semantic patch that makes this change is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> expression a,flag;
> expression list args;
> statement S;
> @@
> 
>   a =
> -  \(kmalloc\|kzalloc\)(...,flag)
> +  kasprintf(flag,args)
>   <... when != a
>   if (a == NULL || ...) S
>   ...>
> - sprintf(a,args);
> // </smpl>
> 
> Signed-off-by: Julia Lawall <julia@...u.dk>
> 
> ---
>  drivers/acpi/video.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff -u -p a/drivers/acpi/video.c b/drivers/acpi/video.c
> --- a/drivers/acpi/video.c
> +++ b/drivers/acpi/video.c
> @@ -1005,11 +1005,10 @@ static void acpi_video_device_find_cap(s
>  		result = acpi_video_init_brightness(device);
>  		if (result)
>  			return;
> -		name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
> +		name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
>  		if (!name)
>  			return;
>  
> -		sprintf(name, "acpi_video%d", count++);
>  		device->backlight = backlight_device_register(name,
>  			NULL, device, &acpi_backlight_ops);
>  		kfree(name);
> @@ -1056,10 +1055,9 @@ static void acpi_video_device_find_cap(s
>  		if (device->cap._DCS && device->cap._DSS) {
>  			static int count;
>  			char *name;
> -			name = kzalloc(MAX_NAME_LEN, GFP_KERNEL);
> +			name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
>  			if (!name)
>  				return;
> -			sprintf(name, "acpi_video%d", count++);
>  			device->output_dev = video_output_register(name,
>  					NULL, device, &acpi_output_properties);
>  			kfree(name);

It's not an equivalent change - if the memory allocation fails, this
patch will now cause `count' to be incremented anyway, presumably
leaving a gap in the enumeration.

I don't know if that matters.  If so, this:

--- a/drivers/acpi/video.c~drivers-acpi-use-kasprintf-fix
+++ a/drivers/acpi/video.c
@@ -1006,9 +1006,10 @@ static void acpi_video_device_find_cap(s
 		result = acpi_video_init_brightness(device);
 		if (result)
 			return;
-		name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
+		name = kasprintf(GFP_KERNEL, "acpi_video%d", count);
 		if (!name)
 			return;
+		count++;
 
 		memset(&props, 0, sizeof(struct backlight_properties));
 		props.max_brightness = device->brightness->count - 3;
@@ -1058,9 +1059,10 @@ static void acpi_video_device_find_cap(s
 		if (device->cap._DCS && device->cap._DSS) {
 			static int count;
 			char *name;
-			name = kasprintf(GFP_KERNEL, "acpi_video%d", count++);
+			name = kasprintf(GFP_KERNEL, "acpi_video%d", count);
 			if (!name)
 				return;
+			count++;
 			device->output_dev = video_output_register(name,
 					NULL, device, &acpi_output_properties);
 			kfree(name);
_

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