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 <i...@freedesktop.org
<mailto:i...@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 <cheme...@gmail.com
    <mailto:cheme...@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@lists.freedesktop.org
    <mailto:mesa-dev@lists.freedesktop.org>
     > https://lists.freedesktop.org/mailman/listinfo/mesa-dev




_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to