[Mesa-dev] [PATCH 3/3] mesa: Verify memory allocations success in _mesa_PushAttrib

Brian Paul brianp at vmware.com
Mon Dec 9 09:05:54 PST 2013


On 12/09/2013 08:19 AM, Pohjolainen, Topi wrote:
> On Mon, Dec 09, 2013 at 02:29:21PM +0200, Juha-Pekka Heikkila wrote:
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
>> ---
>>   src/mesa/main/attrib.c | 320 +++++++++++++++++++++++++++++++++++++++++--------
>>   1 file changed, 272 insertions(+), 48 deletions(-)
>>
>> diff --git a/src/mesa/main/attrib.c b/src/mesa/main/attrib.c
>> index 87797d1..3083e21 100644
>> --- a/src/mesa/main/attrib.c
>> +++ b/src/mesa/main/attrib.c
>> @@ -224,40 +224,89 @@ _mesa_PushAttrib(GLbitfield mask)
>>      if (mask & GL_ACCUM_BUFFER_BIT) {
>>         struct gl_accum_attrib *attr;
>>         attr = MALLOC_STRUCT( gl_accum_attrib );
>> -      memcpy( attr, &ctx->Accum, sizeof(struct gl_accum_attrib) );
>> -      save_attrib_data(&head, GL_ACCUM_BUFFER_BIT, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_ACCUM_BUFFER_BIT, attr)) {
>> +         memcpy( attr, &ctx->Accum, sizeof(struct gl_accum_attrib) );
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>
> This pattern seems to repeat many times with only the GL_*_BIT being different.
> How about introducing a helper taking the bit as an argument?

You could at least put the attr==NULL test in save_attrib_data().


>
>>      }
>>
>>      if (mask & GL_COLOR_BUFFER_BIT) {
>>         GLuint i;
>>         struct gl_colorbuffer_attrib *attr;
>>         attr = MALLOC_STRUCT( gl_colorbuffer_attrib );
>> -      memcpy( attr, &ctx->Color, sizeof(struct gl_colorbuffer_attrib) );
>> -      /* push the Draw FBO's DrawBuffer[] state, not ctx->Color.DrawBuffer[] */
>> -      for (i = 0; i < ctx->Const.MaxDrawBuffers; i ++)
>> -         attr->DrawBuffer[i] = ctx->DrawBuffer->ColorDrawBuffer[i];
>> -      save_attrib_data(&head, GL_COLOR_BUFFER_BIT, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +    if (save_attrib_data(&head, GL_COLOR_BUFFER_BIT, attr)) {
>> +         memcpy( attr, &ctx->Color, sizeof(struct gl_colorbuffer_attrib) );
>> +         /* push the Draw FBO's DrawBuffer[] state, not ctx->Color.DrawBuffer[] */
>> +         for (i = 0; i < ctx->Const.MaxDrawBuffers; i ++)
>> +            attr->DrawBuffer[i] = ctx->DrawBuffer->ColorDrawBuffer[i];
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>      if (mask & GL_CURRENT_BIT) {
>>         struct gl_current_attrib *attr;
>>         FLUSH_CURRENT( ctx, 0 );
>>         attr = MALLOC_STRUCT( gl_current_attrib );
>> -      memcpy( attr, &ctx->Current, sizeof(struct gl_current_attrib) );
>> -      save_attrib_data(&head, GL_CURRENT_BIT, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_CURRENT_BIT, attr)) {
>> +         memcpy( attr, &ctx->Current, sizeof(struct gl_current_attrib) );
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>      if (mask & GL_DEPTH_BUFFER_BIT) {
>>         struct gl_depthbuffer_attrib *attr;
>>         attr = MALLOC_STRUCT( gl_depthbuffer_attrib );
>> -      memcpy( attr, &ctx->Depth, sizeof(struct gl_depthbuffer_attrib) );
>> -      save_attrib_data(&head, GL_DEPTH_BUFFER_BIT, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_DEPTH_BUFFER_BIT, attr)) {
>> +         memcpy( attr, &ctx->Depth, sizeof(struct gl_depthbuffer_attrib) );
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>      if (mask & GL_ENABLE_BIT) {
>>         struct gl_enable_attrib *attr;
>>         GLuint i;
>>         attr = MALLOC_STRUCT( gl_enable_attrib );
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>>         /* Copy enable flags from all other attributes into the enable struct. */
>>         attr->AlphaTest = ctx->Color.AlphaEnabled;
>>         attr->AutoNormal = ctx->Eval.AutoNormal;
>> @@ -324,7 +373,11 @@ _mesa_PushAttrib(GLbitfield mask)
>>         /* GL_ARB_fragment_program */
>>         attr->FragmentProgram = ctx->FragmentProgram.Enabled;
>>
>> -      save_attrib_data(&head, GL_ENABLE_BIT, attr);
>> +      if (!save_attrib_data(&head, GL_ENABLE_BIT, attr)) {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>
>>         /* GL_ARB_framebuffer_sRGB / GL_EXT_framebuffer_sRGB */
>>         attr->sRGBEnabled = ctx->Color.sRGBEnabled;
>> @@ -333,88 +386,220 @@ _mesa_PushAttrib(GLbitfield mask)
>>      if (mask & GL_EVAL_BIT) {
>>         struct gl_eval_attrib *attr;
>>         attr = MALLOC_STRUCT( gl_eval_attrib );
>> -      memcpy( attr, &ctx->Eval, sizeof(struct gl_eval_attrib) );
>> -      save_attrib_data(&head, GL_EVAL_BIT, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_EVAL_BIT, attr)) {
>> +         memcpy( attr, &ctx->Eval, sizeof(struct gl_eval_attrib) );
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>      if (mask & GL_FOG_BIT) {
>>         struct gl_fog_attrib *attr;
>>         attr = MALLOC_STRUCT( gl_fog_attrib );
>> -      memcpy( attr, &ctx->Fog, sizeof(struct gl_fog_attrib) );
>> -      save_attrib_data(&head, GL_FOG_BIT, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_FOG_BIT, attr)) {
>> +         memcpy( attr, &ctx->Fog, sizeof(struct gl_fog_attrib) );
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>      if (mask & GL_HINT_BIT) {
>>         struct gl_hint_attrib *attr;
>>         attr = MALLOC_STRUCT( gl_hint_attrib );
>> -      memcpy( attr, &ctx->Hint, sizeof(struct gl_hint_attrib) );
>> -      save_attrib_data(&head, GL_HINT_BIT, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_HINT_BIT, attr)) {
>> +         memcpy( attr, &ctx->Hint, sizeof(struct gl_hint_attrib) );
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>      if (mask & GL_LIGHTING_BIT) {
>>         struct gl_light_attrib *attr;
>>         FLUSH_CURRENT(ctx, 0);	/* flush material changes */
>>         attr = MALLOC_STRUCT( gl_light_attrib );
>> -      memcpy( attr, &ctx->Light, sizeof(struct gl_light_attrib) );
>> -      save_attrib_data(&head, GL_LIGHTING_BIT, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_LIGHTING_BIT, attr)) {
>> +         memcpy( attr, &ctx->Light, sizeof(struct gl_light_attrib) );
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>      if (mask & GL_LINE_BIT) {
>>         struct gl_line_attrib *attr;
>>         attr = MALLOC_STRUCT( gl_line_attrib );
>> -      memcpy( attr, &ctx->Line, sizeof(struct gl_line_attrib) );
>> -      save_attrib_data(&head, GL_LINE_BIT, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_LINE_BIT, attr)) {
>> +         memcpy( attr, &ctx->Line, sizeof(struct gl_line_attrib) );
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>      if (mask & GL_LIST_BIT) {
>>         struct gl_list_attrib *attr;
>>         attr = MALLOC_STRUCT( gl_list_attrib );
>> -      memcpy( attr, &ctx->List, sizeof(struct gl_list_attrib) );
>> -      save_attrib_data(&head, GL_LIST_BIT, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_LIST_BIT, attr)) {
>> +         memcpy( attr, &ctx->List, sizeof(struct gl_list_attrib) );
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>      if (mask & GL_PIXEL_MODE_BIT) {
>>         struct gl_pixel_attrib *attr;
>>         attr = MALLOC_STRUCT( gl_pixel_attrib );
>> -      memcpy( attr, &ctx->Pixel, sizeof(struct gl_pixel_attrib) );
>> -      /* push the Read FBO's ReadBuffer state, not ctx->Pixel.ReadBuffer */
>> -      attr->ReadBuffer = ctx->ReadBuffer->ColorReadBuffer;
>> -      save_attrib_data(&head, GL_PIXEL_MODE_BIT, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_PIXEL_MODE_BIT, attr)) {
>> +         memcpy( attr, &ctx->Pixel, sizeof(struct gl_pixel_attrib) );
>> +         /* push the Read FBO's ReadBuffer state, not ctx->Pixel.ReadBuffer */
>> +         attr->ReadBuffer = ctx->ReadBuffer->ColorReadBuffer;
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>      if (mask & GL_POINT_BIT) {
>>         struct gl_point_attrib *attr;
>>         attr = MALLOC_STRUCT( gl_point_attrib );
>> -      memcpy( attr, &ctx->Point, sizeof(struct gl_point_attrib) );
>> -      save_attrib_data(&head, GL_POINT_BIT, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_POINT_BIT, attr)) {
>> +         memcpy( attr, &ctx->Point, sizeof(struct gl_point_attrib) );
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>      if (mask & GL_POLYGON_BIT) {
>>         struct gl_polygon_attrib *attr;
>>         attr = MALLOC_STRUCT( gl_polygon_attrib );
>> -      memcpy( attr, &ctx->Polygon, sizeof(struct gl_polygon_attrib) );
>> -      save_attrib_data(&head, GL_POLYGON_BIT, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_POLYGON_BIT, attr)) {
>> +         memcpy( attr, &ctx->Polygon, sizeof(struct gl_polygon_attrib) );
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>      if (mask & GL_POLYGON_STIPPLE_BIT) {
>>         GLuint *stipple;
>>         stipple = malloc( 32*sizeof(GLuint) );
>> -      memcpy( stipple, ctx->PolygonStipple, 32*sizeof(GLuint) );
>> -      save_attrib_data(&head, GL_POLYGON_STIPPLE_BIT, stipple);
>> +      if (stipple == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_POLYGON_STIPPLE_BIT, stipple)) {
>> +         memcpy( stipple, ctx->PolygonStipple, 32*sizeof(GLuint) );
>> +      }
>> +      else {
>> +         FREE( stipple );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>      if (mask & GL_SCISSOR_BIT) {
>>         struct gl_scissor_attrib *attr;
>>         attr = MALLOC_STRUCT( gl_scissor_attrib );
>> -      memcpy( attr, &ctx->Scissor, sizeof(struct gl_scissor_attrib) );
>> -      save_attrib_data(&head, GL_SCISSOR_BIT, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_SCISSOR_BIT, attr)) {
>> +         memcpy( attr, &ctx->Scissor, sizeof(struct gl_scissor_attrib) );
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>      if (mask & GL_STENCIL_BUFFER_BIT) {
>>         struct gl_stencil_attrib *attr;
>>         attr = MALLOC_STRUCT( gl_stencil_attrib );
>> -      memcpy( attr, &ctx->Stencil, sizeof(struct gl_stencil_attrib) );
>> -      save_attrib_data(&head, GL_STENCIL_BUFFER_BIT, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_STENCIL_BUFFER_BIT, attr)) {
>> +         memcpy( attr, &ctx->Stencil, sizeof(struct gl_stencil_attrib) );
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>      if (mask & GL_TEXTURE_BIT) {
>> @@ -426,6 +611,12 @@ _mesa_PushAttrib(GLbitfield mask)
>>            goto end;
>>         }
>>
>> +      if (!save_attrib_data(&head, GL_TEXTURE_BIT, texstate)) {
>> +         FREE( texstate );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib(GL_TEXTURE_BIT)");
>> +         goto end;
>> +      }
>> +
>>         _mesa_lock_context_textures(ctx);
>>
>>         /* copy/save the bulk of texture state here */
>> @@ -452,35 +643,68 @@ _mesa_PushAttrib(GLbitfield mask)
>>         _mesa_reference_shared_state(ctx, &texstate->SharedRef, ctx->Shared);
>>
>>         _mesa_unlock_context_textures(ctx);
>> -
>> -      save_attrib_data(&head, GL_TEXTURE_BIT, texstate);
>>      }
>>
>>      if (mask & GL_TRANSFORM_BIT) {
>>         struct gl_transform_attrib *attr;
>>         attr = MALLOC_STRUCT( gl_transform_attrib );
>> -      memcpy( attr, &ctx->Transform, sizeof(struct gl_transform_attrib) );
>> -      save_attrib_data(&head, GL_TRANSFORM_BIT, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_TRANSFORM_BIT, attr)) {
>> +         memcpy( attr, &ctx->Transform, sizeof(struct gl_transform_attrib) );
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>      if (mask & GL_VIEWPORT_BIT) {
>>         struct gl_viewport_attrib *attr;
>>         attr = MALLOC_STRUCT( gl_viewport_attrib );
>> -      memcpy( attr, &ctx->Viewport, sizeof(struct gl_viewport_attrib) );
>> -      save_attrib_data(&head, GL_VIEWPORT_BIT, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_VIEWPORT_BIT, attr)) {
>> +         memcpy( attr, &ctx->Viewport, sizeof(struct gl_viewport_attrib) );
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>      /* GL_ARB_multisample */
>>      if (mask & GL_MULTISAMPLE_BIT_ARB) {
>>         struct gl_multisample_attrib *attr;
>>         attr = MALLOC_STRUCT( gl_multisample_attrib );
>> -      memcpy( attr, &ctx->Multisample, sizeof(struct gl_multisample_attrib) );
>> -      save_attrib_data(&head, GL_MULTISAMPLE_BIT_ARB, attr);
>> +      if (attr == NULL) {
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>> +
>> +      if (save_attrib_data(&head, GL_MULTISAMPLE_BIT_ARB, attr)) {
>> +         memcpy( attr, &ctx->Multisample, sizeof(struct gl_multisample_attrib) );
>> +      }
>> +      else {
>> +         FREE( attr );
>> +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glPushAttrib");
>> +         goto end;
>> +      }
>>      }
>>
>>   end:
>> -   ctx->AttribStack[ctx->AttribStackDepth] = head;
>> -   ctx->AttribStackDepth++;
>> +   if (head != NULL) {
>> +       ctx->AttribStack[ctx->AttribStackDepth] = head;
>> +       ctx->AttribStackDepth++;
>> +   }
>>   }
>>
>>
>> --
>> 1.8.1.2
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/mesa-dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=lGQMzzTgII0I7jefp2FHq7WtZ%2BTLs8wadB%2BiIj9xpBY%3D%0A&m=3aP4pdVWTMd4H%2BjF2PgJxyWAT6ABIiO%2BVZf1QwGEA90%3D%0A&s=2d814eba0fcef75b020f47db59dd21a93297d4686be9ff11c6e4a171fc85cc86
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/mesa-dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=lGQMzzTgII0I7jefp2FHq7WtZ%2BTLs8wadB%2BiIj9xpBY%3D%0A&m=3aP4pdVWTMd4H%2BjF2PgJxyWAT6ABIiO%2BVZf1QwGEA90%3D%0A&s=2d814eba0fcef75b020f47db59dd21a93297d4686be9ff11c6e4a171fc85cc86
>



More information about the mesa-dev mailing list