* [PATCH] drm/fb-helper: only unmap if buffer not null
@ 2021-02-28 4:46 Tong Zhang
2021-03-01 8:26 ` Thomas Zimmermann
0 siblings, 1 reply; 4+ messages in thread
From: Tong Zhang @ 2021-02-28 4:46 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Daniel Vetter, dri-devel, linux-kernel
Cc: ztong0001
drm_fbdev_cleanup() can be called when fb_helper->buffer is null, hence
fb_helper->buffer should be checked before calling
drm_client_buffer_vunmap(). This buffer is also checked in
drm_client_framebuffer_delete(), so we should also do the same thing for
drm_client_buffer_vunmap().
[ 199.128742] RIP: 0010:drm_client_buffer_vunmap+0xd/0x20
[ 199.129031] Code: 43 18 48 8b 53 20 49 89 45 00 49 89 55 08 5b 44 89 e0 41 5c 41 5d 41 5e 5d
c3 0f 1f 00 53 48 89 fb 48 8d 7f 10 e8 73 7d a1 ff <48> 8b 7b 10 48 8d 73 18 5b e9 75 53 fc ff 0
f 1f 44 00 00 48 b8 00
[ 199.130041] RSP: 0018:ffff888103f3fc88 EFLAGS: 00010282
[ 199.130329] RAX: 0000000000000001 RBX: 0000000000000000 RCX: ffffffff8214d46d
[ 199.130733] RDX: 1ffffffff079c6b9 RSI: 0000000000000246 RDI: ffffffff83ce35c8
[ 199.131119] RBP: ffff888103d25458 R08: 0000000000000001 R09: fffffbfff0791761
[ 199.131505] R10: ffffffff83c8bb07 R11: fffffbfff0791760 R12: 0000000000000000
[ 199.131891] R13: ffff888103d25468 R14: ffff888103d25418 R15: ffff888103f18120
[ 199.132277] FS: 00007f36fdcbb6a0(0000) GS:ffff88815b400000(0000) knlGS:0000000000000000
[ 199.132721] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 199.133033] CR2: 0000000000000010 CR3: 0000000103d26000 CR4: 00000000000006f0
[ 199.133420] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 199.133807] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 199.134195] Call Trace:
[ 199.134333] drm_fbdev_cleanup+0x179/0x1a0
[ 199.134562] drm_fbdev_client_unregister+0x2b/0x40
[ 199.134828] drm_client_dev_unregister+0xa8/0x180
[ 199.135088] drm_dev_unregister+0x61/0x110
[ 199.135315] mgag200_pci_remove+0x38/0x52 [mgag200]
[ 199.135586] pci_device_remove+0x62/0xe0
[ 199.135806] device_release_driver_internal+0x148/0x270
[ 199.136094] driver_detach+0x76/0xe0
[ 199.136294] bus_remove_driver+0x7e/0x100
[ 199.136521] pci_unregister_driver+0x28/0xf0
[ 199.136759] __x64_sys_delete_module+0x268/0x300
[ 199.137016] ? __ia32_sys_delete_module+0x300/0x300
[ 199.137285] ? call_rcu+0x3e4/0x580
[ 199.137481] ? fpregs_assert_state_consistent+0x4d/0x60
[ 199.137767] ? exit_to_user_mode_prepare+0x2f/0x130
[ 199.138037] do_syscall_64+0x33/0x40
[ 199.138237] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 199.138517] RIP: 0033:0x7f36fdc3dcf7
Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
drivers/gpu/drm/drm_fb_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index b9a616737c0e..f6baa2046124 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2048,7 +2048,7 @@ static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
if (shadow)
vfree(shadow);
- else
+ else if (fb_helper->buffer)
drm_client_buffer_vunmap(fb_helper->buffer);
drm_client_framebuffer_delete(fb_helper->buffer);
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/fb-helper: only unmap if buffer not null
2021-02-28 4:46 [PATCH] drm/fb-helper: only unmap if buffer not null Tong Zhang
@ 2021-03-01 8:26 ` Thomas Zimmermann
2021-03-02 3:29 ` Tong Zhang
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Zimmermann @ 2021-03-01 8:26 UTC (permalink / raw)
To: Tong Zhang, Maarten Lankhorst, Maxime Ripard, David Airlie,
Daniel Vetter, dri-devel, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 3668 bytes --]
Hi
Am 28.02.21 um 05:46 schrieb Tong Zhang:
> drm_fbdev_cleanup() can be called when fb_helper->buffer is null, hence
> fb_helper->buffer should be checked before calling
> drm_client_buffer_vunmap(). This buffer is also checked in
> drm_client_framebuffer_delete(), so we should also do the same thing for
> drm_client_buffer_vunmap().
I think a lot of drivers are affected by this problem; probably most of
the ones that use the generic fbdev code. How did you produce the error?
What I'm more concerned about is why the buffer is NULL. Was ther eno
hotplug event? Do you have a display attached?
Best regards
Thomas
>
> [ 199.128742] RIP: 0010:drm_client_buffer_vunmap+0xd/0x20
> [ 199.129031] Code: 43 18 48 8b 53 20 49 89 45 00 49 89 55 08 5b 44 89 e0 41 5c 41 5d 41 5e 5d
> c3 0f 1f 00 53 48 89 fb 48 8d 7f 10 e8 73 7d a1 ff <48> 8b 7b 10 48 8d 73 18 5b e9 75 53 fc ff 0
> f 1f 44 00 00 48 b8 00
> [ 199.130041] RSP: 0018:ffff888103f3fc88 EFLAGS: 00010282
> [ 199.130329] RAX: 0000000000000001 RBX: 0000000000000000 RCX: ffffffff8214d46d
> [ 199.130733] RDX: 1ffffffff079c6b9 RSI: 0000000000000246 RDI: ffffffff83ce35c8
> [ 199.131119] RBP: ffff888103d25458 R08: 0000000000000001 R09: fffffbfff0791761
> [ 199.131505] R10: ffffffff83c8bb07 R11: fffffbfff0791760 R12: 0000000000000000
> [ 199.131891] R13: ffff888103d25468 R14: ffff888103d25418 R15: ffff888103f18120
> [ 199.132277] FS: 00007f36fdcbb6a0(0000) GS:ffff88815b400000(0000) knlGS:0000000000000000
> [ 199.132721] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 199.133033] CR2: 0000000000000010 CR3: 0000000103d26000 CR4: 00000000000006f0
> [ 199.133420] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 199.133807] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 199.134195] Call Trace:
> [ 199.134333] drm_fbdev_cleanup+0x179/0x1a0
> [ 199.134562] drm_fbdev_client_unregister+0x2b/0x40
> [ 199.134828] drm_client_dev_unregister+0xa8/0x180
> [ 199.135088] drm_dev_unregister+0x61/0x110
> [ 199.135315] mgag200_pci_remove+0x38/0x52 [mgag200]
> [ 199.135586] pci_device_remove+0x62/0xe0
> [ 199.135806] device_release_driver_internal+0x148/0x270
> [ 199.136094] driver_detach+0x76/0xe0
> [ 199.136294] bus_remove_driver+0x7e/0x100
> [ 199.136521] pci_unregister_driver+0x28/0xf0
> [ 199.136759] __x64_sys_delete_module+0x268/0x300
> [ 199.137016] ? __ia32_sys_delete_module+0x300/0x300
> [ 199.137285] ? call_rcu+0x3e4/0x580
> [ 199.137481] ? fpregs_assert_state_consistent+0x4d/0x60
> [ 199.137767] ? exit_to_user_mode_prepare+0x2f/0x130
> [ 199.138037] do_syscall_64+0x33/0x40
> [ 199.138237] entry_SYSCALL_64_after_hwframe+0x44/0xae
> [ 199.138517] RIP: 0033:0x7f36fdc3dcf7
>
> Signed-off-by: Tong Zhang <ztong0001@gmail.com>
> ---
> drivers/gpu/drm/drm_fb_helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index b9a616737c0e..f6baa2046124 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2048,7 +2048,7 @@ static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
>
> if (shadow)
> vfree(shadow);
> - else
> + else if (fb_helper->buffer)
> drm_client_buffer_vunmap(fb_helper->buffer);
>
> drm_client_framebuffer_delete(fb_helper->buffer);
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/fb-helper: only unmap if buffer not null
2021-03-01 8:26 ` Thomas Zimmermann
@ 2021-03-02 3:29 ` Tong Zhang
2021-03-02 10:06 ` Thomas Zimmermann
0 siblings, 1 reply; 4+ messages in thread
From: Tong Zhang @ 2021-03-02 3:29 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: Maarten Lankhorst, Maxime Ripard, David Airlie, Daniel Vetter,
DRI Development, open list
Hi Tomas,
I think the issue could be possibly caused by the following,
Please correct me if I'm wrong.
drm_fb_helper_single_fb_probe() can fail with
"Cannot find any crtc or sizes"
which will cause fb_helper->funcs->fb_probe not being called,
thus fb_helper->buffer remains NULL --
Since there could be the case that the fb_probe is never called,
a subsequent modprobe -r will cause the following
drm_client_buffer_vunmap(NULL) in drm_fbdev_cleanup()
Best,
- Tong
On Mon, Mar 1, 2021 at 3:26 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Hi
>
> Am 28.02.21 um 05:46 schrieb Tong Zhang:
> > drm_fbdev_cleanup() can be called when fb_helper->buffer is null, hence
> > fb_helper->buffer should be checked before calling
> > drm_client_buffer_vunmap(). This buffer is also checked in
> > drm_client_framebuffer_delete(), so we should also do the same thing for
> > drm_client_buffer_vunmap().
>
> I think a lot of drivers are affected by this problem; probably most of
> the ones that use the generic fbdev code. How did you produce the error?
>
> What I'm more concerned about is why the buffer is NULL. Was ther eno
> hotplug event? Do you have a display attached?
>
> Best regards
> Thomas
>
>
> >
> > [ 199.128742] RIP: 0010:drm_client_buffer_vunmap+0xd/0x20
> > [ 199.129031] Code: 43 18 48 8b 53 20 49 89 45 00 49 89 55 08 5b 44 89 e0 41 5c 41 5d 41 5e 5d
> > c3 0f 1f 00 53 48 89 fb 48 8d 7f 10 e8 73 7d a1 ff <48> 8b 7b 10 48 8d 73 18 5b e9 75 53 fc ff 0
> > f 1f 44 00 00 48 b8 00
> > [ 199.130041] RSP: 0018:ffff888103f3fc88 EFLAGS: 00010282
> > [ 199.130329] RAX: 0000000000000001 RBX: 0000000000000000 RCX: ffffffff8214d46d
> > [ 199.130733] RDX: 1ffffffff079c6b9 RSI: 0000000000000246 RDI: ffffffff83ce35c8
> > [ 199.131119] RBP: ffff888103d25458 R08: 0000000000000001 R09: fffffbfff0791761
> > [ 199.131505] R10: ffffffff83c8bb07 R11: fffffbfff0791760 R12: 0000000000000000
> > [ 199.131891] R13: ffff888103d25468 R14: ffff888103d25418 R15: ffff888103f18120
> > [ 199.132277] FS: 00007f36fdcbb6a0(0000) GS:ffff88815b400000(0000) knlGS:0000000000000000
> > [ 199.132721] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [ 199.133033] CR2: 0000000000000010 CR3: 0000000103d26000 CR4: 00000000000006f0
> > [ 199.133420] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > [ 199.133807] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > [ 199.134195] Call Trace:
> > [ 199.134333] drm_fbdev_cleanup+0x179/0x1a0
> > [ 199.134562] drm_fbdev_client_unregister+0x2b/0x40
> > [ 199.134828] drm_client_dev_unregister+0xa8/0x180
> > [ 199.135088] drm_dev_unregister+0x61/0x110
> > [ 199.135315] mgag200_pci_remove+0x38/0x52 [mgag200]
> > [ 199.135586] pci_device_remove+0x62/0xe0
> > [ 199.135806] device_release_driver_internal+0x148/0x270
> > [ 199.136094] driver_detach+0x76/0xe0
> > [ 199.136294] bus_remove_driver+0x7e/0x100
> > [ 199.136521] pci_unregister_driver+0x28/0xf0
> > [ 199.136759] __x64_sys_delete_module+0x268/0x300
> > [ 199.137016] ? __ia32_sys_delete_module+0x300/0x300
> > [ 199.137285] ? call_rcu+0x3e4/0x580
> > [ 199.137481] ? fpregs_assert_state_consistent+0x4d/0x60
> > [ 199.137767] ? exit_to_user_mode_prepare+0x2f/0x130
> > [ 199.138037] do_syscall_64+0x33/0x40
> > [ 199.138237] entry_SYSCALL_64_after_hwframe+0x44/0xae
> > [ 199.138517] RIP: 0033:0x7f36fdc3dcf7
> >
> > Signed-off-by: Tong Zhang <ztong0001@gmail.com>
> > ---
> > drivers/gpu/drm/drm_fb_helper.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> > index b9a616737c0e..f6baa2046124 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -2048,7 +2048,7 @@ static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
> >
> > if (shadow)
> > vfree(shadow);
> > - else
> > + else if (fb_helper->buffer)
> > drm_client_buffer_vunmap(fb_helper->buffer);
> >
> > drm_client_framebuffer_delete(fb_helper->buffer);
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/fb-helper: only unmap if buffer not null
2021-03-02 3:29 ` Tong Zhang
@ 2021-03-02 10:06 ` Thomas Zimmermann
0 siblings, 0 replies; 4+ messages in thread
From: Thomas Zimmermann @ 2021-03-02 10:06 UTC (permalink / raw)
To: Tong Zhang; +Cc: David Airlie, open list, DRI Development
[-- Attachment #1.1: Type: text/plain, Size: 5006 bytes --]
Hi
Am 02.03.21 um 04:29 schrieb Tong Zhang:
> Hi Tomas,
>
> I think the issue could be possibly caused by the following,
> Please correct me if I'm wrong.
>
> drm_fb_helper_single_fb_probe() can fail with
> "Cannot find any crtc or sizes"
> which will cause fb_helper->funcs->fb_probe not being called,
> thus fb_helper->buffer remains NULL --
> Since there could be the case that the fb_probe is never called,
> a subsequent modprobe -r will cause the following
> drm_client_buffer_vunmap(NULL) in drm_fbdev_cleanup()
Thanks! I added Fixes tags to your patch and merged it into drm-misc-fixes.
Best regards
Thomas
>
> Best,
> - Tong
>
> On Mon, Mar 1, 2021 at 3:26 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Hi
>>
>> Am 28.02.21 um 05:46 schrieb Tong Zhang:
>>> drm_fbdev_cleanup() can be called when fb_helper->buffer is null, hence
>>> fb_helper->buffer should be checked before calling
>>> drm_client_buffer_vunmap(). This buffer is also checked in
>>> drm_client_framebuffer_delete(), so we should also do the same thing for
>>> drm_client_buffer_vunmap().
>>
>> I think a lot of drivers are affected by this problem; probably most of
>> the ones that use the generic fbdev code. How did you produce the error?
>>
>> What I'm more concerned about is why the buffer is NULL. Was ther eno
>> hotplug event? Do you have a display attached?
>>
>> Best regards
>> Thomas
>>
>>
>>>
>>> [ 199.128742] RIP: 0010:drm_client_buffer_vunmap+0xd/0x20
>>> [ 199.129031] Code: 43 18 48 8b 53 20 49 89 45 00 49 89 55 08 5b 44 89 e0 41 5c 41 5d 41 5e 5d
>>> c3 0f 1f 00 53 48 89 fb 48 8d 7f 10 e8 73 7d a1 ff <48> 8b 7b 10 48 8d 73 18 5b e9 75 53 fc ff 0
>>> f 1f 44 00 00 48 b8 00
>>> [ 199.130041] RSP: 0018:ffff888103f3fc88 EFLAGS: 00010282
>>> [ 199.130329] RAX: 0000000000000001 RBX: 0000000000000000 RCX: ffffffff8214d46d
>>> [ 199.130733] RDX: 1ffffffff079c6b9 RSI: 0000000000000246 RDI: ffffffff83ce35c8
>>> [ 199.131119] RBP: ffff888103d25458 R08: 0000000000000001 R09: fffffbfff0791761
>>> [ 199.131505] R10: ffffffff83c8bb07 R11: fffffbfff0791760 R12: 0000000000000000
>>> [ 199.131891] R13: ffff888103d25468 R14: ffff888103d25418 R15: ffff888103f18120
>>> [ 199.132277] FS: 00007f36fdcbb6a0(0000) GS:ffff88815b400000(0000) knlGS:0000000000000000
>>> [ 199.132721] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [ 199.133033] CR2: 0000000000000010 CR3: 0000000103d26000 CR4: 00000000000006f0
>>> [ 199.133420] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>>> [ 199.133807] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>>> [ 199.134195] Call Trace:
>>> [ 199.134333] drm_fbdev_cleanup+0x179/0x1a0
>>> [ 199.134562] drm_fbdev_client_unregister+0x2b/0x40
>>> [ 199.134828] drm_client_dev_unregister+0xa8/0x180
>>> [ 199.135088] drm_dev_unregister+0x61/0x110
>>> [ 199.135315] mgag200_pci_remove+0x38/0x52 [mgag200]
>>> [ 199.135586] pci_device_remove+0x62/0xe0
>>> [ 199.135806] device_release_driver_internal+0x148/0x270
>>> [ 199.136094] driver_detach+0x76/0xe0
>>> [ 199.136294] bus_remove_driver+0x7e/0x100
>>> [ 199.136521] pci_unregister_driver+0x28/0xf0
>>> [ 199.136759] __x64_sys_delete_module+0x268/0x300
>>> [ 199.137016] ? __ia32_sys_delete_module+0x300/0x300
>>> [ 199.137285] ? call_rcu+0x3e4/0x580
>>> [ 199.137481] ? fpregs_assert_state_consistent+0x4d/0x60
>>> [ 199.137767] ? exit_to_user_mode_prepare+0x2f/0x130
>>> [ 199.138037] do_syscall_64+0x33/0x40
>>> [ 199.138237] entry_SYSCALL_64_after_hwframe+0x44/0xae
>>> [ 199.138517] RIP: 0033:0x7f36fdc3dcf7
>>>
>>> Signed-off-by: Tong Zhang <ztong0001@gmail.com>
>>> ---
>>> drivers/gpu/drm/drm_fb_helper.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
>>> index b9a616737c0e..f6baa2046124 100644
>>> --- a/drivers/gpu/drm/drm_fb_helper.c
>>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>>> @@ -2048,7 +2048,7 @@ static void drm_fbdev_cleanup(struct drm_fb_helper *fb_helper)
>>>
>>> if (shadow)
>>> vfree(shadow);
>>> - else
>>> + else if (fb_helper->buffer)
>>> drm_client_buffer_vunmap(fb_helper->buffer);
>>>
>>> drm_client_framebuffer_delete(fb_helper->buffer);
>>>
>>
>> --
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>> (HRB 36809, AG Nürnberg)
>> Geschäftsführer: Felix Imendörffer
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-02 10:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-28 4:46 [PATCH] drm/fb-helper: only unmap if buffer not null Tong Zhang
2021-03-01 8:26 ` Thomas Zimmermann
2021-03-02 3:29 ` Tong Zhang
2021-03-02 10:06 ` Thomas Zimmermann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).