* [PATCH] drm/amdgpu: Call drm_framebuffer_init last for framebuffer init
@ 2021-06-16 10:46 Michel Dänzer
2021-06-17 10:33 ` Michel Dänzer
0 siblings, 1 reply; 3+ messages in thread
From: Michel Dänzer @ 2021-06-16 10:46 UTC (permalink / raw
To: amd-gfx; +Cc: Mark Yacoub
From: Michel Dänzer <mdaenzer@redhat.com>
Once drm_framebuffer_init has returned 0, the framebuffer is hooked up
to the reference counting machinery and can no longer be destroyed with
a simple kfree. Therefore, it must be called last.
Fixes: f258907fdd835e "drm/amdgpu: Verify bo size can fit framebuffer size on init."
Signed-off-by: Michel Dänzer <mdaenzer@redhat.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index c13985fb35be..2a4cd7d377bf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -1047,11 +1047,12 @@ int amdgpu_display_gem_fb_init(struct drm_device *dev,
rfb->base.obj[0] = obj;
drm_helper_mode_fill_fb_struct(dev, &rfb->base, mode_cmd);
- ret = drm_framebuffer_init(dev, &rfb->base, &amdgpu_fb_funcs);
+
+ ret = amdgpu_display_framebuffer_init(dev, rfb, mode_cmd, obj);
if (ret)
goto err;
- ret = amdgpu_display_framebuffer_init(dev, rfb, mode_cmd, obj);
+ ret = drm_framebuffer_init(dev, &rfb->base, &amdgpu_fb_funcs);
if (ret)
goto err;
@@ -1071,9 +1072,6 @@ int amdgpu_display_gem_fb_verify_and_init(
rfb->base.obj[0] = obj;
drm_helper_mode_fill_fb_struct(dev, &rfb->base, mode_cmd);
- ret = drm_framebuffer_init(dev, &rfb->base, &amdgpu_fb_funcs);
- if (ret)
- goto err;
/* Verify that the modifier is supported. */
if (!drm_any_plane_has_format(dev, mode_cmd->pixel_format,
mode_cmd->modifier[0])) {
@@ -1092,6 +1090,10 @@ int amdgpu_display_gem_fb_verify_and_init(
if (ret)
goto err;
+ ret = drm_framebuffer_init(dev, &rfb->base, &amdgpu_fb_funcs);
+ if (ret)
+ goto err;
+
return 0;
err:
drm_dbg_kms(dev, "Failed to verify and init gem fb: %d\n", ret);
--
2.32.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/amdgpu: Call drm_framebuffer_init last for framebuffer init
2021-06-16 10:46 [PATCH] drm/amdgpu: Call drm_framebuffer_init last for framebuffer init Michel Dänzer
@ 2021-06-17 10:33 ` Michel Dänzer
2021-06-18 20:51 ` Alex Deucher
0 siblings, 1 reply; 3+ messages in thread
From: Michel Dänzer @ 2021-06-17 10:33 UTC (permalink / raw
To: amd-gfx; +Cc: Mark Yacoub
On 2021-06-16 12:46 p.m., Michel Dänzer wrote:
> From: Michel Dänzer <mdaenzer@redhat.com>
>
> Once drm_framebuffer_init has returned 0, the framebuffer is hooked up
> to the reference counting machinery and can no longer be destroyed with
> a simple kfree. Therefore, it must be called last.
>
> Fixes: f258907fdd835e "drm/amdgpu: Verify bo size can fit framebuffer size on init."
In case the commit log wasn't clear: If drm_framebuffer_init returns 0 but its caller then returns non-0, there will likely be memory corruption fireworks down the road. The following lead me to this fix:
[ 12.891228] kernel BUG at lib/list_debug.c:25!
[...]
[ 12.891263] RIP: 0010:__list_add_valid+0x4b/0x70
[...]
[ 12.891324] Call Trace:
[ 12.891330] drm_framebuffer_init+0xb5/0x100 [drm]
[ 12.891378] amdgpu_display_gem_fb_verify_and_init+0x47/0x120 [amdgpu]
[ 12.891592] ? amdgpu_display_user_framebuffer_create+0x10d/0x1f0 [amdgpu]
[ 12.891794] amdgpu_display_user_framebuffer_create+0x126/0x1f0 [amdgpu]
[ 12.891995] drm_internal_framebuffer_create+0x378/0x3f0 [drm]
[ 12.892036] ? drm_internal_framebuffer_create+0x3f0/0x3f0 [drm]
[ 12.892075] drm_mode_addfb2+0x34/0xd0 [drm]
[ 12.892115] ? drm_internal_framebuffer_create+0x3f0/0x3f0 [drm]
[ 12.892153] drm_ioctl_kernel+0xe2/0x150 [drm]
[ 12.892193] drm_ioctl+0x3da/0x460 [drm]
[ 12.892232] ? drm_internal_framebuffer_create+0x3f0/0x3f0 [drm]
[ 12.892274] amdgpu_drm_ioctl+0x43/0x80 [amdgpu]
[ 12.892475] __se_sys_ioctl+0x72/0xc0
[ 12.892483] do_syscall_64+0x33/0x40
[ 12.892491] entry_SYSCALL_64_after_hwframe+0x44/0xae
--
Earthling Michel Dänzer | https://redhat.com
Libre software enthusiast | Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/amdgpu: Call drm_framebuffer_init last for framebuffer init
2021-06-17 10:33 ` Michel Dänzer
@ 2021-06-18 20:51 ` Alex Deucher
0 siblings, 0 replies; 3+ messages in thread
From: Alex Deucher @ 2021-06-18 20:51 UTC (permalink / raw
To: Michel Dänzer; +Cc: amd-gfx list, Mark Yacoub
Applied. Thanks!
Alex
On Thu, Jun 17, 2021 at 6:33 AM Michel Dänzer <michel@daenzer.net> wrote:
>
> On 2021-06-16 12:46 p.m., Michel Dänzer wrote:
> > From: Michel Dänzer <mdaenzer@redhat.com>
> >
> > Once drm_framebuffer_init has returned 0, the framebuffer is hooked up
> > to the reference counting machinery and can no longer be destroyed with
> > a simple kfree. Therefore, it must be called last.
> >
> > Fixes: f258907fdd835e "drm/amdgpu: Verify bo size can fit framebuffer size on init."
>
> In case the commit log wasn't clear: If drm_framebuffer_init returns 0 but its caller then returns non-0, there will likely be memory corruption fireworks down the road. The following lead me to this fix:
>
> [ 12.891228] kernel BUG at lib/list_debug.c:25!
> [...]
> [ 12.891263] RIP: 0010:__list_add_valid+0x4b/0x70
> [...]
> [ 12.891324] Call Trace:
> [ 12.891330] drm_framebuffer_init+0xb5/0x100 [drm]
> [ 12.891378] amdgpu_display_gem_fb_verify_and_init+0x47/0x120 [amdgpu]
> [ 12.891592] ? amdgpu_display_user_framebuffer_create+0x10d/0x1f0 [amdgpu]
> [ 12.891794] amdgpu_display_user_framebuffer_create+0x126/0x1f0 [amdgpu]
> [ 12.891995] drm_internal_framebuffer_create+0x378/0x3f0 [drm]
> [ 12.892036] ? drm_internal_framebuffer_create+0x3f0/0x3f0 [drm]
> [ 12.892075] drm_mode_addfb2+0x34/0xd0 [drm]
> [ 12.892115] ? drm_internal_framebuffer_create+0x3f0/0x3f0 [drm]
> [ 12.892153] drm_ioctl_kernel+0xe2/0x150 [drm]
> [ 12.892193] drm_ioctl+0x3da/0x460 [drm]
> [ 12.892232] ? drm_internal_framebuffer_create+0x3f0/0x3f0 [drm]
> [ 12.892274] amdgpu_drm_ioctl+0x43/0x80 [amdgpu]
> [ 12.892475] __se_sys_ioctl+0x72/0xc0
> [ 12.892483] do_syscall_64+0x33/0x40
> [ 12.892491] entry_SYSCALL_64_after_hwframe+0x44/0xae
>
>
>
> --
> Earthling Michel Dänzer | https://redhat.com
> Libre software enthusiast | Mesa and X developer
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-06-18 20:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-06-16 10:46 [PATCH] drm/amdgpu: Call drm_framebuffer_init last for framebuffer init Michel Dänzer
2021-06-17 10:33 ` Michel Dänzer
2021-06-18 20:51 ` Alex Deucher
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.