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

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

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@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);
 
-	/* Eat the last delimiter */
-	buf[len - 1] = '\0';
+	sprintf(buf + len, "%0*x", field_width, crc->crc[i]);
+
 	return buf;
 }
 
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Jani Nikula]

On Mon, 28 Jan 2019, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@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*.

BR,
Jani.

>  
> -	/* Eat the last delimiter */
> -	buf[len - 1] = '\0';
> +	sprintf(buf + len, "%0*x", field_width, crc->crc[i]);
> +
>  	return buf;
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [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@linux.intel.com> wrote:
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@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

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [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@linux.intel.com> wrote:
> Op 28-01-2019 om 13:24 schreef Jani Nikula:
>> On Mon, 28 Jan 2019, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@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@intel.com>




-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [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@linux.intel.com> wrote:
>> Op 28-01-2019 om 13:24 schreef Jani Nikula:
>>> On Mon, 28 Jan 2019, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@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@intel.com>
>
>
>
>
Thanks, pushed. :)

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for debugfs: Fix writing an extra zero out of bounds in igt_crc_to_string_extended()

[Patchwork]

== Series Details ==

Series: debugfs: Fix writing an extra zero out of bounds in igt_crc_to_string_extended()
URL   : https://patchwork.freedesktop.org/series/55832/
State : success

== Summary ==

CI Bug Log - changes from IGT_4794 -> IGTPW_2305
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/55832/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2305 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> INCOMPLETE [fdo#102657]
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  
#### Possible fixes ####

  * igt@pm_rpm@basic-pci-d3-state:
    - fi-byt-n2820:       {SKIP} [fdo#109271] -> PASS

  * igt@pm_rpm@basic-rte:
    - fi-byt-n2820:       FAIL [fdo#108800] -> PASS

  
  [fdo#102657]: https://bugs.freedesktop.org/show_bug.cgi?id=102657
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271


Participating hosts (45 -> 40)
------------------------------

  Missing    (5): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-icl-y 


Build changes
-------------

    * IGT: IGT_4794 -> IGTPW_2305

  CI_DRM_5492: bc2fcdf82a5b94f015126dfa9db03f88feb9ae17 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2305: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2305/
  IGT_4794: 4a94878797999d1d2f7d224c6cf2e2855cd3109a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2305/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.IGT: success for debugfs: Fix writing an extra zero out of bounds in igt_crc_to_string_extended()

[Patchwork]

== Series Details ==

Series: debugfs: Fix writing an extra zero out of bounds in igt_crc_to_string_extended()
URL   : https://patchwork.freedesktop.org/series/55832/
State : success

== Summary ==

CI Bug Log - changes from IGT_4794_full -> IGTPW_2305_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/55832/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2305_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@reset-stress:
    - shard-snb:          PASS -> INCOMPLETE [fdo#105411]

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-kbl:          PASS -> FAIL [fdo#106641]

  * igt@kms_busy@extended-modeset-hang-newfb-render-c:
    - shard-glk:          NOTRUN -> DMESG-WARN [fdo#107956] +1

  * igt@kms_busy@extended-pageflip-hang-newfb-render-c:
    - shard-apl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-kbl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_color@pipe-c-ctm-max:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108147]
    - shard-apl:          NOTRUN -> FAIL [fdo#108147]

  * igt@kms_color@pipe-c-degamma:
    - shard-kbl:          NOTRUN -> FAIL [fdo#104782]
    - shard-apl:          PASS -> FAIL [fdo#104782]

  * igt@kms_content_protection@legacy:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108597]

  * igt@kms_cursor_crc@cursor-128x128-onscreen:
    - shard-kbl:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-apl:          PASS -> FAIL [fdo#103191] / [fdo#103232]

  * igt@kms_cursor_crc@cursor-256x85-onscreen:
    - shard-apl:          NOTRUN -> FAIL [fdo#103232] +1

  * igt@kms_cursor_crc@cursor-256x85-sliding:
    - shard-kbl:          PASS -> FAIL [fdo#103232] +3
    - shard-apl:          PASS -> FAIL [fdo#103232] +3
    - shard-glk:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-kbl:          PASS -> FAIL [fdo#103191] / [fdo#103232]

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-glk:          PASS -> FAIL [fdo#102887] / [fdo#105363]

  * igt@kms_flip@flip-vs-modeset-interruptible:
    - shard-hsw:          PASS -> DMESG-WARN [fdo#102614]

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665] +1

  * igt@kms_plane@plane-position-covered-pipe-b-planes:
    - shard-glk:          PASS -> FAIL [fdo#103166]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
    - shard-glk:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
    - shard-kbl:          PASS -> FAIL [fdo#103166]
    - shard-glk:          NOTRUN -> FAIL [fdo#103166]

  * igt@kms_sysfs_edid_timing:
    - shard-kbl:          NOTRUN -> FAIL [fdo#100047]

  
#### Possible fixes ####

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-apl:          FAIL [fdo#103191] / [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - shard-apl:          FAIL [fdo#103232] -> PASS +6
    - shard-kbl:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-64x64-sliding:
    - shard-glk:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-size-change:
    - shard-hsw:          DMESG-FAIL [fdo#102614] / [fdo#103232] -> PASS

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-max:
    - shard-glk:          FAIL [fdo#108145] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
    - shard-apl:          FAIL [fdo#103166] -> PASS +3

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-yf:
    - shard-kbl:          FAIL [fdo#103166] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-none:
    - shard-glk:          FAIL [fdo#103166] -> PASS +5

  * igt@pm_rc6_residency@rc6-accuracy:
    - shard-snb:          {SKIP} [fdo#109271] -> PASS

  * igt@prime_busy@hang-vebox:
    - shard-hsw:          FAIL [fdo#108807] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108597]: https://bugs.freedesktop.org/show_bug.cgi?id=108597
  [fdo#108807]: https://bugs.freedesktop.org/show_bug.cgi?id=108807
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109373]: https://bugs.freedesktop.org/show_bug.cgi?id=109373
  [fdo#109467]: https://bugs.freedesktop.org/show_bug.cgi?id=109467
  [k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321


Participating hosts (6 -> 5)
------------------------------

  Missing    (1): shard-skl 


Build changes
-------------

    * IGT: IGT_4794 -> IGTPW_2305

  CI_DRM_5492: bc2fcdf82a5b94f015126dfa9db03f88feb9ae17 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2305: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2305/
  IGT_4794: 4a94878797999d1d2f7d224c6cf2e2855cd3109a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2305/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev