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

[Maarten Lankhorst]

Op 01-02-2019 om 10:38 schreef 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>
>
>
>
>
Thanks, pushed. :)