Re: [RFC] [PATCH]: ACPI: Rename ACPI processor device bus ID

[yakui_zhao <yakui.zhao@intel.com>]

On Sun, 2009-05-24 at 05:01 +0800, Henrique de Moraes Holschuh wrote:
> On Sat, 23 May 2009, yakui_zhao wrote:
> > On Sat, 2009-05-23 at 11:18 +0800, Henrique de Moraes Holschuh wrote:
> > > On Thu, 21 May 2009, yakui_zhao wrote:
> > > > +	sprintf(acpi_device_bid(device), "CPU%X", pr->id);
> > > 
> > > Is this safe against overflows, i.e. is pr->id something *we* set?  Because
> > > if it is in any way read from the ACPI firmware, you have to either use
> > > snprintf, or use the format string to limit the %X to a safe lenght...
> > Thanks for pointing out this issue.
> > Now the array size of acpi_bus_id is 5. And when the cpu number is above
> > 256, the overflow will happen. But it is very luck that the following
> > three bytes are not used by other variable because of align. And this
> > still can work.
> > Of course I already sent a patch, in which the array size is changed
> > from 5 to 8.
> > 
> > At the same time if the cpu number is less than or equal to 256, the
> > length of format string is safe.
> 
> Yeah, but I was really asking if, even with space for 8 chars, isn't there a
> risk of pr->id being, say, 0xfffffffe due to some wierdness...

If the pr->id is set to 0xfffffffe, it is a risk.
But in fact OS will get the correct cpu id by using the function of
get_cpu_id. The result value is -1 or other correct cpu id.

So it is unnecessary to worry that the incorrect value is assigned to
pr->id.

Of course it will be OK to use the snprintf instead of sprintf.
thanks.
> If there is such a risk, you should use snprintf, or a a length limit in the
> format...

>