On Thu, Jun 22, 2017 at 08:06:24AM +0200, Peter Rosin wrote:
> I think the gamma_store can end up invalid on error. But the way I read
> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should
> this pesky legacy fbdev stuff be any better?
> 
> Signed-off-by: Peter Rosin <p...@axentia.se>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 574af01..25315fb 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1223,12 +1223,16 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, 
> struct fb_info *info)
>       const struct drm_crtc_helper_funcs *crtc_funcs;
>       u16 *red, *green, *blue, *transp;
>       struct drm_crtc *crtc;
> +     u16 *r, *g, *b;
>       int i, j, rc = 0;
>       int start;
>  
>       if (oops_in_progress)
>               return -EBUSY;
>  
> +     if (cmap->start + cmap->len < cmap->start)
> +             return -EINVAL;

Doesn't the fbdev core check this for us?

> +
>       drm_modeset_lock_all(dev);
>       if (!drm_fb_helper_is_bound(fb_helper)) {
>               drm_modeset_unlock_all(dev);
> @@ -1245,6 +1249,29 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct 
> fb_info *info)
>               transp = cmap->transp;
>               start = cmap->start;
>  
> +             if (info->fix.visual != FB_VISUAL_TRUECOLOR) {

I think switching to a positive check of visual == PSEUDOCOLOR is a bit
clearer. Also this makes more sense once we move over to the main
gamma_set function, since as-is it's unclear that setcolreg only does
something for PSEUDOCOLOR. But interim I guess that's ok.
-Daniel

> +                     if (!crtc->gamma_size) {
> +                             rc = -EINVAL;
> +                             goto out;
> +                     }
> +
> +                     if (cmap->start + cmap->len > crtc->gamma_size) {
> +                             rc = -EINVAL;
> +                             goto out;
> +                     }
> +
> +                     r = crtc->gamma_store;
> +                     g = r + crtc->gamma_size;
> +                     b = g + crtc->gamma_size;
> +
> +                     memcpy(r + cmap->start, cmap->red,
> +                            cmap->len * sizeof(u16));
> +                     memcpy(g + cmap->start, cmap->green,
> +                            cmap->len * sizeof(u16));
> +                     memcpy(b + cmap->start, cmap->blue,
> +                            cmap->len * sizeof(u16));
> +             }
> +
>               for (j = 0; j < cmap->len; j++) {
>                       u16 hred, hgreen, hblue, htransp = 0xffff;
>  
> -- 
> 2.1.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to