[virglrenderer-devel] [PATCH] renderer: fix NULL pointer deref in vrend_clear

[Li Qiang]

Hi,

2017-01-06 1:04 GMT+08:00 Marc-André Lureau <mlureau at redhat.com>:

>
> > Signed-off-by: Li Qiang <liq3ea at gmail.com>
> > ---
> >  src/vrend_renderer.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/vrend_renderer.c b/src/vrend_renderer.c
> > index 00b61eb..cd8055d 100644
> > --- a/src/vrend_renderer.c
> > +++ b/src/vrend_renderer.c
> > @@ -2354,10 +2354,10 @@ void vrend_clear(struct vrend_context *ctx,
> >           mask = buffers >> 2;
> >           while (mask) {
> >              i = u_bit_scan(&mask);
> > -            if (util_format_is_pure_uint(ctx->sub->surf[i]->format))
> > +            if (i < 8 && ctx->sub->surf[i] &&
>
> I would rather introduce a define, VREND_NDRAWBUFFERS ?
>
>
True, I would like to use a define, but as the definition of 'surf' is hard
coded, so do I.

struct vrend_sub_context {
   struct list_head head;

   ...
   uint32_t fb_id;
   int nr_cbufs, old_nr_cbufs;
   struct vrend_surface *zsurf;
   struct vrend_surface *surf[8];


> util_format_is_pure_uint(ctx->sub->surf[i] && ctx->sub->surf[i]->format))
>
> That looks wrong, why do you check for ctx->sub->surf[i] twice here?
>
>

Here we deref ctx->sub->surf[i]->format, so we should be sure the 'i' is in
bound and the surf[i] is not NULL.
So if i >=8 or surf[i] is NULL in, it will fall in the 'else if', but here
we still do this sanity check because we
don't know the reason of first 'if' fails, by the sanity check or by the
call of util_format_is_pure_uint. I have thought
several solution but it seems every methods will have repeat code.

Any idea?



> >                 glClearBufferuiv(GL_COLOR,
> >                                  i, (GLuint *)color);
> > -            else if (util_format_is_pure_sint(ctx-
> >sub->surf[i]->format))
> > +            else if (i < 8 && ctx->sub->surf[i] &&
> > util_format_is_pure_sint(ctx->sub->surf[i] &&
> ctx->sub->surf[i]->format))
> >                 glClearBufferiv(GL_COLOR,
> >                                  i, (GLint *)color);
> >              else
>
> otherwise, looks ok
>
> > --
> > 2.7.4
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/virglrenderer-devel/attachments/20170106/ecfad8f9/attachment.html>