linux-kernel - Re: [PATCH] drm: platform: Don't initialize driver-private data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1397581.cKqVYz2Abn@avalon>
Date:	Wed, 31 Oct 2012 09:31:27 +0100
From:	Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:	Thierry Reding <thierry.reding@...onic-design.de>
Cc:	David Airlie <airlied@...ux.ie>, dri-devel@...ts.freedesktop.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm: platform: Don't initialize driver-private data

Hi Thierry,

On Wednesday 31 October 2012 09:26:07 Thierry Reding wrote:
> On Fri, Oct 26, 2012 at 04:06:27PM +0200, Laurent Pinchart wrote:
> > On Monday 15 October 2012 20:03:42 Thierry Reding wrote:
> > > Platform device drivers usually use the driver-private data for their
> > > own purposes. Having it overwritten by drm_platform_init() is confusing
> > > and error-prone.
> > 
> > If you want to push drivers that way, you should get rid of the
> > pci_set_drvdata() call in core DRM as well. This would push device driver
> > data handling down to all drivers, so I'm not convinced it would actually
> > make things simpler.
> 
> I think the problem doesn't exist for PCI-based DRM drivers, so I didn't
> look at it. The issue only arises once the DRM needs to glue together
> multiple devices, as is usual with the drivers for embedded devices,
> where the drivers are based on platform devices.
> 
> I agree, though, that for consistency it would be nicer not to do this
> for the PCI-based DRM drivers either. If David agrees I can take a look
> at converting the other drivers along with the change to the DRM core.
> 
> Pushing the handling of the driver-private data down to the drivers may
> not make things easier, but at least it would be consistent with other
> drivers. I didn't mention this in the patch description but it actually
> took me a day to track down why the driver kept crashing until I figured
> out that drm_platform_init() actually modified the pointer.

So we either need your patch, or a documentation update :-)

The patch itself is fine, I'll let others comment on the approach.

-- 
Regards,

Laurent Pinchart

Download attachment "signature.asc" of type "application/pgp-signature" (491 bytes)