[igt-dev] [PATCH i-g-t] debugfs: Fix writing an extra zero out of bounds in igt_crc_to_string_extended()

[Jani Nikula]

On Thu, 31 Jan 2019, Maarten Lankhorst <maarten.lankhorst at linux.intel.com> wrote:
> Op 28-01-2019 om 13:24 schreef Jani Nikula:
>> On Mon, 28 Jan 2019, Maarten Lankhorst <maarten.lankhorst at linux.intel.com> wrote:
>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>>> ---
>>>  lib/igt_debugfs.c | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
>>> index 3656c66a5674..d1fc0ff7f710 100644
>>> --- a/lib/igt_debugfs.c
>>> +++ b/lib/igt_debugfs.c
>>> @@ -458,17 +458,17 @@ char *igt_crc_to_string_extended(igt_crc_t *crc, char delimiter, int crc_size)
>>>  	int i;
>>>  	int len = 0;
>>>  	int field_width = 2 * crc_size; /* Two chars per byte. */
>>> -	char *buf = malloc((field_width+1) * crc->n_words * sizeof(char));
>>> +	char *buf = malloc((field_width+1) * crc->n_words);
>>>  
>>>  	if (!buf)
>>>  		return NULL;
>>>  
>>> -	for (i = 0; i < crc->n_words; i++)
>>> +	for (i = 0; i < crc->n_words - 1; i++)
>>>  		len += sprintf(buf + len, "%0*x%c", field_width,
>>>  			       crc->crc[i], delimiter);
>> Or keep the loop condition and make this:
>>
>> 		len += sprintf(buf + len, "%s%0*x", i ? &delimiter : "",
>> 			       field_width, crc->crc[i]);
>>
>> Additionally could make the delimiter passed here a char*.
>
> That is also a valid solution. :) Is it ok to go with the solution I
> proposed or do you have a strong preference for that?
>
> I don't think we can just pass &delimiter, we would depend on the next
> byte in memory accidentally being \0.

D'oh. Yeah, you'd have to pass the delimiter as a string to make this
really work, or add a local buf. Meh. Something for the future.

On the original patch,

Reviewed-by: Jani Nikula <jani.nikula at intel.com>




-- 
Jani Nikula, Intel Open Source Graphics Center