Re: drm: fix blob pointer check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dan,

Thanks a lot for pointing this out. I saw you previous comment but didn't realize the issue.
I'll set the pointer to NULL.

I don't if there is an agreement on this, but do you think the unref/free functions should check for error pointers?

-
Lionel

On 11/03/16 11:39, Dan Carpenter wrote:
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux