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

[Maarten Lankhorst]

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.

~Maarten