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... > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
