All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch] x86, efi: fix pointer math issue in handle_ramdisks()
@ 2012-03-02 19:01 ` Dan Carpenter
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2012-03-02 19:01 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Thomas Gleixner, Ingo Molnar, x86, Matt Fleming,
	Maarten Lankhorst, linux-kernel, kernel-janitors

"filename" is a efi_char16_t string so this check for reaching the end
of the array doesn't work.  We need to cast it to char pointer before
doing the math.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index fec216f..cf4cdb7 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -559,7 +559,7 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
 			str++;
 
 		while (*str && *str != ' ' && *str != '\n') {
-			if (p >= filename + sizeof(filename))
+			if ((char *)p >= (char *)filename + sizeof(filename))
 				break;
 
 			*p++ = *str++;

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [patch] x86, efi: fix pointer math issue in handle_ramdisks()
@ 2012-03-02 19:01 ` Dan Carpenter
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2012-03-02 19:01 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Thomas Gleixner, Ingo Molnar, x86, Matt Fleming,
	Maarten Lankhorst, linux-kernel, kernel-janitors

"filename" is a efi_char16_t string so this check for reaching the end
of the array doesn't work.  We need to cast it to char pointer before
doing the math.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index fec216f..cf4cdb7 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -559,7 +559,7 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
 			str++;
 
 		while (*str && *str != ' ' && *str != '\n') {
-			if (p >= filename + sizeof(filename))
+			if ((char *)p >= (char *)filename + sizeof(filename))
 				break;
 
 			*p++ = *str++;

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [patch] x86, efi: fix pointer math issue in handle_ramdisks()
  2012-03-02 19:01 ` Dan Carpenter
@ 2012-03-03  7:54   ` Ingo Molnar
  -1 siblings, 0 replies; 18+ messages in thread
From: Ingo Molnar @ 2012-03-03  7:54 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, Matt Fleming,
	Maarten Lankhorst, linux-kernel, kernel-janitors


* Dan Carpenter <dan.carpenter@oracle.com> wrote:

> "filename" is a efi_char16_t string so this check for reaching 
> the end of the array doesn't work.  We need to cast it to char 
> pointer before doing the math.

That name should really be changed, 'filename' is a char * by 
convention pretty much everywhere in the kernel - so the current 
naming is highly misleading and results in bugs like this.

filename_16, filename_2byte or filename_UTF or so would be 
suggestive enough to avoid such mishaps in the future.

> @@ -559,7 +559,7 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
>  			str++;
>  
>  		while (*str && *str != ' ' && *str != '\n') {
> -			if (p >= filename + sizeof(filename))
> +			if ((char *)p >= (char *)filename + sizeof(filename))
>  				break;

I'd also make that void *, because this isnt really a C 
character string anymore.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [patch] x86, efi: fix pointer math issue in handle_ramdisks()
@ 2012-03-03  7:54   ` Ingo Molnar
  0 siblings, 0 replies; 18+ messages in thread
From: Ingo Molnar @ 2012-03-03  7:54 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, Matt Fleming,
	Maarten Lankhorst, linux-kernel, kernel-janitors


* Dan Carpenter <dan.carpenter@oracle.com> wrote:

> "filename" is a efi_char16_t string so this check for reaching 
> the end of the array doesn't work.  We need to cast it to char 
> pointer before doing the math.

That name should really be changed, 'filename' is a char * by 
convention pretty much everywhere in the kernel - so the current 
naming is highly misleading and results in bugs like this.

filename_16, filename_2byte or filename_UTF or so would be 
suggestive enough to avoid such mishaps in the future.

> @@ -559,7 +559,7 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
>  			str++;
>  
>  		while (*str && *str != ' ' && *str != '\n') {
> -			if (p >= filename + sizeof(filename))
> +			if ((char *)p >= (char *)filename + sizeof(filename))
>  				break;

I'd also make that void *, because this isnt really a C 
character string anymore.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [patch v2] x86, efi: fix pointer math issue in handle_ramdisks()
  2012-03-03  7:54   ` Ingo Molnar
@ 2012-03-05 18:06     ` Dan Carpenter
  -1 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2012-03-05 18:06 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Thomas Gleixner, Ingo Molnar, x86, Matt Fleming,
	Maarten Lankhorst, linux-kernel, kernel-janitors

"filename" is a efi_char16_t string so this check for reaching the end
of the array doesn't work.  We need to cast the pointer to (u8 *) before
doing the math.

This patch changes the "filename" to "filename_16" to avoid confusion in
the future.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: Rename "filename" to "filename_16"
    Also changed cast from (char *) to (u8 *) because it's not a C
    character string.  Ingo suggested (void *) but that's a GCCism
    I think.

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index fec216f..0cdfc0d 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -539,7 +539,7 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
 		struct initrd *initrd;
 		efi_file_handle_t *h;
 		efi_file_info_t *info;
-		efi_char16_t filename[256];
+		efi_char16_t filename_16[256];
 		unsigned long info_sz;
 		efi_guid_t info_guid = EFI_FILE_INFO_ID;
 		efi_char16_t *p;
@@ -552,14 +552,14 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
 		str += 7;
 
 		initrd = &initrds[i];
-		p = filename;
+		p = filename_16;
 
 		/* Skip any leading slashes */
 		while (*str = '/' || *str = '\\')
 			str++;
 
 		while (*str && *str != ' ' && *str != '\n') {
-			if (p >= filename + sizeof(filename))
+			if ((u8 *)p >= (u8 *)filename_16 + sizeof(filename_16))
 				break;
 
 			*p++ = *str++;
@@ -583,7 +583,7 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
 				goto free_initrds;
 		}
 
-		status = efi_call_phys5(fh->open, fh, &h, filename,
+		status = efi_call_phys5(fh->open, fh, &h, filename_16,
 					EFI_FILE_MODE_READ, (u64)0);
 		if (status != EFI_SUCCESS)
 			goto close_handles;

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [patch v2] x86, efi: fix pointer math issue in handle_ramdisks()
@ 2012-03-05 18:06     ` Dan Carpenter
  0 siblings, 0 replies; 18+ messages in thread
From: Dan Carpenter @ 2012-03-05 18:06 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Thomas Gleixner, Ingo Molnar, x86, Matt Fleming,
	Maarten Lankhorst, linux-kernel, kernel-janitors

"filename" is a efi_char16_t string so this check for reaching the end
of the array doesn't work.  We need to cast the pointer to (u8 *) before
doing the math.

This patch changes the "filename" to "filename_16" to avoid confusion in
the future.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2: Rename "filename" to "filename_16"
    Also changed cast from (char *) to (u8 *) because it's not a C
    character string.  Ingo suggested (void *) but that's a GCCism
    I think.

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index fec216f..0cdfc0d 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -539,7 +539,7 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
 		struct initrd *initrd;
 		efi_file_handle_t *h;
 		efi_file_info_t *info;
-		efi_char16_t filename[256];
+		efi_char16_t filename_16[256];
 		unsigned long info_sz;
 		efi_guid_t info_guid = EFI_FILE_INFO_ID;
 		efi_char16_t *p;
@@ -552,14 +552,14 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
 		str += 7;
 
 		initrd = &initrds[i];
-		p = filename;
+		p = filename_16;
 
 		/* Skip any leading slashes */
 		while (*str == '/' || *str == '\\')
 			str++;
 
 		while (*str && *str != ' ' && *str != '\n') {
-			if (p >= filename + sizeof(filename))
+			if ((u8 *)p >= (u8 *)filename_16 + sizeof(filename_16))
 				break;
 
 			*p++ = *str++;
@@ -583,7 +583,7 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
 				goto free_initrds;
 		}
 
-		status = efi_call_phys5(fh->open, fh, &h, filename,
+		status = efi_call_phys5(fh->open, fh, &h, filename_16,
 					EFI_FILE_MODE_READ, (u64)0);
 		if (status != EFI_SUCCESS)
 			goto close_handles;

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [patch v2] x86, efi: fix pointer math issue in handle_ramdisks()
  2012-03-05 18:06     ` Dan Carpenter
@ 2012-03-05 18:42       ` walter harms
  -1 siblings, 0 replies; 18+ messages in thread
From: walter harms @ 2012-03-05 18:42 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, Matt Fleming,
	Maarten Lankhorst, linux-kernel, kernel-janitors



Am 05.03.2012 19:06, schrieb Dan Carpenter:
> "filename" is a efi_char16_t string so this check for reaching the end
> of the array doesn't work.  We need to cast the pointer to (u8 *) before
> doing the math.
> 
> This patch changes the "filename" to "filename_16" to avoid confusion in
> the future.
> 

maybe it is a bit late,  but ...
is efi_char16_t a generic requirement for EFI ? perhaps we can use wchar_t
since it is intended for such cases. additional we would get an api for free.

just my 2 cents,
 wh


> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: Rename "filename" to "filename_16"
>     Also changed cast from (char *) to (u8 *) because it's not a C
>     character string.  Ingo suggested (void *) but that's a GCCism
>     I think.
> 
> diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
> index fec216f..0cdfc0d 100644
> --- a/arch/x86/boot/compressed/eboot.c
> +++ b/arch/x86/boot/compressed/eboot.c
> @@ -539,7 +539,7 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
>  		struct initrd *initrd;
>  		efi_file_handle_t *h;
>  		efi_file_info_t *info;
> -		efi_char16_t filename[256];
> +		efi_char16_t filename_16[256];
>  		unsigned long info_sz;
>  		efi_guid_t info_guid = EFI_FILE_INFO_ID;
>  		efi_char16_t *p;
> @@ -552,14 +552,14 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
>  		str += 7;
>  
>  		initrd = &initrds[i];
> -		p = filename;
> +		p = filename_16;
>  
>  		/* Skip any leading slashes */
>  		while (*str = '/' || *str = '\\')
>  			str++;
>  
>  		while (*str && *str != ' ' && *str != '\n') {
> -			if (p >= filename + sizeof(filename))
> +			if ((u8 *)p >= (u8 *)filename_16 + sizeof(filename_16))
>  				break;
>  
>  			*p++ = *str++;
> @@ -583,7 +583,7 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
>  				goto free_initrds;
>  		}
>  
> -		status = efi_call_phys5(fh->open, fh, &h, filename,
> +		status = efi_call_phys5(fh->open, fh, &h, filename_16,
>  					EFI_FILE_MODE_READ, (u64)0);
>  		if (status != EFI_SUCCESS)
>  			goto close_handles;
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [patch v2] x86, efi: fix pointer math issue in handle_ramdisks()
@ 2012-03-05 18:42       ` walter harms
  0 siblings, 0 replies; 18+ messages in thread
From: walter harms @ 2012-03-05 18:42 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86, Matt Fleming,
	Maarten Lankhorst, linux-kernel, kernel-janitors



Am 05.03.2012 19:06, schrieb Dan Carpenter:
> "filename" is a efi_char16_t string so this check for reaching the end
> of the array doesn't work.  We need to cast the pointer to (u8 *) before
> doing the math.
> 
> This patch changes the "filename" to "filename_16" to avoid confusion in
> the future.
> 

maybe it is a bit late,  but ...
is efi_char16_t a generic requirement for EFI ? perhaps we can use wchar_t
since it is intended for such cases. additional we would get an api for free.

just my 2 cents,
 wh


> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: Rename "filename" to "filename_16"
>     Also changed cast from (char *) to (u8 *) because it's not a C
>     character string.  Ingo suggested (void *) but that's a GCCism
>     I think.
> 
> diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
> index fec216f..0cdfc0d 100644
> --- a/arch/x86/boot/compressed/eboot.c
> +++ b/arch/x86/boot/compressed/eboot.c
> @@ -539,7 +539,7 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
>  		struct initrd *initrd;
>  		efi_file_handle_t *h;
>  		efi_file_info_t *info;
> -		efi_char16_t filename[256];
> +		efi_char16_t filename_16[256];
>  		unsigned long info_sz;
>  		efi_guid_t info_guid = EFI_FILE_INFO_ID;
>  		efi_char16_t *p;
> @@ -552,14 +552,14 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
>  		str += 7;
>  
>  		initrd = &initrds[i];
> -		p = filename;
> +		p = filename_16;
>  
>  		/* Skip any leading slashes */
>  		while (*str == '/' || *str == '\\')
>  			str++;
>  
>  		while (*str && *str != ' ' && *str != '\n') {
> -			if (p >= filename + sizeof(filename))
> +			if ((u8 *)p >= (u8 *)filename_16 + sizeof(filename_16))
>  				break;
>  
>  			*p++ = *str++;
> @@ -583,7 +583,7 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
>  				goto free_initrds;
>  		}
>  
> -		status = efi_call_phys5(fh->open, fh, &h, filename,
> +		status = efi_call_phys5(fh->open, fh, &h, filename_16,
>  					EFI_FILE_MODE_READ, (u64)0);
>  		if (status != EFI_SUCCESS)
>  			goto close_handles;
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [patch v2] x86, efi: fix pointer math issue in handle_ramdisks()
  2012-03-05 18:42       ` walter harms
@ 2012-03-05 19:33         ` H. Peter Anvin
  -1 siblings, 0 replies; 18+ messages in thread
From: H. Peter Anvin @ 2012-03-05 19:33 UTC (permalink / raw)
  To: wharms
  Cc: Dan Carpenter, Thomas Gleixner, Ingo Molnar, x86, Matt Fleming,
	Maarten Lankhorst, linux-kernel, kernel-janitors

On 03/05/2012 10:42 AM, walter harms wrote:
> 
> 
> Am 05.03.2012 19:06, schrieb Dan Carpenter:
>> "filename" is a efi_char16_t string so this check for reaching the end
>> of the array doesn't work.  We need to cast the pointer to (u8 *) before
>> doing the math.
>>
>> This patch changes the "filename" to "filename_16" to avoid confusion in
>> the future.
>>
> 
> maybe it is a bit late,  but ...
> is efi_char16_t a generic requirement for EFI ? perhaps we can use wchar_t
> since it is intended for such cases. additional we would get an api for free.
> 

wchar_t is typically 32 bits on Linux.  16-bit anything is a major fail
since Unicode doesn't actually fit in 16 bits.

	-hpa


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [patch v2] x86, efi: fix pointer math issue in handle_ramdisks()
@ 2012-03-05 19:33         ` H. Peter Anvin
  0 siblings, 0 replies; 18+ messages in thread
From: H. Peter Anvin @ 2012-03-05 19:33 UTC (permalink / raw)
  To: wharms
  Cc: Dan Carpenter, Thomas Gleixner, Ingo Molnar, x86, Matt Fleming,
	Maarten Lankhorst, linux-kernel, kernel-janitors

On 03/05/2012 10:42 AM, walter harms wrote:
> 
> 
> Am 05.03.2012 19:06, schrieb Dan Carpenter:
>> "filename" is a efi_char16_t string so this check for reaching the end
>> of the array doesn't work.  We need to cast the pointer to (u8 *) before
>> doing the math.
>>
>> This patch changes the "filename" to "filename_16" to avoid confusion in
>> the future.
>>
> 
> maybe it is a bit late,  but ...
> is efi_char16_t a generic requirement for EFI ? perhaps we can use wchar_t
> since it is intended for such cases. additional we would get an api for free.
> 

wchar_t is typically 32 bits on Linux.  16-bit anything is a major fail
since Unicode doesn't actually fit in 16 bits.

	-hpa


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [patch v2] x86, efi: fix pointer math issue in handle_ramdisks()
  2012-03-05 19:33         ` H. Peter Anvin
@ 2012-03-06  8:44           ` walter harms
  -1 siblings, 0 replies; 18+ messages in thread
From: walter harms @ 2012-03-06  8:44 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Dan Carpenter, Thomas Gleixner, Ingo Molnar, x86, Matt Fleming,
	Maarten Lankhorst, linux-kernel, kernel-janitors



Am 05.03.2012 20:33, schrieb H. Peter Anvin:
> On 03/05/2012 10:42 AM, walter harms wrote:
>>
>>
>> Am 05.03.2012 19:06, schrieb Dan Carpenter:
>>> "filename" is a efi_char16_t string so this check for reaching the end
>>> of the array doesn't work.  We need to cast the pointer to (u8 *) before
>>> doing the math.
>>>
>>> This patch changes the "filename" to "filename_16" to avoid confusion in
>>> the future.
>>>
>>
>> maybe it is a bit late,  but ...
>> is efi_char16_t a generic requirement for EFI ? perhaps we can use wchar_t
>> since it is intended for such cases. additional we would get an api for free.
>>
> 
> wchar_t is typically 32 bits on Linux.  16-bit anything is a major fail
> since Unicode doesn't actually fit in 16 bits.
> 
hi,

yep, but i was asking about efi. The basic idea is of cause to map efi_char16_t -> wchar_t
and back and make this a prototype for every driver that needs a special charset.
That would make it possible to recycle the wcs* interface of libc.
IMHO it seems more reasonable than adding one for each (upcoming) type.

re,
 wh

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [patch v2] x86, efi: fix pointer math issue in handle_ramdisks()
@ 2012-03-06  8:44           ` walter harms
  0 siblings, 0 replies; 18+ messages in thread
From: walter harms @ 2012-03-06  8:44 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Dan Carpenter, Thomas Gleixner, Ingo Molnar, x86, Matt Fleming,
	Maarten Lankhorst, linux-kernel, kernel-janitors



Am 05.03.2012 20:33, schrieb H. Peter Anvin:
> On 03/05/2012 10:42 AM, walter harms wrote:
>>
>>
>> Am 05.03.2012 19:06, schrieb Dan Carpenter:
>>> "filename" is a efi_char16_t string so this check for reaching the end
>>> of the array doesn't work.  We need to cast the pointer to (u8 *) before
>>> doing the math.
>>>
>>> This patch changes the "filename" to "filename_16" to avoid confusion in
>>> the future.
>>>
>>
>> maybe it is a bit late,  but ...
>> is efi_char16_t a generic requirement for EFI ? perhaps we can use wchar_t
>> since it is intended for such cases. additional we would get an api for free.
>>
> 
> wchar_t is typically 32 bits on Linux.  16-bit anything is a major fail
> since Unicode doesn't actually fit in 16 bits.
> 
hi,

yep, but i was asking about efi. The basic idea is of cause to map efi_char16_t -> wchar_t
and back and make this a prototype for every driver that needs a special charset.
That would make it possible to recycle the wcs* interface of libc.
IMHO it seems more reasonable than adding one for each (upcoming) type.

re,
 wh

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [patch v2] x86, efi: fix pointer math issue in handle_ramdisks()
  2012-03-05 18:06     ` Dan Carpenter
  (?)
  (?)
@ 2012-03-16 15:20     ` Matt Fleming
  -1 siblings, 0 replies; 18+ messages in thread
From: Matt Fleming @ 2012-03-16 15:20 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: H. Peter Anvin, Thomas Gleixner, Ingo Molnar, x86,
	Maarten Lankhorst, linux-kernel, kernel-janitors

On Mon, 2012-03-05 at 21:06 +0300, Dan Carpenter wrote:
> "filename" is a efi_char16_t string so this check for reaching the end
> of the array doesn't work.  We need to cast the pointer to (u8 *) before
> doing the math.
> 
> This patch changes the "filename" to "filename_16" to avoid confusion in
> the future.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2: Rename "filename" to "filename_16"
>     Also changed cast from (char *) to (u8 *) because it's not a C
>     character string.  Ingo suggested (void *) but that's a GCCism
>     I think.

Looks good to me. Is someone going to pick this up?


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [patch v2] x86, efi: fix pointer math issue in handle_ramdisks()
  2012-03-06  8:44           ` walter harms
@ 2012-03-16 19:55             ` H. Peter Anvin
  -1 siblings, 0 replies; 18+ messages in thread
From: H. Peter Anvin @ 2012-03-16 19:55 UTC (permalink / raw)
  To: wharms
  Cc: Dan Carpenter, Thomas Gleixner, Ingo Molnar, x86, Matt Fleming,
	Maarten Lankhorst, linux-kernel, kernel-janitors

On 03/06/2012 12:44 AM, walter harms wrote:
>>
> hi,
> 
> yep, but i was asking about efi. The basic idea is of cause to map efi_char16_t -> wchar_t
> and back and make this a prototype for every driver that needs a special charset.
> That would make it possible to recycle the wcs* interface of libc.
> IMHO it seems more reasonable than adding one for each (upcoming) type.
> 

Actually libc and the C standard is going the opposite ways, adding new
interfaces for UTF-16 and UTF-32.  The wchar_t interface just doesn't
work very well.

	-hpa


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [patch v2] x86, efi: fix pointer math issue in handle_ramdisks()
@ 2012-03-16 19:55             ` H. Peter Anvin
  0 siblings, 0 replies; 18+ messages in thread
From: H. Peter Anvin @ 2012-03-16 19:55 UTC (permalink / raw)
  To: wharms
  Cc: Dan Carpenter, Thomas Gleixner, Ingo Molnar, x86, Matt Fleming,
	Maarten Lankhorst, linux-kernel, kernel-janitors

On 03/06/2012 12:44 AM, walter harms wrote:
>>
> hi,
> 
> yep, but i was asking about efi. The basic idea is of cause to map efi_char16_t -> wchar_t
> and back and make this a prototype for every driver that needs a special charset.
> That would make it possible to recycle the wcs* interface of libc.
> IMHO it seems more reasonable than adding one for each (upcoming) type.
> 

Actually libc and the C standard is going the opposite ways, adding new
interfaces for UTF-16 and UTF-32.  The wchar_t interface just doesn't
work very well.

	-hpa


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [tip:x86/urgent] x86, efi: Fix pointer math issue in handle_ramdisks()
  2012-03-05 18:06     ` Dan Carpenter
                       ` (2 preceding siblings ...)
  (?)
@ 2012-03-16 21:27     ` tip-bot for Dan Carpenter
  -1 siblings, 0 replies; 18+ messages in thread
From: tip-bot for Dan Carpenter @ 2012-03-16 21:27 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, tglx, hpa, matt.fleming, dan.carpenter

Commit-ID:  c7b738351ba92f48b943ac59aff6b5b0f17f37c9
Gitweb:     http://git.kernel.org/tip/c7b738351ba92f48b943ac59aff6b5b0f17f37c9
Author:     Dan Carpenter <dan.carpenter@oracle.com>
AuthorDate: Mon, 5 Mar 2012 21:06:14 +0300
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Fri, 16 Mar 2012 13:03:24 -0700

x86, efi: Fix pointer math issue in handle_ramdisks()

"filename" is a efi_char16_t string so this check for reaching the end
of the array doesn't work.  We need to cast the pointer to (u8 *) before
doing the math.

This patch changes the "filename" to "filename_16" to avoid confusion in
the future.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Link: http://lkml.kernel.org/r/20120305180614.GA26880@elgon.mountain
Acked-by: Matt Fleming <matt.fleming@intel.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/boot/compressed/eboot.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index fec216f..0cdfc0d 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -539,7 +539,7 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
 		struct initrd *initrd;
 		efi_file_handle_t *h;
 		efi_file_info_t *info;
-		efi_char16_t filename[256];
+		efi_char16_t filename_16[256];
 		unsigned long info_sz;
 		efi_guid_t info_guid = EFI_FILE_INFO_ID;
 		efi_char16_t *p;
@@ -552,14 +552,14 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
 		str += 7;
 
 		initrd = &initrds[i];
-		p = filename;
+		p = filename_16;
 
 		/* Skip any leading slashes */
 		while (*str == '/' || *str == '\\')
 			str++;
 
 		while (*str && *str != ' ' && *str != '\n') {
-			if (p >= filename + sizeof(filename))
+			if ((u8 *)p >= (u8 *)filename_16 + sizeof(filename_16))
 				break;
 
 			*p++ = *str++;
@@ -583,7 +583,7 @@ static efi_status_t handle_ramdisks(efi_loaded_image_t *image,
 				goto free_initrds;
 		}
 
-		status = efi_call_phys5(fh->open, fh, &h, filename,
+		status = efi_call_phys5(fh->open, fh, &h, filename_16,
 					EFI_FILE_MODE_READ, (u64)0);
 		if (status != EFI_SUCCESS)
 			goto close_handles;

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [patch v2] x86, efi: fix pointer math issue in handle_ramdisks()
  2012-03-16 19:55             ` H. Peter Anvin
@ 2012-03-18 14:33               ` walter harms
  -1 siblings, 0 replies; 18+ messages in thread
From: walter harms @ 2012-03-18 14:33 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Dan Carpenter, Thomas Gleixner, Ingo Molnar, x86, Matt Fleming,
	Maarten Lankhorst, linux-kernel, kernel-janitors



Am 16.03.2012 20:55, schrieb H. Peter Anvin:
> On 03/06/2012 12:44 AM, walter harms wrote:
>>>
>> hi,
>>
>> yep, but i was asking about efi. The basic idea is of cause to map efi_char16_t -> wchar_t
>> and back and make this a prototype for every driver that needs a special charset.
>> That would make it possible to recycle the wcs* interface of libc.
>> IMHO it seems more reasonable than adding one for each (upcoming) type.
>>
> 
> Actually libc and the C standard is going the opposite ways, adding new
> interfaces for UTF-16 and UTF-32.  The wchar_t interface just doesn't
> work very well.
> 

I admit i do not have the problem yet and i do not like the idea adding even more types.
Oh well may i should be happy that someone is actually thinking about this here and not
simply add yet on other hot fix.

re,
 wh

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [patch v2] x86, efi: fix pointer math issue in handle_ramdisks()
@ 2012-03-18 14:33               ` walter harms
  0 siblings, 0 replies; 18+ messages in thread
From: walter harms @ 2012-03-18 14:33 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Dan Carpenter, Thomas Gleixner, Ingo Molnar, x86, Matt Fleming,
	Maarten Lankhorst, linux-kernel, kernel-janitors



Am 16.03.2012 20:55, schrieb H. Peter Anvin:
> On 03/06/2012 12:44 AM, walter harms wrote:
>>>
>> hi,
>>
>> yep, but i was asking about efi. The basic idea is of cause to map efi_char16_t -> wchar_t
>> and back and make this a prototype for every driver that needs a special charset.
>> That would make it possible to recycle the wcs* interface of libc.
>> IMHO it seems more reasonable than adding one for each (upcoming) type.
>>
> 
> Actually libc and the C standard is going the opposite ways, adding new
> interfaces for UTF-16 and UTF-32.  The wchar_t interface just doesn't
> work very well.
> 

I admit i do not have the problem yet and i do not like the idea adding even more types.
Oh well may i should be happy that someone is actually thinking about this here and not
simply add yet on other hot fix.

re,
 wh

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2012-03-18 14:34 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-02 19:01 [patch] x86, efi: fix pointer math issue in handle_ramdisks() Dan Carpenter
2012-03-02 19:01 ` Dan Carpenter
2012-03-03  7:54 ` Ingo Molnar
2012-03-03  7:54   ` Ingo Molnar
2012-03-05 18:06   ` [patch v2] " Dan Carpenter
2012-03-05 18:06     ` Dan Carpenter
2012-03-05 18:42     ` walter harms
2012-03-05 18:42       ` walter harms
2012-03-05 19:33       ` H. Peter Anvin
2012-03-05 19:33         ` H. Peter Anvin
2012-03-06  8:44         ` walter harms
2012-03-06  8:44           ` walter harms
2012-03-16 19:55           ` H. Peter Anvin
2012-03-16 19:55             ` H. Peter Anvin
2012-03-18 14:33             ` walter harms
2012-03-18 14:33               ` walter harms
2012-03-16 15:20     ` Matt Fleming
2012-03-16 21:27     ` [tip:x86/urgent] x86, efi: Fix " tip-bot for Dan Carpenter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.