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

[Marc-André Lureau]

Hi

----- Original Message -----
> In vrend clear dispatch function, the 'buffers' is read from
> guest. A malicious guest can specify a bad 'buffers' to make
> a the function call util_format_is_pure_uint() even the
> 'ctx->sub->surf[i]' is NULL. This can cause a NULL pointer deref.
> Make a sanity check to avoid this.
> 
> 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 ?

> 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?

>                 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
> 
>