On Tue, Dec 11, 2018 at 3:23 AM Pekka Paalanen <[email protected]> wrote:
>
> On Mon, 10 Dec 2018 23:34:11 -0500
> Ilia Mirkin <[email protected]> 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 <[email protected]>
> > ---
> >
> > 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 <[email protected]>
Thanks! Here are the relevant bits of the function:
char conn_id[5];
...
len = conn - (blob_data + 4);
if (len + 1> 5)
return -1;
memcpy(conn_id, blob_data + 4, len);
conn_id[len] = '\0';
id = strtoul(conn_id, NULL, 10);
(previously that was conn_id[len+1])
>
>
> 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;
>
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
