[PATCH] drm: Pad drm_mode_get_connector to 64-bit boundary

[Ben Hutchings]

On Wed, 2013-10-16 at 13:09 +0300, Ville Syrjälä wrote:
> On Wed, Oct 16, 2013 at 09:49:02AM +0100, Chris Wilson wrote:
> > Pavel Roskin reported that DRM_IOCTL_MODE_GETCONNECTOR was overwritting
> > the 4 bytes beyond the end of its structure with a 32-bit userspace
> > running on a 64-bit kernel. This is due to the padding gcc inserts as
> > the drm_mode_get_connector struct includes a u64 and its size is not a
> > natural multiple of u64s.
> > 
> > 64-bit kernel:
> > 
> > sizeof(drm_mode_get_connector)=80, alignof=8
> > sizeof(drm_mode_get_encoder)=20, alignof=4
> > sizeof(drm_mode_modeinfo)=68, alignof=4
> > 
> > 32-bit userspace:
> > 
> > sizeof(drm_mode_get_connector)=76, alignof=4
> > sizeof(drm_mode_get_encoder)=20, alignof=4
> > sizeof(drm_mode_modeinfo)=68, alignof=4
> > 
> > Fortuituously we can insert explicit padding to the tail of our
> > structures without breaking ABI.
> > 
> > Reported-by: Pavel Roskin <proski at gnu.org>
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Dave Airlie <airlied at redhat.com>
> > Cc: dri-devel at lists.freedesktop.org
> > Cc: stable at vger.kernel.org
> 
> Hmm. But that only fixes things if you recompile the 32bit userland
> code.

Which is not a fix at all, but an even worse ABI break (now 32-bit
kernels will overrun userland buffers too).

> We could also fix old 32bit userland by adopting the same kind of size
> handling that we use for driver specific ioctls. The code is already
> there, we just need to set asize and usize appropriately.

Right, you have to do something like that.

Ben.

> > ---
> >  include/uapi/drm/drm_mode.h | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> > index 550811712f78..28acbaf4a81e 100644
> > --- a/include/uapi/drm/drm_mode.h
> > +++ b/include/uapi/drm/drm_mode.h
> > @@ -223,6 +223,8 @@ struct drm_mode_get_connector {
> >  	__u32 connection;
> >  	__u32 mm_width, mm_height; /**< HxW in millimeters */
> >  	__u32 subpixel;
> > +
> > +	__u32 pad;
> >  };
> >  
> >  #define DRM_MODE_PROP_PENDING	(1<<0)
> > -- 
> > 1.8.4.rc3
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Ben Hutchings
Horngren's Observation:
                   Among economists, the real world is often a special case.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 828 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20131017/381036d4/attachment.pgp>