* [PATCH 1/4] drm/exynos: fixed a comment to gem size.
2012-06-29 8:02 [PATCH 0/4] drm/exynos: add exceptions to framebuffer module Inki Dae
@ 2012-06-29 8:02 ` Inki Dae
2012-06-29 8:02 ` [PATCH 2/4] drm/exynos: add packed_size not aligned in page unit Inki Dae
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Inki Dae @ 2012-06-29 8:02 UTC (permalink / raw)
To: airlied, dri-devel; +Cc: Inki Dae, kyungmin.park, sw0312.kim
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/gpu/drm/exynos/exynos_drm_gem.h | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
index 14d038b..085b2a5 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
@@ -63,7 +63,8 @@ struct exynos_drm_gem_buf {
* by user request or at framebuffer creation.
* continuous memory region allocated by user request
* or at framebuffer creation.
- * @size: total memory size to physically non-continuous memory region.
+ * @size: size requested from user, in bytes and this size is aligned
+ * in page unit.
* @flags: indicate memory type to allocated buffer and cache attruibute.
*
* P.S. this object would be transfered to user as kms_bo.handle so
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/4] drm/exynos: add packed_size not aligned in page unit.
2012-06-29 8:02 [PATCH 0/4] drm/exynos: add exceptions to framebuffer module Inki Dae
2012-06-29 8:02 ` [PATCH 1/4] drm/exynos: fixed a comment to gem size Inki Dae
@ 2012-06-29 8:02 ` Inki Dae
2012-06-29 8:02 ` [PATCH 3/4] drm/exynos: check if gem type is valid or not for fb Inki Dae
2012-06-29 8:02 ` [PATCH 4/4] drm/exynos: check if framebuffer and gem size are valid or not Inki Dae
3 siblings, 0 replies; 8+ messages in thread
From: Inki Dae @ 2012-06-29 8:02 UTC (permalink / raw)
To: airlied, dri-devel; +Cc: Inki Dae, kyungmin.park, sw0312.kim
this patch adds packed_size variable in exynos_drm_gem_obj struct
and this variable is used to check for valid framebuffer and gem 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_gem.c | 2 ++
drivers/gpu/drm/exynos/exynos_drm_gem.h | 3 +++
2 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 411d82b..94e8137 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -345,6 +345,7 @@ struct exynos_drm_gem_obj *exynos_drm_gem_create(struct drm_device *dev,
{
struct exynos_drm_gem_obj *exynos_gem_obj;
struct exynos_drm_gem_buf *buf;
+ unsigned long packed_size = size;
int ret;
if (!size) {
@@ -369,6 +370,7 @@ struct exynos_drm_gem_obj *exynos_drm_gem_create(struct drm_device *dev,
goto err_fini_buf;
}
+ exynos_gem_obj->packed_size = packed_size;
exynos_gem_obj->buffer = buf;
/* set memory type and cache attribute from user side. */
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h b/drivers/gpu/drm/exynos/exynos_drm_gem.h
index 085b2a5..1d80cb2 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
@@ -65,6 +65,8 @@ struct exynos_drm_gem_buf {
* or at framebuffer creation.
* @size: size requested from user, in bytes and this size is aligned
* in page unit.
+ * @packed_size: real size of the gem object, in bytes and
+ * this size isn't aligned in page unit.
* @flags: indicate memory type to allocated buffer and cache attruibute.
*
* P.S. this object would be transfered to user as kms_bo.handle so
@@ -74,6 +76,7 @@ struct exynos_drm_gem_obj {
struct drm_gem_object base;
struct exynos_drm_gem_buf *buffer;
unsigned long size;
+ unsigned long packed_size;
unsigned int flags;
};
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/4] drm/exynos: check if gem type is valid or not for fb.
2012-06-29 8:02 [PATCH 0/4] drm/exynos: add exceptions to framebuffer module Inki Dae
2012-06-29 8:02 ` [PATCH 1/4] drm/exynos: fixed a comment to gem size Inki Dae
2012-06-29 8:02 ` [PATCH 2/4] drm/exynos: add packed_size not aligned in page unit Inki Dae
@ 2012-06-29 8:02 ` Inki Dae
2012-06-29 8:02 ` [PATCH 4/4] drm/exynos: check if framebuffer and gem size are valid or not Inki Dae
3 siblings, 0 replies; 8+ messages in thread
From: Inki Dae @ 2012-06-29 8:02 UTC (permalink / raw)
To: airlied, dri-devel; +Cc: Inki Dae, kyungmin.park, sw0312.kim
physically non-contiguous memory can't be used for framebuffer yet.
so this patch checks if the gem memory type is valid or not for the
framebuffer.
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 | 38 ++++++++++++++++++++++++++++++++
1 files changed, 38 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
index 4ccfe43..6aba1e5 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
@@ -31,6 +31,7 @@
#include "drm_crtc_helper.h"
#include "drm_fb_helper.h"
+#include "exynos_drm.h"
#include "exynos_drm_drv.h"
#include "exynos_drm_fb.h"
#include "exynos_drm_gem.h"
@@ -48,6 +49,22 @@ struct exynos_drm_fb {
struct exynos_drm_gem_obj *exynos_gem_obj[MAX_FB_BUFFER];
};
+static int check_fb_gem_memory_type(struct drm_device *drm_dev,
+ struct exynos_drm_gem_obj *exynos_gem_obj)
+{
+ unsigned int flags;
+
+ flags = exynos_gem_obj->flags;
+
+ /* not support physically non-continuous memory for fb yet. TODO */
+ if (IS_NONCONTIG_BUFFER(flags)) {
+ DRM_ERROR("cannot use this gem memory type for fb.\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);
@@ -107,8 +124,17 @@ exynos_drm_framebuffer_init(struct drm_device *dev,
struct drm_gem_object *obj)
{
struct exynos_drm_fb *exynos_fb;
+ struct exynos_drm_gem_obj *exynos_gem_obj;
int ret;
+ exynos_gem_obj = to_exynos_gem_obj(obj);
+
+ ret = check_fb_gem_memory_type(dev, exynos_gem_obj);
+ if (ret < 0) {
+ DRM_ERROR("cannot use this gem memory type for fb.\n");
+ return ERR_PTR(-EINVAL);
+ }
+
exynos_fb = kzalloc(sizeof(*exynos_fb), GFP_KERNEL);
if (!exynos_fb) {
DRM_ERROR("failed to allocate exynos drm framebuffer\n");
@@ -155,6 +181,9 @@ exynos_user_fb_create(struct drm_device *dev, struct drm_file *file_priv,
nr = exynos_drm_format_num_buffers(fb->pixel_format);
for (i = 1; i < nr; i++) {
+ struct exynos_drm_gem_obj *exynos_gem_obj;
+ int ret;
+
obj = drm_gem_object_lookup(dev, file_priv,
mode_cmd->handles[i]);
if (!obj) {
@@ -163,6 +192,15 @@ exynos_user_fb_create(struct drm_device *dev, struct drm_file *file_priv,
return ERR_PTR(-ENOENT);
}
+ exynos_gem_obj = to_exynos_gem_obj(obj);
+
+ ret = check_fb_gem_memory_type(dev, exynos_gem_obj);
+ if (ret < 0) {
+ DRM_ERROR("cannot use this gem memory type for fb.\n");
+ exynos_drm_fb_destroy(fb);
+ return ERR_PTR(ret);
+ }
+
exynos_fb->exynos_gem_obj[i] = to_exynos_gem_obj(obj);
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 4/4] drm/exynos: check if framebuffer and gem size are valid or not.
2012-06-29 8:02 [PATCH 0/4] drm/exynos: add exceptions to framebuffer module Inki Dae
` (2 preceding siblings ...)
2012-06-29 8:02 ` [PATCH 3/4] drm/exynos: check if gem type is valid or not for fb Inki Dae
@ 2012-06-29 8:02 ` Inki Dae
2012-07-09 5:23 ` [PATCH v2] " Inki Dae
3 siblings, 1 reply; 8+ messages in thread
From: Inki Dae @ 2012-06-29 8:02 UTC (permalink / raw)
To: airlied, dri-devel; +Cc: Inki Dae, kyungmin.park, sw0312.kim
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.
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 | 48 ++++++++++++++++++++++++++++++-
1 files changed, 46 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
index 6aba1e5..ce0f18d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
@@ -65,6 +65,45 @@ static int check_fb_gem_memory_type(struct drm_device *drm_dev,
return 0;
}
+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->width * ((fb->bits_per_pixel + 7) >> 3)
+ * 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;
+
+ 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);
@@ -160,8 +199,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__);
@@ -204,6 +242,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);
+ }
+
return fb;
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v2] drm/exynos: check if framebuffer and gem size are valid or not.
2012-06-29 8:02 ` [PATCH 4/4] drm/exynos: check if framebuffer and gem size are valid or not Inki Dae
@ 2012-07-09 5:23 ` Inki Dae
2012-07-19 13:49 ` Laurent Pinchart
0 siblings, 1 reply; 8+ messages in thread
From: Inki Dae @ 2012-07-09 5:23 UTC (permalink / raw)
To: airlied, dri-devel; +Cc: Inki Dae, kyungmin.park, sw0312.kim
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;
+
+ 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);
+ }
+
return fb;
}
--
1.7.4.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] drm/exynos: check if framebuffer and gem size are valid or not.
2012-07-09 5:23 ` [PATCH v2] " Inki Dae
@ 2012-07-19 13:49 ` Laurent Pinchart
2012-07-20 7:28 ` InKi Dae
0 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2012-07-19 13:49 UTC (permalink / raw)
To: dri-devel; +Cc: Inki Dae, kyungmin.park, sw0312.kim
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;
> }
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] drm/exynos: check if framebuffer and gem size are valid or not.
2012-07-19 13:49 ` Laurent Pinchart
@ 2012-07-20 7:28 ` InKi Dae
0 siblings, 0 replies; 8+ messages in thread
From: InKi Dae @ 2012-07-20 7:28 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: Inki Dae, kyungmin.park, sw0312.kim, dri-devel
Hi Laurent,
2012/7/19, Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
> 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.
>
Right, it's my missing point. this part will be fixed for merge window.
>> +
>> + 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 ?
>
I agree with you.
Thanks,
Inki Dae
>> return fb;
>> }
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
^ permalink raw reply [flat|nested] 8+ messages in thread