| Message ID | 20230113112743.188486-2-mcanal@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Check for valid framebuffer's format | expand |
On Fri, Jan 13, 2023 at 08:27:42AM -0300, Maíra Canal wrote: > Currently, framebuffer_check() doesn't check if the pixel format is > supported, which can lead to the acceptance of invalid pixel formats > e.g. the acceptance of invalid modifiers. Therefore, add a check for > valid formats on framebuffer_check(), so that the ADDFB2 IOCTL rejects > calls with invalid formats. > > Moreover, note that this check is only valid for atomic drivers, > because, for non-atomic drivers, checking drm_any_plane_has_format() is > not possible since the format list for the primary plane is fake, and > we'd therefore reject valid formats. > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > Signed-off-by: Maíra Canal <mcanal@igalia.com> > --- > Documentation/gpu/todo.rst | 9 ++++----- > drivers/gpu/drm/drm_framebuffer.c | 8 ++++++++ > 2 files changed, 12 insertions(+), 5 deletions(-) > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > index 1f8a5ebe188e..3a79c26c5cc7 100644 > --- a/Documentation/gpu/todo.rst > +++ b/Documentation/gpu/todo.rst > @@ -276,11 +276,10 @@ Various hold-ups: > - Need to switch to drm_fbdev_generic_setup(), otherwise a lot of the custom fb > setup code can't be deleted. > > -- Many drivers wrap drm_gem_fb_create() only to check for valid formats. For > - atomic drivers we could check for valid formats by calling > - drm_plane_check_pixel_format() against all planes, and pass if any plane > - supports the format. For non-atomic that's not possible since like the format > - list for the primary plane is fake and we'd therefor reject valid formats. > +- Need to switch to drm_gem_fb_create(), as now framebuffer_check() checks for > + valid formats for atomic drivers. > + > +- Add an addfb format validation for non-atomic drivers. > > - Many drivers subclass drm_framebuffer, we'd need a embedding compatible > version of the varios drm_gem_fb_create functions. Maybe called > diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c > index aff3746dedfb..605642bf3650 100644 > --- a/drivers/gpu/drm/drm_framebuffer.c > +++ b/drivers/gpu/drm/drm_framebuffer.c > @@ -280,6 +280,14 @@ static int framebuffer_check(struct drm_device *dev, > } > } > > + /* Verify that the modifier is supported. */ > + if (drm_drv_uses_atomic_modeset(dev) && > + !drm_any_plane_has_format(dev, r->pixel_format, r->modifier[0])) { > + drm_dbg_kms(dev, "Unsupported pixel format %p4cc / modifier 0x%llx\n", > + &r->pixel_format, r->modifier[0]); > + return -EINVAL; > + } Like I said this is still wrong for the !modifiers case. > + > return 0; > } > > -- > 2.39.0
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 1f8a5ebe188e..3a79c26c5cc7 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -276,11 +276,10 @@ Various hold-ups: - Need to switch to drm_fbdev_generic_setup(), otherwise a lot of the custom fb setup code can't be deleted. -- Many drivers wrap drm_gem_fb_create() only to check for valid formats. For - atomic drivers we could check for valid formats by calling - drm_plane_check_pixel_format() against all planes, and pass if any plane - supports the format. For non-atomic that's not possible since like the format - list for the primary plane is fake and we'd therefor reject valid formats. +- Need to switch to drm_gem_fb_create(), as now framebuffer_check() checks for + valid formats for atomic drivers. + +- Add an addfb format validation for non-atomic drivers. - Many drivers subclass drm_framebuffer, we'd need a embedding compatible version of the varios drm_gem_fb_create functions. Maybe called diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c index aff3746dedfb..605642bf3650 100644 --- a/drivers/gpu/drm/drm_framebuffer.c +++ b/drivers/gpu/drm/drm_framebuffer.c @@ -280,6 +280,14 @@ static int framebuffer_check(struct drm_device *dev, } } + /* Verify that the modifier is supported. */ + if (drm_drv_uses_atomic_modeset(dev) && + !drm_any_plane_has_format(dev, r->pixel_format, r->modifier[0])) { + drm_dbg_kms(dev, "Unsupported pixel format %p4cc / modifier 0x%llx\n", + &r->pixel_format, r->modifier[0]); + return -EINVAL; + } + return 0; }