Hi Inki,
On Monday 09 July 2012 14:23:23 Inki Dae wrote:
> with addfb request by user, wrong framebuffer or gem size could be sent
> to kernel side so this could induce invalid memory access by dma of a
> device. this patch checks if framebuffer and gem size are valid or not to
> avoid this issue.
>
> Changelog v2:
> use fb->pitches instead of caculating it with fb->width and fb->bpp
> as line size.
>
> Signed-off-by: Inki Dae <inki....@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.p...@samsung.com>
> ---
> drivers/gpu/drm/exynos/exynos_drm_fb.c | 47 ++++++++++++++++++++++++++++-
> 1 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c
> b/drivers/gpu/drm/exynos/exynos_drm_fb.c index 4ccfe43..f1b1008 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> @@ -48,6 +48,44 @@ struct exynos_drm_fb {
> struct exynos_drm_gem_obj *exynos_gem_obj[MAX_FB_BUFFER];
> };
>
> +static int check_fb_gem_size(struct drm_device *drm_dev,
> + struct drm_framebuffer *fb,
> + unsigned int nr)
> +{
> + unsigned long fb_size;
> + struct drm_gem_object *obj;
> + struct exynos_drm_gem_obj *exynos_gem_obj;
> + struct exynos_drm_fb *exynos_fb = to_exynos_fb(fb);
> +
> + /* in case of RGB format, only one plane is used. */
> + if (nr < 2) {
> + exynos_gem_obj = exynos_fb->exynos_gem_obj[0];
> + obj = &exynos_gem_obj->base;
> + fb_size = fb->pitches[0] * fb->height;
> +
> + if (fb_size != exynos_gem_obj->packed_size) {
> + DRM_ERROR("invalid fb or gem size.\n");
> + return -EINVAL;
> + }
> + /* in case of NV12MT, YUV420M and so on, two and three planes. */
> + } else {
> + unsigned int i;
> +
> + for (i = 0; i < nr; i++) {
> + exynos_gem_obj = exynos_fb->exynos_gem_obj[i];
> + obj = &exynos_gem_obj->base;
> + fb_size = fb->pitches[i] * fb->height;
I think you need to take vertical chroma subsampling into account here, as
well as fb->offsets[i]. See https://patchwork.kernel.org/patch/1147061/ for an
ongoing discussion on this subject.
> +
> + if (fb_size != exynos_gem_obj->packed_size) {
> + DRM_ERROR("invalid fb or gem size.\n");
> + return -EINVAL;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> static void exynos_drm_fb_destroy(struct drm_framebuffer *fb)
> {
> struct exynos_drm_fb *exynos_fb = to_exynos_fb(fb);
> @@ -134,8 +172,7 @@ exynos_user_fb_create(struct drm_device *dev, struct
> drm_file *file_priv, struct drm_gem_object *obj;
> struct drm_framebuffer *fb;
> struct exynos_drm_fb *exynos_fb;
> - int nr;
> - int i;
> + int nr, i, ret;
>
> DRM_DEBUG_KMS("%s\n", __FILE__);
>
> @@ -166,6 +203,12 @@ exynos_user_fb_create(struct drm_device *dev, struct
> drm_file *file_priv, exynos_fb->exynos_gem_obj[i] = to_exynos_gem_obj(obj);
> }
>
> + ret = check_fb_gem_size(dev, fb, nr);
> + if (ret < 0) {
> + exynos_drm_fb_destroy(fb);
> + return ERR_PTR(ret);
> + }
> +
What about checking the size before creating the frame buffer ?
> return fb;
> }
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
