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