[PATCH xserver] modesetting: fix conn_id termination and potential overrun by 1 byte

[Pekka Paalanen]

On Mon, 10 Dec 2018 23:34:11 -0500
Ilia Mirkin <imirkin at alum.mit.edu> wrote:

> Noticed when porting this logic to xf86-video-nouveau, and valgrind
> complained about conditional jump based on uninitialized data.
> 
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> ---
> 
> memcpy sets conn_id[0..len-1], so conn_id[len] is the one that should
> get the 0.

Hi,

you're certainly right about memcpy vs. len. I didn't check the type of
conn_id, but if it's an array of bytes, then

Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.com>


Thanks,
pq

> 
>  hw/xfree86/drivers/modesetting/drmmode_display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c
> index 939f07f8f..5c1b0ea96 100644
> --- a/hw/xfree86/drivers/modesetting/drmmode_display.c
> +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c
> @@ -2834,7 +2834,7 @@ static int parse_path_blob(drmModePropertyBlobPtr path_blob, int *conn_base_id,
>      if (len + 1> 5)
>          return -1;
>      memcpy(conn_id, blob_data + 4, len);
> -    conn_id[len + 1] = '\0';
> +    conn_id[len] = '\0';
>      id = strtoul(conn_id, NULL, 10);
>  
>      *conn_base_id = id;

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.x.org/archives/xorg-devel/attachments/20181211/0ff78039/attachment.sig>