[Mesa-dev] [PATCH 09/11] meta: Avoid null access on setup_glsl_msaa_blit_shader()

[Anuj Phogat]

On Mon, Sep 8, 2014 at 11:53 PM, Juha-Pekka Heikkila
<juhapekka.heikkila at gmail.com> wrote:
> On default fallback path there was null access on src_rb
>
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
> ---
>  src/mesa/drivers/common/meta_blit.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta_blit.c b/src/mesa/drivers/common/meta_blit.c
> index 3cd06a5..1603f43 100644
> --- a/src/mesa/drivers/common/meta_blit.c
> +++ b/src/mesa/drivers/common/meta_blit.c
> @@ -70,26 +70,28 @@ setup_glsl_msaa_blit_shader(struct gl_context *ctx,
>     const char *sampler_array_suffix = "";
>     char *name;
>     const char *texcoord_type = "vec2";
> -   const int samples = MAX2(src_rb->NumSamples, 1);
> +   int samples;
>     int shader_offset = 0;
>
> -   /* We expect only power of 2 samples in source multisample buffer. */
> -   assert((samples & (samples - 1)) == 0);
> -   while (samples >> (shader_offset + 1)) {
> -      shader_offset++;
> -   }
> -   /* Update the assert if we plan to support more than 16X MSAA. */
> -   assert(shader_offset >= 0 && shader_offset <= 4);
> -
>     if (src_rb) {
> +      samples = MAX2(src_rb->NumSamples, 1);
>        src_datatype = _mesa_get_format_datatype(src_rb->Format);
>     } else {
>        /* depth-or-color glCopyTexImage fallback path that passes a NULL rb and
>         * doesn't handle integer.
>         */
Unrelated to your changes, I'm not sure if we really need the else
case? Do we have a piglit test for src_rb==null case? Currently an
application will just crash in this function for such a case.

> +      samples = 1;
>        src_datatype = GL_UNSIGNED_NORMALIZED;
>     }
>
> +   /* We expect only power of 2 samples in source multisample buffer. */
> +   assert((samples & (samples - 1)) == 0);
> +   while (samples >> (shader_offset + 1)) {
> +      shader_offset++;
> +   }
> +   /* Update the assert if we plan to support more than 16X MSAA. */
> +   assert(shader_offset >= 0 && shader_offset <= 4);
> +
>     if (ctx->DrawBuffer->Visual.samples > 1) {
>        /* If you're calling meta_BlitFramebuffer with the destination
>         * multisampled, this is the only path that will work -- swrast and
Above changes look fine but we still have unguarded dereferencing of src_rb
(src_rb->_BaseFormat) in switch below this code.

Just add an assert(src_rb) at top if you decide to get rid of else case above.

> --
> 1.8.5.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev