Re: [PATCH v2] drm/exynos: check if framebuffer and gem size are valid or not.

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.dae@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@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; }