On Fri, 08 Jun 2018, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> Hello Jani Nikula,
>
> The patch 2a33d93486f2: "drm/i915/bios: add support for MIPI sequence
> block v3" from Jan 11, 2016, leads to the following static checker
> warning:
>
> drivers/gpu/drm/i915/intel_bios.c:926 goto_next_sequence_v3()
> warn: potentially one past the end of array 'data[index]'
>
> drivers/gpu/drm/i915/intel_bios.c
> 897 /* Skip Sequence Byte. */
> 898 index++;
> 899
> 900 /*
> 901 * Size of Sequence. Excludes the Sequence Byte and the size itself,
> 902 * includes MIPI_SEQ_ELEM_END byte, excludes the final MIPI_SEQ_END
> 903 * byte.
> 904 */
> 905 size_of_sequence = *((const uint32_t *)(data + index));
> 906 index += 4;
> 907
> 908 seq_end = index + size_of_sequence;
> 909 if (seq_end > total) {
> 910 DRM_ERROR("Invalid sequence size\n");
> 911 return 0;
> 912 }
> 913
> 914 for (; index < total; index += len) {
The data being parsed here is a sort of TLV coded blob with len here
referring to the payload length.
It's a sort of TLV coded blob with len here referring to the payload
length. T being the 1-byte operation_byte, L being the 1-byte len.
> 915 u8 operation_byte = *(data + index);
index is now at T, or operation byte.
> 916 index++;
> ^^^^^^^
index is now at L, or length.
> 917
> 918 if (operation_byte == MIPI_SEQ_ELEM_END) {
it could also be a marker for end of the whole thing, in which case the
operation_byte is 0.
> 919 if (index != seq_end) {
> 920 DRM_ERROR("Invalid element structure\n");
> 921 return 0;
> 922 }
> 923 return index;
> 924 }
> 925
> 926 len = *(data + index);
> ^^^^^^^^^^^^^^^^^^^^^
> This does look to uninitiated eyes as if it might be one past the end?
>
> 927 index++;
index is now at the payload, which is len bytes.
Makes sense? N.b. I didn't specify the format...
BR,
Jani.
> 928
> 929 /*
> 930 * FIXME: Would be nice to check elements like for v1/v2 in
> 931 * goto_next_sequence() above.
> 932 */
> 933 switch (operation_byte) {
> 934 case MIPI_SEQ_ELEM_SEND_PKT:
> 935 case MIPI_SEQ_ELEM_DELAY:
> 936 case MIPI_SEQ_ELEM_GPIO:
> 937 case MIPI_SEQ_ELEM_I2C:
> 938 case MIPI_SEQ_ELEM_SPI:
> 939 case MIPI_SEQ_ELEM_PMIC:
> 940 break;
> 941 default:
>
> regards,
> dan carpenter
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
