On Thu, Aug 03, 2023 at 08:48:56AM -0400, Justin Green wrote: > > See c91acda3a380 ("drm/gem: Check for valid formats") and the related gem > fb helper functions to see how this is supposed to be done. > > Oh that's interesting, so does this imply that the infrastructure > automatically calls format_mod_supported() during framebuffer > creation? In that case, this entire patch might be unnecessary in the > tip of tree kernel. It /should/, but maybe a wheel fell off somewhere. So please double-check that it doesn indeed work. Also because we had to put the check into gem helpers, if your driver doesn't use those but hand-rolls a bit of that code (the helpers predate a bunch of drivers, not sure all got converted), then you might also have a validation gap here. Cheers, Sima > > On Thu, Aug 3, 2023 at 4:24 AM Daniel Vetter <daniel@xxxxxxxx> wrote: > > > > On Mon, Jul 24, 2023 at 01:58:39PM -0400, Justin Green wrote: > > > Add a check to mtk_drm_mode_fb_create() that rejects any modifier that > > > is not the AFBC mode supported by MT8195's display overlays. > > > > > > Tested by booting ChromeOS and verifying the UI works, and by running > > > the ChromeOS kms_addfb_basic binary, which has a test called > > > "addfb25-bad-modifier" that attempts to create a framebuffer with the > > > modifier DRM_FORMAT_MOD_INVALID and verifies the ADDFB2 ioctl returns > > > EINVAL. > > > > > > Signed-off-by: Justin Green <greenjustin@xxxxxxxxxxxx> > > > --- > > > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 7 +++++++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > > index cd5b18ef7951..2096e8a794ad 100644 > > > --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > > +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c > > > @@ -51,6 +51,13 @@ mtk_drm_mode_fb_create(struct drm_device *dev, > > > if (info->num_planes != 1) > > > return ERR_PTR(-EINVAL); > > > > > > + if (cmd->modifier[0] && > > > + cmd->modifier[0] != DRM_FORMAT_MOD_ARM_AFBC( > > > + AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 | > > > + AFBC_FORMAT_MOD_SPLIT | > > > + AFBC_FORMAT_MOD_SPARSE)) > > > + return ERR_PTR(-EINVAL); > > > > If you set the modifiers/format properties correctly and use all the > > helpers then the drm core should already validate that only formats you > > can use are allowed. > > > > See c91acda3a380 ("drm/gem: Check for valid formats") and the related gem > > fb helper functions to see how this is supposed to be done. These kind of > > checks in driver code for gem drivers (which really is everyone at this > > point) should not be needed at all. > > > > Cheers, Sima > > > > > + > > > return drm_gem_fb_create(dev, file, cmd); > > > } > > > > > > -- > > > 2.41.0.162.gfafddb0af9-goog > > > > > > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch