* [PATCH libdrm] tests/exynos: fix memory issues of error path in g2d test
[not found] <CGME20170316015952epcas1p20a285e6029d1b94cb32b1fca2aeb1d18@epcas1p2.samsung.com>
@ 2017-03-16 2:00 ` Seung-Woo Kim
2017-03-20 0:08 ` Emil Velikov
[not found] ` <CGME20170320005235epcas5p3a5671536d1bc4a01be83567d060d6653@epcas5p3.samsung.com>
0 siblings, 2 replies; 5+ messages in thread
From: Seung-Woo Kim @ 2017-03-16 2:00 UTC (permalink / raw)
To: dri-devel, robclark, inki.dae; +Cc: sw0312.kim, emil.l.velikov
This patch fixes memory issues including NULL deference and leak
in g2d test in error path.
Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
---
tests/exynos/exynos_fimg2d_test.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/tests/exynos/exynos_fimg2d_test.c b/tests/exynos/exynos_fimg2d_test.c
index 797fb6e..2177e08 100644
--- a/tests/exynos/exynos_fimg2d_test.c
+++ b/tests/exynos/exynos_fimg2d_test.c
@@ -59,7 +59,6 @@ static void connector_find_mode(int fd, struct connector *c,
if (!connector) {
fprintf(stderr, "could not get connector %i: %s\n",
resources->connectors[i], strerror(errno));
- drmModeFreeConnector(connector);
continue;
}
@@ -98,7 +97,6 @@ static void connector_find_mode(int fd, struct connector *c,
if (!c->encoder) {
fprintf(stderr, "could not get encoder %i: %s\n",
resources->encoders[i], strerror(errno));
- drmModeFreeEncoder(c->encoder);
continue;
}
@@ -264,7 +262,8 @@ static int g2d_copy_test(struct exynos_device *dev, struct exynos_bo *src,
userptr = (unsigned long)malloc(size);
if (!userptr) {
fprintf(stderr, "failed to allocate userptr.\n");
- return -EFAULT;
+ ret = -EFAULT;
+ goto fail;
}
src_img.user_ptr[0].userptr = userptr;
@@ -469,7 +468,8 @@ static int g2d_copy_with_scale_test(struct exynos_device *dev,
userptr = (unsigned long)malloc(size);
if (!userptr) {
fprintf(stderr, "failed to allocate userptr.\n");
- return -EFAULT;
+ ret = -EFAULT;
+ goto fail;
}
src_img.user_ptr[0].userptr = userptr;
@@ -557,7 +557,8 @@ static int g2d_blend_test(struct exynos_device *dev,
userptr = (unsigned long)malloc(size);
if (!userptr) {
fprintf(stderr, "failed to allocate userptr.\n");
- return -EFAULT;
+ ret = -EFAULT;
+ goto fail;
}
src_img.user_ptr[0].userptr = userptr;
@@ -755,7 +756,7 @@ int main(int argc, char **argv)
dev = exynos_device_create(fd);
if (!dev) {
- drmClose(dev->fd);
+ drmClose(fd);
return -EFAULT;
}
--
1.7.4.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH libdrm] tests/exynos: fix memory issues of error path in g2d test
2017-03-16 2:00 ` [PATCH libdrm] tests/exynos: fix memory issues of error path in g2d test Seung-Woo Kim
@ 2017-03-20 0:08 ` Emil Velikov
2017-03-20 0:14 ` Seung-Woo Kim
[not found] ` <CGME20170320005235epcas5p3a5671536d1bc4a01be83567d060d6653@epcas5p3.samsung.com>
1 sibling, 1 reply; 5+ messages in thread
From: Emil Velikov @ 2017-03-20 0:08 UTC (permalink / raw)
To: Seung-Woo Kim; +Cc: Rob Clark, ML dri-devel
Hi Seung-Woo Kim,
On 16 March 2017 at 02:00, Seung-Woo Kim <sw0312.kim@samsung.com> wrote:
> This patch fixes memory issues including NULL deference and leak
> in g2d test in error path.
>
> Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
> ---
> tests/exynos/exynos_fimg2d_test.c | 13 +++++++------
> 1 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/tests/exynos/exynos_fimg2d_test.c b/tests/exynos/exynos_fimg2d_test.c
> index 797fb6e..2177e08 100644
> --- a/tests/exynos/exynos_fimg2d_test.c
> +++ b/tests/exynos/exynos_fimg2d_test.c
> @@ -59,7 +59,6 @@ static void connector_find_mode(int fd, struct connector *c,
> if (!connector) {
> fprintf(stderr, "could not get connector %i: %s\n",
> resources->connectors[i], strerror(errno));
> - drmModeFreeConnector(connector);
> continue;
> }
>
> @@ -98,7 +97,6 @@ static void connector_find_mode(int fd, struct connector *c,
> if (!c->encoder) {
> fprintf(stderr, "could not get encoder %i: %s\n",
> resources->encoders[i], strerror(errno));
> - drmModeFreeEncoder(c->encoder);
> continue;
> }
>
> @@ -264,7 +262,8 @@ static int g2d_copy_test(struct exynos_device *dev, struct exynos_bo *src,
> userptr = (unsigned long)malloc(size);
> if (!userptr) {
> fprintf(stderr, "failed to allocate userptr.\n");
> - return -EFAULT;
> + ret = -EFAULT;
> + goto fail;
Even if you have the best of intentions, but there's yet another bug
in the existing code :-\
Namely: we always return 0 even on error - i.e. the "return 0", after
the g2d_fini() must be "return ret"
This applies to all the tests, afaics.
> @@ -755,7 +756,7 @@ int main(int argc, char **argv)
>
> dev = exynos_device_create(fd);
> if (!dev) {
> - drmClose(dev->fd);
> + drmClose(fd);
Seems correct, but an alternative (better IMHO) solution is to:
- flip/fix the call drmClose() <> exynos_device_destroy() order in
err_drm_close.
- use "fd" in exynos_device_destroy's drmClose.
- add separate label and use it in the above case.
Can you give this a try, please ?
Thanks
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH libdrm] tests/exynos: fix memory issues of error path in g2d test
2017-03-20 0:08 ` Emil Velikov
@ 2017-03-20 0:14 ` Seung-Woo Kim
0 siblings, 0 replies; 5+ messages in thread
From: Seung-Woo Kim @ 2017-03-20 0:14 UTC (permalink / raw)
To: Emil Velikov; +Cc: Seung-Woo Kim, Rob Clark, ML dri-devel
Hello Emil,
Thanks for comment.
On 2017년 03월 20일 09:08, Emil Velikov wrote:
> Hi Seung-Woo Kim,
>
> On 16 March 2017 at 02:00, Seung-Woo Kim <sw0312.kim@samsung.com> wrote:
>> This patch fixes memory issues including NULL deference and leak
>> in g2d test in error path.
>>
>> Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
>> ---
>> tests/exynos/exynos_fimg2d_test.c | 13 +++++++------
>> 1 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/tests/exynos/exynos_fimg2d_test.c b/tests/exynos/exynos_fimg2d_test.c
>> index 797fb6e..2177e08 100644
>> --- a/tests/exynos/exynos_fimg2d_test.c
>> +++ b/tests/exynos/exynos_fimg2d_test.c
>> @@ -59,7 +59,6 @@ static void connector_find_mode(int fd, struct connector *c,
>> if (!connector) {
>> fprintf(stderr, "could not get connector %i: %s\n",
>> resources->connectors[i], strerror(errno));
>> - drmModeFreeConnector(connector);
>> continue;
>> }
>>
>> @@ -98,7 +97,6 @@ static void connector_find_mode(int fd, struct connector *c,
>> if (!c->encoder) {
>> fprintf(stderr, "could not get encoder %i: %s\n",
>> resources->encoders[i], strerror(errno));
>> - drmModeFreeEncoder(c->encoder);
>> continue;
>> }
>>
>> @@ -264,7 +262,8 @@ static int g2d_copy_test(struct exynos_device *dev, struct exynos_bo *src,
>> userptr = (unsigned long)malloc(size);
>> if (!userptr) {
>> fprintf(stderr, "failed to allocate userptr.\n");
>> - return -EFAULT;
>> + ret = -EFAULT;
>> + goto fail;
> Even if you have the best of intentions, but there's yet another bug
> in the existing code :-\
>
> Namely: we always return 0 even on error - i.e. the "return 0", after
> the g2d_fini() must be "return ret"
> This applies to all the tests, afaics.
You are right. I will fix it.
>
>
>> @@ -755,7 +756,7 @@ int main(int argc, char **argv)
>>
>> dev = exynos_device_create(fd);
>> if (!dev) {
>> - drmClose(dev->fd);
>> + drmClose(fd);
> Seems correct, but an alternative (better IMHO) solution is to:
> - flip/fix the call drmClose() <> exynos_device_destroy() order in
> err_drm_close.
> - use "fd" in exynos_device_destroy's drmClose.
> - add separate label and use it in the above case.
Ok, I will add error label.
>
> Can you give this a try, please ?
After fixing as your comment, I will send v2.
Regards,
- Seung-Woo Kim
>
> Thanks
> Emil
>
>
>
--
Seung-Woo Kim
Samsung Software R&D Center
--
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] tests/exynos: fix invalid code of error path in g2d test
[not found] ` <CGME20170320005235epcas5p3a5671536d1bc4a01be83567d060d6653@epcas5p3.samsung.com>
@ 2017-03-20 0:52 ` Seung-Woo Kim
2017-04-03 16:28 ` Emil Velikov
0 siblings, 1 reply; 5+ messages in thread
From: Seung-Woo Kim @ 2017-03-20 0:52 UTC (permalink / raw)
To: dri-devel, robclark, inki.dae; +Cc: sw0312.kim, emil.l.velikov
This patch fixes invalid code of error path including NULL
deference and leak in g2d test.
Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
---
tests/exynos/exynos_fimg2d_test.c | 39 +++++++++++++++++++-----------------
1 files changed, 21 insertions(+), 18 deletions(-)
diff --git a/tests/exynos/exynos_fimg2d_test.c b/tests/exynos/exynos_fimg2d_test.c
index 797fb6e..e4e960c 100644
--- a/tests/exynos/exynos_fimg2d_test.c
+++ b/tests/exynos/exynos_fimg2d_test.c
@@ -59,7 +59,6 @@ static void connector_find_mode(int fd, struct connector *c,
if (!connector) {
fprintf(stderr, "could not get connector %i: %s\n",
resources->connectors[i], strerror(errno));
- drmModeFreeConnector(connector);
continue;
}
@@ -98,7 +97,6 @@ static void connector_find_mode(int fd, struct connector *c,
if (!c->encoder) {
fprintf(stderr, "could not get encoder %i: %s\n",
resources->encoders[i], strerror(errno));
- drmModeFreeEncoder(c->encoder);
continue;
}
@@ -264,7 +262,8 @@ static int g2d_copy_test(struct exynos_device *dev, struct exynos_bo *src,
userptr = (unsigned long)malloc(size);
if (!userptr) {
fprintf(stderr, "failed to allocate userptr.\n");
- return -EFAULT;
+ ret = -EFAULT;
+ goto fail;
}
src_img.user_ptr[0].userptr = userptr;
@@ -469,7 +468,8 @@ static int g2d_copy_with_scale_test(struct exynos_device *dev,
userptr = (unsigned long)malloc(size);
if (!userptr) {
fprintf(stderr, "failed to allocate userptr.\n");
- return -EFAULT;
+ ret = -EFAULT;
+ goto fail;
}
src_img.user_ptr[0].userptr = userptr;
@@ -520,7 +520,7 @@ err_free_userptr:
fail:
g2d_fini(ctx);
- return 0;
+ return ret;;
}
static int g2d_blend_test(struct exynos_device *dev,
@@ -557,7 +557,8 @@ static int g2d_blend_test(struct exynos_device *dev,
userptr = (unsigned long)malloc(size);
if (!userptr) {
fprintf(stderr, "failed to allocate userptr.\n");
- return -EFAULT;
+ ret = -EFAULT;
+ goto fail;
}
src_img.user_ptr[0].userptr = userptr;
@@ -619,7 +620,7 @@ err_free_userptr:
fail:
g2d_fini(ctx);
- return 0;
+ return ret;
}
static int g2d_checkerboard_test(struct exynos_device *dev,
@@ -645,8 +646,8 @@ static int g2d_checkerboard_test(struct exynos_device *dev,
dst_y = 0;
checkerboard = create_checkerboard_pattern(screen_width / 32, screen_height / 32, 32);
- if (checkerboard == NULL) {
- ret = -1;
+ if (!checkerboard) {
+ ret = -EFAULT;
goto fail;
}
@@ -755,8 +756,8 @@ int main(int argc, char **argv)
dev = exynos_device_create(fd);
if (!dev) {
- drmClose(dev->fd);
- return -EFAULT;
+ ret = -EFAULT;
+ goto err_drm_close;
}
resources = drmModeGetResources(dev->fd);
@@ -764,7 +765,7 @@ int main(int argc, char **argv)
fprintf(stderr, "drmModeGetResources failed: %s\n",
strerror(errno));
ret = -EFAULT;
- goto err_drm_close;
+ goto err_dev_destory;
}
connector_find_mode(dev->fd, &con, resources);
@@ -773,7 +774,7 @@ int main(int argc, char **argv)
if (!con.mode) {
fprintf(stderr, "failed to find usable connector\n");
ret = -EFAULT;
- goto err_drm_close;
+ goto err_dev_destory;
}
screen_width = con.mode->hdisplay;
@@ -782,7 +783,7 @@ int main(int argc, char **argv)
if (screen_width == 0 || screen_height == 0) {
fprintf(stderr, "failed to find sane resolution on connector\n");
ret = -EFAULT;
- goto err_drm_close;
+ goto err_dev_destory;
}
printf("screen width = %d, screen height = %d\n", screen_width,
@@ -791,7 +792,7 @@ int main(int argc, char **argv)
bo = exynos_create_buffer(dev, screen_width * screen_height * 4, 0);
if (!bo) {
ret = -EFAULT;
- goto err_drm_close;
+ goto err_dev_destory;
}
handles[0] = bo->handle;
@@ -882,9 +883,11 @@ err_rm_fb:
err_destroy_buffer:
exynos_destroy_buffer(bo);
-err_drm_close:
- drmClose(dev->fd);
+err_dev_destory:
exynos_device_destroy(dev);
- return 0;
+err_drm_close:
+ drmClose(fd);
+
+ return ret;
}
--
1.7.4.1
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] tests/exynos: fix invalid code of error path in g2d test
2017-03-20 0:52 ` [PATCH] tests/exynos: fix invalid code " Seung-Woo Kim
@ 2017-04-03 16:28 ` Emil Velikov
0 siblings, 0 replies; 5+ messages in thread
From: Emil Velikov @ 2017-04-03 16:28 UTC (permalink / raw)
To: Seung-Woo Kim; +Cc: Rob Clark, ML dri-devel
On 20 March 2017 at 00:52, Seung-Woo Kim <sw0312.kim@samsung.com> wrote:
> This patch fixes invalid code of error path including NULL
> deference and leak in g2d test.
>
Thanks for the update. R-b and pushed to master.
For future do add v2 in the subject prefix (git send-email -v2 ...).
It makes it easier to find the correct version of the patch.
-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-04-03 16:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20170316015952epcas1p20a285e6029d1b94cb32b1fca2aeb1d18@epcas1p2.samsung.com>
2017-03-16 2:00 ` [PATCH libdrm] tests/exynos: fix memory issues of error path in g2d test Seung-Woo Kim
2017-03-20 0:08 ` Emil Velikov
2017-03-20 0:14 ` Seung-Woo Kim
[not found] ` <CGME20170320005235epcas5p3a5671536d1bc4a01be83567d060d6653@epcas5p3.samsung.com>
2017-03-20 0:52 ` [PATCH] tests/exynos: fix invalid code " Seung-Woo Kim
2017-04-03 16:28 ` Emil Velikov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.