Re: [RFC 1/7] drm/omap: Use devm_kzalloc() to allocate omap_drm_private

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Peter,

On Monday, 4 September 2017 14:16:32 EEST Peter Ujfalusi wrote:
> On 2017-09-04 12:41, Laurent Pinchart wrote:
> > At the moment the memory is freed at .remove() time, which can lead to
> > memory corruption if a user has a handle on the device (for instance an
> > open file handle that is then close()d). Fixing this requires moving
> > memory free to the drm_driver::release() handler. devm_kzalloc() goes in
> > the wrong direction.
> Ah, OK, so the current way is buggy as well.
> 
> How do you plan to fix that?
> I think this should work:
> 
> struct omap_drm_private {
> 	/* First member in the private struct! */
> +	struct drm_device ddev;
> ...
> };
> 
> Use drm_dev_init(&priv->ddev, ...); to initialize the drm_device instead
> of drm_dev_alloc()
> 
> then priv->ddev.dev_private = priv;
> 
> in this case the drm_dev_unref() would free up our omap_drm_private, right?

That's the idea, yes. I got a local patch for that in my tree.

> I think this is what other DRM drivers are doing, not all, but i915 does
> this at least.
> 
> But by the description most of the DRM drivers are doing this wrong, right?

Correct, most drivers get it wrong. We'll have to fix it, but given that we 
have race conditions in the core that prevent proper hot-unplug support at the 
moment, I didn't want to start pushing for fixing drivers. Once we get the 
core sorted out, it will be time to address the other side of the issue.

-- 
Regards,

Laurent Pinchart

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