| Message ID | 20191024104801.3122-1-colin.king@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [next] drm/v3d: fix double free of bin | expand |
On Thu, Oct 24, 2019 at 11:48:01AM +0100, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Two different fixes have addressed the same memory leak of bin and > this now causes a double free of bin. While the individual memory > leak fixes are fine, both fixes together are problematic. > > Addresses-Coverity: ("Double free") > Fixes: 29cd13cfd762 ("drm/v3d: Fix memory leak in v3d_submit_cl_ioctl") > Fixes: 0d352a3a8a1f (" rm/v3d: don't leak bin job if v3d_job_init fails.") > Signed-off-by: Colin Ian King <colin.king@canonical.com> That sounds like wrong merge resolution somewhere, and we don't have those patches merged together in any final tree yet anywhere. What's this based on? -Daniel > --- > drivers/gpu/drm/v3d/v3d_gem.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c > index 549dde83408b..37515e47b47e 100644 > --- a/drivers/gpu/drm/v3d/v3d_gem.c > +++ b/drivers/gpu/drm/v3d/v3d_gem.c > @@ -568,7 +568,6 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, > ret = v3d_job_init(v3d, file_priv, &bin->base, > v3d_job_free, args->in_sync_bcl); > if (ret) { > - kfree(bin); > v3d_job_put(&render->base); > kfree(bin); > return ret; > -- > 2.20.1 >
On 24/10/2019 13:38, Daniel Vetter wrote: > On Thu, Oct 24, 2019 at 11:48:01AM +0100, Colin King wrote: >> From: Colin Ian King <colin.king@canonical.com> >> >> Two different fixes have addressed the same memory leak of bin and >> this now causes a double free of bin. While the individual memory >> leak fixes are fine, both fixes together are problematic. >> >> Addresses-Coverity: ("Double free") >> Fixes: 29cd13cfd762 ("drm/v3d: Fix memory leak in v3d_submit_cl_ioctl") >> Fixes: 0d352a3a8a1f (" rm/v3d: don't leak bin job if v3d_job_init fails.") >> Signed-off-by: Colin Ian King <colin.king@canonical.com> > > That sounds like wrong merge resolution somewhere, and we don't have those > patches merged together in any final tree yet anywhere. What's this based > on? > -Daniel linux-next Colin > >> --- >> drivers/gpu/drm/v3d/v3d_gem.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c >> index 549dde83408b..37515e47b47e 100644 >> --- a/drivers/gpu/drm/v3d/v3d_gem.c >> +++ b/drivers/gpu/drm/v3d/v3d_gem.c >> @@ -568,7 +568,6 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, >> ret = v3d_job_init(v3d, file_priv, &bin->base, >> v3d_job_free, args->in_sync_bcl); >> if (ret) { >> - kfree(bin); >> v3d_job_put(&render->base); >> kfree(bin); >> return ret; >> -- >> 2.20.1 >> >
On Thu, Oct 24, 2019 at 2:43 PM Colin Ian King <colin.king@canonical.com> wrote: > > On 24/10/2019 13:38, Daniel Vetter wrote: > > On Thu, Oct 24, 2019 at 11:48:01AM +0100, Colin King wrote: > >> From: Colin Ian King <colin.king@canonical.com> > >> > >> Two different fixes have addressed the same memory leak of bin and > >> this now causes a double free of bin. While the individual memory > >> leak fixes are fine, both fixes together are problematic. > >> > >> Addresses-Coverity: ("Double free") > >> Fixes: 29cd13cfd762 ("drm/v3d: Fix memory leak in v3d_submit_cl_ioctl") > >> Fixes: 0d352a3a8a1f (" rm/v3d: don't leak bin job if v3d_job_init fails.") > >> Signed-off-by: Colin Ian King <colin.king@canonical.com> > > > > That sounds like wrong merge resolution somewhere, and we don't have those > > patches merged together in any final tree yet anywhere. What's this based > > on? > > -Daniel > > linux-next Ok adding Stephen. There's a merge conflict between drm-misc-fixes and drm-next (I think) and the merge double-added the kfree(bin). See above for the relevant sha1. Dave is already on here as a heads-up, but also adding drm-misc maintainers. Cheers, Daniel > > Colin > > > >> --- > >> drivers/gpu/drm/v3d/v3d_gem.c | 1 - > >> 1 file changed, 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c > >> index 549dde83408b..37515e47b47e 100644 > >> --- a/drivers/gpu/drm/v3d/v3d_gem.c > >> +++ b/drivers/gpu/drm/v3d/v3d_gem.c > >> @@ -568,7 +568,6 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, > >> ret = v3d_job_init(v3d, file_priv, &bin->base, > >> v3d_job_free, args->in_sync_bcl); > >> if (ret) { > >> - kfree(bin); > >> v3d_job_put(&render->base); > >> kfree(bin); > >> return ret; > >> -- > >> 2.20.1 > >> > > > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi all, On Thu, 24 Oct 2019 14:49:36 +0200 Daniel Vetter <daniel@ffwll.ch> wrote: > > Ok adding Stephen. There's a merge conflict between drm-misc-fixes and > drm-next (I think) and the merge double-added the kfree(bin). See > above for the relevant sha1. Dave is already on here as a heads-up, > but also adding drm-misc maintainers. > > > >> --- > > >> drivers/gpu/drm/v3d/v3d_gem.c | 1 - > > >> 1 file changed, 1 deletion(-) > > >> > > >> diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c > > >> index 549dde83408b..37515e47b47e 100644 > > >> --- a/drivers/gpu/drm/v3d/v3d_gem.c > > >> +++ b/drivers/gpu/drm/v3d/v3d_gem.c > > >> @@ -568,7 +568,6 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, > > >> ret = v3d_job_init(v3d, file_priv, &bin->base, > > >> v3d_job_free, args->in_sync_bcl); > > >> if (ret) { > > >> - kfree(bin); > > >> v3d_job_put(&render->base); > > >> kfree(bin); > > >> return ret; I will add this as a merge fixup until drm-misc-fixes is merged into the drm tree.
On Thu, Oct 24, 2019 at 02:38:53PM +0200, Daniel Vetter wrote: > On Thu, Oct 24, 2019 at 11:48:01AM +0100, Colin King wrote: > > From: Colin Ian King <colin.king@canonical.com> > > > > Two different fixes have addressed the same memory leak of bin and > > this now causes a double free of bin. While the individual memory > > leak fixes are fine, both fixes together are problematic. > > > > Addresses-Coverity: ("Double free") > > Fixes: 29cd13cfd762 ("drm/v3d: Fix memory leak in v3d_submit_cl_ioctl") > > Fixes: 0d352a3a8a1f (" rm/v3d: don't leak bin job if v3d_job_init fails.") > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > > That sounds like wrong merge resolution somewhere, and we don't have those > patches merged together in any final tree yet anywhere. What's this based > on? > -Daniel linux-next. I sent this fix to you and Stephen Rothwell yesterday so this one is sorted already. Stephen will apply my patch until you guys merge your drm trees. regards, dan carpenter
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c index 549dde83408b..37515e47b47e 100644 --- a/drivers/gpu/drm/v3d/v3d_gem.c +++ b/drivers/gpu/drm/v3d/v3d_gem.c @@ -568,7 +568,6 @@ v3d_submit_cl_ioctl(struct drm_device *dev, void *data, ret = v3d_job_init(v3d, file_priv, &bin->base, v3d_job_free, args->in_sync_bcl); if (ret) { - kfree(bin); v3d_job_put(&render->base); kfree(bin); return ret;