re: drm: fix blob pointer check

[Dan Carpenter <dan.carpenter@oracle.com>]

Hi Lionel Landwerlin,

The patch 562c5b4d8986: "drm: fix blob pointer check" from Mar 10,
2016, has a problem.

drivers/gpu/drm/drm_atomic_helper.c
  2924          blob = drm_property_create_blob(dev,
  2925                                          sizeof(struct drm_color_lut) * size,
  2926                                          NULL);
  2927          if (IS_ERR(blob)) {
  2928                  ret = PTR_ERR(blob);
  2929                  goto fail;

These types of goto fails are a trap for the unwary.

I deliberately reported the bug instead of fixing it because I am a jerk
and because last time when I did this jerky thing, people were not
convinced that they would fall for the trap every single time.

http://www.spinics.net/lists/cgroups/msg15262.html

  2930          }
  2931  

[ snip ]

  2973  fail:
  2974          if (ret == -EDEADLK)
  2975                  goto backoff;
  2976  
  2977          drm_atomic_state_free(state);
  2978          drm_property_unreference_blob(blob);
                                              ^^^^
Blob is an error pointer here so it will oops inside the function call.

The better way to write this is to unwind in the reverse order from the
allocations.  So since we allocated state first then we free it last.
Use explicit names based on what the goto does.  err_unreference:
err_free_state:.  Don't free things that haven't been allocated.

Or you could set "blob = NULL;" before the goto.

  2979  
  2980          return;
  2981  backoff:
  2982          drm_atomic_state_clear(state);
  2983          drm_atomic_legacy_backoff(state);
  2984  
  2985          goto retry;
  2986  }

regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel