* [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 2017-03-20 0:52 ` [PATCH] tests/exynos: fix invalid code " Seung-Woo Kim 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 2017-03-20 0:52 ` [PATCH] tests/exynos: fix invalid code " Seung-Woo Kim 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 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:52 ` Seung-Woo Kim 2017-04-03 16:28 ` Emil Velikov 1 sibling, 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
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.