[PATCH] fix uninitialized string in get_model_name()

[PATCH] fix uninitialized string in get_model_name()

[Stephane Eranian]

Hello,

If ia64_pal_get_brand_info() fails in get_model_name() we may be copying
an invalid string.

changelog:
	- initialize brand[] in get_model_name()

signed-off-by: stephane eranian <eranian@hpl.hp.com>

diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index 2fd4b7d..cc82f6a 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -616,6 +616,8 @@ get_model_name(__u8 family, __u8 model)
 {
 	char brand[128];
 
+	memset(brand, 0, sizeof(brand));
+
 	if (ia64_pal_get_brand_info(brand)) {
 		if (family = 0x7)
 			memcpy(brand, "Merced", 7);
-- 

-Stephane

RE: [PATCH] fix uninitialized string in get_model_name()

[Luck, Tony]

> If ia64_pal_get_brand_info() fails in get_model_name() we may be copying
> an invalid string.

I'm not sure I see how.  If ia64_pal_get_brand_info() succeeds, it returns
0, and must have copied a brand string to brand[].  If it fails (returns non-zero)
we look family and model and fill in brand[] with something.

I suppose that techincally there is a gap in the switch statement for
model 0x1f as there is no default: case ... but I'm sure that 0, 1, 2
cover all the models that were released with family 0x1f.

> +	memset(brand, 0, sizeof(brand));

But if I missed something, this is overkill, there is no need to
fill the entire brand[] array with NUL ... just:

	brand[0] = '\0';

should be enough.  Or you could use memcpy(brand, "Unknown", 8) and
delete it from the "else" clause just below.

-Tony

Ahh ... do you have a Montecito with some pre-production PAL that
doesn't implement PAL_GET_BRAND_INFO?  That could mess up as we
don't cover family = 0x20 inside the 'if' statement (as I thought
that this PAL call was implemented early enough in Montecito that
everyone would have it).

Re: [PATCH] fix uninitialized string in get_model_name()

[Stephane Eranian]

On Fri, Dec 01, 2006 at 09:15:05AM -0800, Luck, Tony wrote:
> > If ia64_pal_get_brand_info() fails in get_model_name() we may be copying
> > an invalid string.
> 
> I'm not sure I see how.  If ia64_pal_get_brand_info() succeeds, it returns
> 0, and must have copied a brand string to brand[].  If it fails (returns non-zero)
> we look family and model and fill in brand[] with something.
> 
Ah, yes, now I see that my logic was wrong on the return value of
ia64_pal_get_brand_info(), so I think the code is fine.

> 
> Ahh ... do you have a Montecito with some pre-production PAL that
> doesn't implement PAL_GET_BRAND_INFO?  That could mess up as we
> don't cover family = 0x20 inside the 'if' statement (as I thought
> that this PAL call was implemented early enough in Montecito that
> everyone would have it).

This can be a problem.

-- 
-Stephane