On Wed, Sep 20, 2023 at 03:02:27PM +0200, Matthieu Herrb wrote:
> On Wed, Sep 20, 2023 at 08:08:23AM +0200, Otto Moerbeek wrote:
> >
> > The other, a write after free that crashed the X server when running
> > picard was diagnosed by me. This one was a bit nasty, as it required
> > instrumenting malloc to print some extra info to find the root cause.
> >
> > The bug is that the call in
> > https://github.com/openbsd/xenocara/blob/master/xserver/Xext/xvdisp.c#L1002
> > overwrites the first 4 bytes of the chunk next to the one allocated on
> > line 995.
> >
> > A workaround is to allocate 4 bytes extra, matthieu@ will be looking
> > for a proper fix, as it requires knowledge of the X internals.
> >
>
> Hi,
>
> Thanks again for finding it. Can you try this patch ?
>
> ===
> Fix overflow in glamor_xv_query_image_attributes for NV12 image format
>
> This is a format with num_planes == 2, we have only 2 elements in
> offsets[] and pitches[]
>
> Bug found by Otto Moerbeek on OpenBSD using his strict malloc checking.
> ---
> glamor/glamor_xv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/glamor/glamor_xv.c b/glamor/glamor_xv.c
> index a3d6b3bc3..e0e8e0ba9 100644
> --- a/glamor/glamor_xv.c
> +++ b/glamor/glamor_xv.c
> @@ -291,10 +291,10 @@ glamor_xv_query_image_attributes(int id,
> pitches[0] = size;
> size *= *h;
> if (offsets)
> - offsets[1] = offsets[2] = size;
> + offsets[1] = size;
> tmp = ALIGN(*w, 4);
> if (pitches)
> - pitches[1] = pitches[2] = tmp;
> + pitches[1] = tmp;
> tmp *= (*h >> 1);
> size += tmp;
> break;
> ---
> 2.42.0
Yes, that works for me,
-Otto
