* [patch] x86, efi: fix pointer math issue in handle_ramdisks()
@ 2012-03-02 19:01 Dan Carpenter
2012-03-03 7:54 ` Ingo Molnar
0 siblings, 1 reply; 10+ 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] 10+ messages in thread
* Re: [patch] x86, efi: fix pointer math issue in handle_ramdisks()
2012-03-02 19:01 [patch] x86, efi: fix pointer math issue in handle_ramdisks() Dan Carpenter
@ 2012-03-03 7:54 ` Ingo Molnar
2012-03-05 18:06 ` [patch v2] " Dan Carpenter
0 siblings, 1 reply; 10+ 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] 10+ 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
2012-03-05 18:42 ` walter harms
` (2 more replies)
0 siblings, 3 replies; 10+ 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] 10+ messages in thread
* Re: [patch v2] x86, efi: fix pointer math issue in handle_ramdisks()
2012-03-05 18:06 ` [patch v2] " Dan Carpenter
@ 2012-03-05 18:42 ` walter harms
2012-03-05 19:33 ` H. Peter Anvin
2012-03-16 15:20 ` Matt Fleming
2012-03-16 21:27 ` [tip:x86/urgent] x86, efi: Fix " tip-bot for Dan Carpenter
2 siblings, 1 reply; 10+ 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] 10+ 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
2012-03-06 8:44 ` walter harms
0 siblings, 1 reply; 10+ 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] 10+ 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
2012-03-16 19:55 ` H. Peter Anvin
0 siblings, 1 reply; 10+ 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] 10+ messages in thread
* Re: [patch v2] x86, efi: fix pointer math issue in handle_ramdisks()
2012-03-05 18:06 ` [patch v2] " Dan Carpenter
2012-03-05 18:42 ` 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
2 siblings, 0 replies; 10+ 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] 10+ 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
2012-03-18 14:33 ` walter harms
0 siblings, 1 reply; 10+ 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] 10+ messages in thread
* [tip:x86/urgent] x86, efi: Fix pointer math issue in handle_ramdisks()
2012-03-05 18:06 ` [patch v2] " Dan Carpenter
2012-03-05 18:42 ` walter harms
2012-03-16 15:20 ` Matt Fleming
@ 2012-03-16 21:27 ` tip-bot for Dan Carpenter
2 siblings, 0 replies; 10+ 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] 10+ 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
0 siblings, 0 replies; 10+ 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] 10+ messages in thread
end of thread, other threads:[~2012-03-18 14:34 UTC | newest]
Thread overview: 10+ 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-03 7:54 ` Ingo Molnar
2012-03-05 18:06 ` [patch v2] " Dan Carpenter
2012-03-05 18:42 ` walter harms
2012-03-05 19:33 ` H. Peter Anvin
2012-03-06 8:44 ` walter harms
2012-03-16 19:55 ` H. Peter Anvin
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).