[Mesa-dev] [PATCH] swrast: fix possible null dereference

[Brian Paul]

On 03/09/2016 04:37 PM, Lars Hamre wrote:
> I have not been able to force a NULL dereference, this is based off
> analyzing the code.
> Yes that is implicitly true, but if at some point the implicit
> relationship is broken, I would
> rather not have a NULL dereference.
>
> If you do not agree, I am fine deferring to your judgement!

I don't think the code in question will be hit if texObj is null.

This code is pretty old (perhaps 15 years) but has been used a lot.  I 
think we'd have known by now if there was a null pointer problem.

-Brian


>
> On Wed, Mar 9, 2016 at 6:23 PM, Ian Romanick <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
>
>     On 03/09/2016 10:21 AM, Lars Hamre wrote:
>      > Fixes a possible null dereference.
>      >
>      > NOTE: this is my first time contributing, please let me know if I
>      >       should be doing anything differently, thanks!
>      >
>      > Signed-off-by: Lars Hamre <chemecse at gmail.com
>     <mailto:chemecse at gmail.com>>
>      > ---
>      >  src/mesa/swrast/s_triangle.c | 7 ++++---
>      >  1 file changed, 4 insertions(+), 3 deletions(-)
>      >
>      > diff --git a/src/mesa/swrast/s_triangle.c
>     b/src/mesa/swrast/s_triangle.c
>      > index 876a74b..9225974 100644
>      > --- a/src/mesa/swrast/s_triangle.c
>      > +++ b/src/mesa/swrast/s_triangle.c
>      > @@ -781,7 +781,7 @@ fast_persp_span(struct gl_context *ctx,
>     SWspan *span,
>      >        }
>      >        break;
>      >     }
>      > -
>      > +
>      >     assert(span->arrayMask & SPAN_RGBA);
>      >     _swrast_write_rgba_span(ctx, span);
>      >
>      > @@ -1063,8 +1063,8 @@ _swrast_choose_triangle( struct gl_context
>     *ctx )
>      >           swImg = swrast_texture_image_const(texImg);
>      >
>      >           format = texImg ? texImg->TexFormat : MESA_FORMAT_NONE;
>      > -         minFilter = texObj2D ? samp->MinFilter : GL_NONE;
>      > -         magFilter = texObj2D ? samp->MagFilter : GL_NONE;
>      > +         minFilter = (texObj2D && samp) ? samp->MinFilter : GL_NONE;
>      > +         magFilter = (texObj2D && samp) ? samp->MagFilter : GL_NONE;
>
>     NAK this hunk.  If texObj2D is not NULL, samp is also not NULL.
>
>     >           envMode = ctx->Texture.Unit[0].EnvMode;
>     >
>     >           /* First see if we can use an optimized 2-D texture function */
>     > @@ -1073,6 +1073,7 @@ _swrast_choose_triangle( struct gl_context *ctx )
>     >               && !ctx->ATIFragmentShader._Enabled
>     >               && ctx->Texture._MaxEnabledTexImageUnit == 0
>     >               && ctx->Texture.Unit[0]._Current->Target == GL_TEXTURE_2D
>     > +             && samp
>
>     I think the 'ctx->Texture.Unit[0]._Current->Target == GL_TEXTURE_2D'
>     implicitly ensures that samp cannot be NULL.  Have you been able to
>     cause a NULL dereference in this code path or is this just based on
>     speculation?
>
>     >               && samp->WrapS == GL_REPEAT
>     >               && samp->WrapT == GL_REPEAT
>     >               && texObj2D->_Swizzle == SWIZZLE_NOOP
>     > --
>     > 2.5.0
>     >
>      > _______________________________________________
>      > mesa-dev mailing list
>      > mesa-dev at lists.freedesktop.org
>     <mailto:mesa-dev at lists.freedesktop.org>
>      > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>