| Message ID | 20180907073213.20410-1-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm: refuse ADDFB2 ioctl for broken bigendian drivers | expand |
On Fri, Sep 07, 2018 at 09:32:13AM +0200, Gerd Hoffmann wrote: > Drivers must set the quirk_addfb_prefer_host_byte_order quirk to make > the drm_mode_addfb() compat code work correctly on bigendian machines. > > If they don't they interpret pixel_format values incorrectly for bug > compatibility, which in turn implies the ADDFB2 ioctl does not work > correctly then. So block it to make userspace fallback to ADDFB. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/drm_crtc_internal.h | 6 ++++-- > drivers/gpu/drm/drm_framebuffer.c | 24 ++++++++++++++++++++++++ > drivers/gpu/drm/drm_ioctl.c | 2 +- > 3 files changed, 29 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h > index b613227633..6719e1c8df 100644 > --- a/drivers/gpu/drm/drm_crtc_internal.h > +++ b/drivers/gpu/drm/drm_crtc_internal.h > @@ -174,6 +174,8 @@ void drm_fb_release(struct drm_file *file_priv); > > int drm_mode_addfb(struct drm_device *dev, struct drm_mode_fb_cmd *or, > struct drm_file *file_priv); > +int drm_mode_addfb2(struct drm_device *dev, > + void *data, struct drm_file *file_priv); > int drm_mode_rmfb(struct drm_device *dev, u32 fb_id, > struct drm_file *file_priv); > > @@ -181,8 +183,8 @@ int drm_mode_rmfb(struct drm_device *dev, u32 fb_id, > /* IOCTL */ > int drm_mode_addfb_ioctl(struct drm_device *dev, > void *data, struct drm_file *file_priv); > -int drm_mode_addfb2(struct drm_device *dev, > - void *data, struct drm_file *file_priv); > +int drm_mode_addfb2_ioctl(struct drm_device *dev, > + void *data, struct drm_file *file_priv); > int drm_mode_rmfb_ioctl(struct drm_device *dev, > void *data, struct drm_file *file_priv); > int drm_mode_getfb(struct drm_device *dev, > diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c > index f863f8a20f..fb7528444c 100644 > --- a/drivers/gpu/drm/drm_framebuffer.c > +++ b/drivers/gpu/drm/drm_framebuffer.c > @@ -363,6 +363,30 @@ int drm_mode_addfb2(struct drm_device *dev, > return 0; > } > > +int drm_mode_addfb2_ioctl(struct drm_device *dev, > + void *data, struct drm_file *file_priv) > +{ > +#ifdef __BIG_ENDIAN > + if (!dev->mode_config.quirk_addfb_prefer_host_byte_order) { > + /* > + * Drivers must set the > + * quirk_addfb_prefer_host_byte_order quirk to make > + * the drm_mode_addfb() compat code work correctly on > + * bigendian machines. > + * > + * If they don't they interpret pixel_format values > + * incorrectly for bug compatibility, which in turn > + * implies the ADDFB2 ioctl does not work correctly > + * then. So block it to make userspace fallback to > + * ADDFB. > + */ > + DRM_DEBUG_KMS("addfb2 broken on bigendian"); > + return -EINVAL; > + } > +#endif > + return drm_mode_addfb2(dev, data, file_priv); > +} > + > struct drm_mode_rmfb_work { > struct work_struct work; > struct list_head fbs; > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index ea10e9a26a..6b4a633b42 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -645,7 +645,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { > DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPROPBLOB, drm_mode_getblob_ioctl, DRM_UNLOCKED), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETFB, drm_mode_getfb, DRM_UNLOCKED), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB, drm_mode_addfb_ioctl, DRM_UNLOCKED), > - DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB2, drm_mode_addfb2, DRM_UNLOCKED), > + DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB2, drm_mode_addfb2_ioctl, DRM_UNLOCKED), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_RMFB, drm_mode_rmfb_ioctl, DRM_UNLOCKED), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_PAGE_FLIP, drm_mode_page_flip_ioctl, DRM_MASTER|DRM_UNLOCKED), > DRM_IOCTL_DEF(DRM_IOCTL_MODE_DIRTYFB, drm_mode_dirtyfb_ioctl, DRM_MASTER|DRM_UNLOCKED), > -- > 2.9.3 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Gerd,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.19-rc2 next-20180906]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Gerd-Hoffmann/drm-refuse-ADDFB2-ioctl-for-broken-bigendian-drivers/20180910-142802
config: xtensa-allmodconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
drivers/gpu//drm/drm_framebuffer.c: In function 'drm_mode_addfb2_ioctl':
>> drivers/gpu//drm/drm_framebuffer.c:359:23: error: 'struct drm_mode_config' has no member named 'quirk_addfb_prefer_host_byte_order'
if (!dev->mode_config.quirk_addfb_prefer_host_byte_order) {
^
vim +359 drivers/gpu//drm/drm_framebuffer.c
354
355 int drm_mode_addfb2_ioctl(struct drm_device *dev,
356 void *data, struct drm_file *file_priv)
357 {
358 #ifdef __BIG_ENDIAN
> 359 if (!dev->mode_config.quirk_addfb_prefer_host_byte_order) {
360 /*
361 * Drivers must set the
362 * quirk_addfb_prefer_host_byte_order quirk to make
363 * the drm_mode_addfb() compat code work correctly on
364 * bigendian machines.
365 *
366 * If they don't they interpret pixel_format values
367 * incorrectly for bug compatibility, which in turn
368 * implies the ADDFB2 ioctl does not work correctly
369 * then. So block it to make userspace fallback to
370 * ADDFB.
371 */
372 DRM_DEBUG_KMS("addfb2 broken on bigendian");
373 return -EINVAL;
374 }
375 #endif
376 return drm_mode_addfb2(dev, data, file_priv);
377 }
378
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h index b613227633..6719e1c8df 100644 --- a/drivers/gpu/drm/drm_crtc_internal.h +++ b/drivers/gpu/drm/drm_crtc_internal.h @@ -174,6 +174,8 @@ void drm_fb_release(struct drm_file *file_priv); int drm_mode_addfb(struct drm_device *dev, struct drm_mode_fb_cmd *or, struct drm_file *file_priv); +int drm_mode_addfb2(struct drm_device *dev, + void *data, struct drm_file *file_priv); int drm_mode_rmfb(struct drm_device *dev, u32 fb_id, struct drm_file *file_priv); @@ -181,8 +183,8 @@ int drm_mode_rmfb(struct drm_device *dev, u32 fb_id, /* IOCTL */ int drm_mode_addfb_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); -int drm_mode_addfb2(struct drm_device *dev, - void *data, struct drm_file *file_priv); +int drm_mode_addfb2_ioctl(struct drm_device *dev, + void *data, struct drm_file *file_priv); int drm_mode_rmfb_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); int drm_mode_getfb(struct drm_device *dev, diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index f863f8a20f..fb7528444c 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -363,6 +363,30 @@ int drm_mode_addfb2(struct drm_device *dev, return 0; } +int drm_mode_addfb2_ioctl(struct drm_device *dev, + void *data, struct drm_file *file_priv) +{ +#ifdef __BIG_ENDIAN + if (!dev->mode_config.quirk_addfb_prefer_host_byte_order) { + /* + * Drivers must set the + * quirk_addfb_prefer_host_byte_order quirk to make + * the drm_mode_addfb() compat code work correctly on + * bigendian machines. + * + * If they don't they interpret pixel_format values + * incorrectly for bug compatibility, which in turn + * implies the ADDFB2 ioctl does not work correctly + * then. So block it to make userspace fallback to + * ADDFB. + */ + DRM_DEBUG_KMS("addfb2 broken on bigendian"); + return -EINVAL; + } +#endif + return drm_mode_addfb2(dev, data, file_priv); +} + struct drm_mode_rmfb_work { struct work_struct work; struct list_head fbs; diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index ea10e9a26a..6b4a633b42 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -645,7 +645,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETPROPBLOB, drm_mode_getblob_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETFB, drm_mode_getfb, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB, drm_mode_addfb_ioctl, DRM_UNLOCKED), - DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB2, drm_mode_addfb2, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_MODE_ADDFB2, drm_mode_addfb2_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_RMFB, drm_mode_rmfb_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_PAGE_FLIP, drm_mode_page_flip_ioctl, DRM_MASTER|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_DIRTYFB, drm_mode_dirtyfb_ioctl, DRM_MASTER|DRM_UNLOCKED),
Drivers must set the quirk_addfb_prefer_host_byte_order quirk to make the drm_mode_addfb() compat code work correctly on bigendian machines. If they don't they interpret pixel_format values incorrectly for bug compatibility, which in turn implies the ADDFB2 ioctl does not work correctly then. So block it to make userspace fallback to ADDFB. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- drivers/gpu/drm/drm_crtc_internal.h | 6 ++++-- drivers/gpu/drm/drm_framebuffer.c | 24 ++++++++++++++++++++++++ drivers/gpu/drm/drm_ioctl.c | 2 +- 3 files changed, 29 insertions(+), 3 deletions(-)