diff mbox series

drm/gem: add checks of drm_gem_object->funcs

Message ID 20210228161022.53468-1-ledoian@kam.mff.cuni.cz (mailing list archive)
State New, archived
Headers show
Series drm/gem: add checks of drm_gem_object->funcs | expand

Commit Message

Pavel Turinský Feb. 28, 2021, 4:10 p.m. UTC
The checks were removed in commit d693def4fd1c ("drm: Remove obsolete GEM
and PRIME callbacks from struct drm_driver") and can lead to following
kernel oops:

[  139.449098] BUG: kernel NULL pointer dereference, address: 0000000000000008
[  139.449110] #PF: supervisor read access in kernel mode
[  139.449113] #PF: error_code(0x0000) - not-present page
[  139.449116] PGD 0 P4D 0
[  139.449121] Oops: 0000 [#1] PREEMPT SMP PTI
[  139.449126] CPU: 4 PID: 1181 Comm: Xorg Not tainted 5.11.2LEdoian #2
[  139.449130] Hardware name: Gigabyte Technology Co., Ltd. To be filled by O.E.M./Z77-DS3H, BIOS F4 04/25/2012
[  139.449133] RIP: 0010:drm_gem_handle_create_tail+0xcb/0x190 [drm]
[  139.449185] Code: 00 48 89 ef e8 06 b4 49 f7 45 85 e4 78 77 48 8d 6b 18 4c 89 ee 48 89 ef e8 c2 f5 00 00 89 c2 85 c0 75 3e 48 8b 83 40 01 00 00 <48> 8b 40 0
8 48 85 c0 74 0f 4c 89 ee 48 89 df e8 71 5d 87 f7 85 c0
[  139.449190] RSP: 0018:ffffbe21c194bd28 EFLAGS: 00010246
[  139.449194] RAX: 0000000000000000 RBX: ffff9da9b3caf078 RCX: 0000000000000000
[  139.449197] RDX: 0000000000000000 RSI: ffffffffc039b893 RDI: 0000000000000000
[  139.449199] RBP: ffff9da9b3caf090 R08: 0000000000000040 R09: ffff9da983b911c0
[  139.449202] R10: ffff9da984749e00 R11: ffff9da9859bfc38 R12: 0000000000000007
[  139.449204] R13: ffff9da9859bfc00 R14: ffff9da9859bfc50 R15: ffff9da9859bfc38
[  139.449207] FS:  00007f6332a56900(0000) GS:ffff9daea7b00000(0000) knlGS:0000000000000000
[  139.449211] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  139.449214] CR2: 0000000000000008 CR3: 00000001319b8005 CR4: 00000000001706e0
[  139.449217] Call Trace:
[  139.449224]  drm_gem_prime_fd_to_handle+0xff/0x1d0 [drm]
[  139.449274]  ? drm_prime_destroy_file_private+0x20/0x20 [drm]
[  139.449323]  drm_ioctl_kernel+0xac/0xf0 [drm]
[  139.449363]  drm_ioctl+0x20f/0x3b0 [drm]
[  139.449403]  ? drm_prime_destroy_file_private+0x20/0x20 [drm]
[  139.449454]  radeon_drm_ioctl+0x49/0x80 [radeon]
[  139.449500]  __x64_sys_ioctl+0x84/0xc0
[  139.449507]  do_syscall_64+0x33/0x40
[  139.449514]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[  139.449522] RIP: 0033:0x7f63330fbe6b
[  139.449526] Code: ff ff ff 85 c0 79 8b 49 c7 c4 ff ff ff ff 5b 5d 4c 89 e0 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f
0 ff ff 73 01 c3 48 8b 0d d5 af 0c 00 f7 d8 64 89 01 48
[  139.449529] RSP: 002b:00007fff1e9c4438 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
[  139.449534] RAX: ffffffffffffffda RBX: 00007fff1e9c447c RCX: 00007f63330fbe6b
[  139.449537] RDX: 00007fff1e9c447c RSI: 00000000c00c642e RDI: 0000000000000012
[  139.449539] RBP: 00000000c00c642e R08: 00007fff1e9c4520 R09: 00007f63331c7a60
[  139.449542] R10: 00007f6329fb9ab0 R11: 0000000000000246 R12: 000055f69810ad40
[  139.449544] R13: 0000000000000012 R14: 0000000000100000 R15: 00007fff1e9c4c20
[  139.449549] Modules linked in: 8021q garp mrp bridge stp llc nls_iso8859_1 vfat fat fuse btrfs blake2b_generic xor raid6_pq libcrc32c crypto_user tun i2c_de
v it87 hwmon_vid snd_seq snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio sg snd_hda_codec_hdmi virtio_balloon snd_hda_intel virtio_console snd_intel_
dspcfg soundwire_intel virtio_pci soundwire_generic_allocation soundwire_cadence virtio_blk snd_hda_codec intel_rapl_msr btusb intel_rapl_common virtio_net btr
tl net_failover uvcvideo snd_usb_audio snd_hda_core btbcm x86_pkg_temp_thermal failover soundwire_bus btintel intel_powerclamp snd_soc_core coretemp snd_usbmid
i_lib iTCO_wdt videobuf2_vmalloc bluetooth intel_pmc_bxt snd_hwdep kvm_intel videobuf2_memops snd_rawmidi snd_compress videobuf2_v4l2 ac97_bus snd_pcm_dmaengin
e iTCO_vendor_support crct10dif_pclmul at24 crc32_pclmul videobuf2_common snd_seq_device mei_hdcp snd_pcm ghash_clmulni_intel kvm videodev aesni_intel crypto_s
imd snd_timer snd cryptd mc ecdh_generic
[  139.449642]  glue_helper rfkill soundcore joydev mousedev rapl ecc intel_cstate r8169 i2c_i801 intel_uncore atl1c realtek irqbypass mdio_devres mei_me libph
y i2c_smbus mei mac_hid lpc_ich ext4 crc32c_generic crc16 mbcache jbd2 dm_mod ata_generic pata_acpi uas usb_storage sr_mod crc32c_intel serio_raw cdrom xhci_pc
i pata_jmicron xhci_pci_renesas radeon usbhid i915 intel_gtt nouveau mxm_wmi wmi video i2c_algo_bit drm_ttm_helper ttm drm_kms_helper syscopyarea sysfillrect s
ysimgblt fb_sys_fops cec drm agpgart
[  139.449707] CR2: 0000000000000008
[  139.449710] ---[ end trace f5ce5774498d18e1 ]---

Signed-off-by: Pavel Turinský <ledoian@kam.mff.cuni.cz>
Fixes: d693def4fd1c ("drm: Remove obsolete GEM and PRIME callbacks from struct drm_driver")
Cc: stable@vger.kernel.org
---

This is a very symptomatic patch around issue I ran into. I do not know if the
funcs property should ever be NULL. I basically restored all the checks that
were removed in the mentioned commit. Unfortunately, I do not understand drm
nor will I have time to delve into it in forseeable future.

 drivers/gpu/drm/drm_gem.c   | 20 ++++++++++----------
 drivers/gpu/drm/drm_prime.c |  2 +-
 2 files changed, 11 insertions(+), 11 deletions(-)

Comments

Thomas Zimmermann March 1, 2021, 9:56 a.m. UTC | #1
(cc'ing amd devs)

Hi

Am 28.02.21 um 17:10 schrieb Pavel Turinský:
> The checks were removed in commit d693def4fd1c ("drm: Remove obsolete GEM
> and PRIME callbacks from struct drm_driver") and can lead to following
> kernel oops:

Thanks for reporting. All drivers are supposed to set the funcs pointer 
in their GEM objects. This looks like a radeon bug. Adding the AMD devs.

Best regards
Thomas

> 
> [  139.449098] BUG: kernel NULL pointer dereference, address: 0000000000000008
> [  139.449110] #PF: supervisor read access in kernel mode
> [  139.449113] #PF: error_code(0x0000) - not-present page
> [  139.449116] PGD 0 P4D 0
> [  139.449121] Oops: 0000 [#1] PREEMPT SMP PTI
> [  139.449126] CPU: 4 PID: 1181 Comm: Xorg Not tainted 5.11.2LEdoian #2
> [  139.449130] Hardware name: Gigabyte Technology Co., Ltd. To be filled by O.E.M./Z77-DS3H, BIOS F4 04/25/2012
> [  139.449133] RIP: 0010:drm_gem_handle_create_tail+0xcb/0x190 [drm]
> [  139.449185] Code: 00 48 89 ef e8 06 b4 49 f7 45 85 e4 78 77 48 8d 6b 18 4c 89 ee 48 89 ef e8 c2 f5 00 00 89 c2 85 c0 75 3e 48 8b 83 40 01 00 00 <48> 8b 40 0
> 8 48 85 c0 74 0f 4c 89 ee 48 89 df e8 71 5d 87 f7 85 c0
> [  139.449190] RSP: 0018:ffffbe21c194bd28 EFLAGS: 00010246
> [  139.449194] RAX: 0000000000000000 RBX: ffff9da9b3caf078 RCX: 0000000000000000
> [  139.449197] RDX: 0000000000000000 RSI: ffffffffc039b893 RDI: 0000000000000000
> [  139.449199] RBP: ffff9da9b3caf090 R08: 0000000000000040 R09: ffff9da983b911c0
> [  139.449202] R10: ffff9da984749e00 R11: ffff9da9859bfc38 R12: 0000000000000007
> [  139.449204] R13: ffff9da9859bfc00 R14: ffff9da9859bfc50 R15: ffff9da9859bfc38
> [  139.449207] FS:  00007f6332a56900(0000) GS:ffff9daea7b00000(0000) knlGS:0000000000000000
> [  139.449211] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  139.449214] CR2: 0000000000000008 CR3: 00000001319b8005 CR4: 00000000001706e0
> [  139.449217] Call Trace:
> [  139.449224]  drm_gem_prime_fd_to_handle+0xff/0x1d0 [drm]
> [  139.449274]  ? drm_prime_destroy_file_private+0x20/0x20 [drm]
> [  139.449323]  drm_ioctl_kernel+0xac/0xf0 [drm]
> [  139.449363]  drm_ioctl+0x20f/0x3b0 [drm]
> [  139.449403]  ? drm_prime_destroy_file_private+0x20/0x20 [drm]
> [  139.449454]  radeon_drm_ioctl+0x49/0x80 [radeon]
> [  139.449500]  __x64_sys_ioctl+0x84/0xc0
> [  139.449507]  do_syscall_64+0x33/0x40
> [  139.449514]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [  139.449522] RIP: 0033:0x7f63330fbe6b
> [  139.449526] Code: ff ff ff 85 c0 79 8b 49 c7 c4 ff ff ff ff 5b 5d 4c 89 e0 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f
> 0 ff ff 73 01 c3 48 8b 0d d5 af 0c 00 f7 d8 64 89 01 48
> [  139.449529] RSP: 002b:00007fff1e9c4438 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> [  139.449534] RAX: ffffffffffffffda RBX: 00007fff1e9c447c RCX: 00007f63330fbe6b
> [  139.449537] RDX: 00007fff1e9c447c RSI: 00000000c00c642e RDI: 0000000000000012
> [  139.449539] RBP: 00000000c00c642e R08: 00007fff1e9c4520 R09: 00007f63331c7a60
> [  139.449542] R10: 00007f6329fb9ab0 R11: 0000000000000246 R12: 000055f69810ad40
> [  139.449544] R13: 0000000000000012 R14: 0000000000100000 R15: 00007fff1e9c4c20
> [  139.449549] Modules linked in: 8021q garp mrp bridge stp llc nls_iso8859_1 vfat fat fuse btrfs blake2b_generic xor raid6_pq libcrc32c crypto_user tun i2c_de
> v it87 hwmon_vid snd_seq snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio sg snd_hda_codec_hdmi virtio_balloon snd_hda_intel virtio_console snd_intel_
> dspcfg soundwire_intel virtio_pci soundwire_generic_allocation soundwire_cadence virtio_blk snd_hda_codec intel_rapl_msr btusb intel_rapl_common virtio_net btr
> tl net_failover uvcvideo snd_usb_audio snd_hda_core btbcm x86_pkg_temp_thermal failover soundwire_bus btintel intel_powerclamp snd_soc_core coretemp snd_usbmid
> i_lib iTCO_wdt videobuf2_vmalloc bluetooth intel_pmc_bxt snd_hwdep kvm_intel videobuf2_memops snd_rawmidi snd_compress videobuf2_v4l2 ac97_bus snd_pcm_dmaengin
> e iTCO_vendor_support crct10dif_pclmul at24 crc32_pclmul videobuf2_common snd_seq_device mei_hdcp snd_pcm ghash_clmulni_intel kvm videodev aesni_intel crypto_s
> imd snd_timer snd cryptd mc ecdh_generic
> [  139.449642]  glue_helper rfkill soundcore joydev mousedev rapl ecc intel_cstate r8169 i2c_i801 intel_uncore atl1c realtek irqbypass mdio_devres mei_me libph
> y i2c_smbus mei mac_hid lpc_ich ext4 crc32c_generic crc16 mbcache jbd2 dm_mod ata_generic pata_acpi uas usb_storage sr_mod crc32c_intel serio_raw cdrom xhci_pc
> i pata_jmicron xhci_pci_renesas radeon usbhid i915 intel_gtt nouveau mxm_wmi wmi video i2c_algo_bit drm_ttm_helper ttm drm_kms_helper syscopyarea sysfillrect s
> ysimgblt fb_sys_fops cec drm agpgart
> [  139.449707] CR2: 0000000000000008
> [  139.449710] ---[ end trace f5ce5774498d18e1 ]---
> 
> Signed-off-by: Pavel Turinský <ledoian@kam.mff.cuni.cz>
> Fixes: d693def4fd1c ("drm: Remove obsolete GEM and PRIME callbacks from struct drm_driver")
> Cc: stable@vger.kernel.org
> ---
> 
> This is a very symptomatic patch around issue I ran into. I do not know if the
> funcs property should ever be NULL. I basically restored all the checks that
> were removed in the mentioned commit. Unfortunately, I do not understand drm
> nor will I have time to delve into it in forseeable future.
> 
>   drivers/gpu/drm/drm_gem.c   | 20 ++++++++++----------
>   drivers/gpu/drm/drm_prime.c |  2 +-
>   2 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 92f89cee213e..451f290c737c 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -249,7 +249,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
>   	struct drm_file *file_priv = data;
>   	struct drm_gem_object *obj = ptr;
>   
> -	if (obj->funcs->close)
> +	if (obj->funcs && obj->funcs->close)
>   		obj->funcs->close(obj, file_priv);
>   
>   	drm_gem_remove_prime_handles(obj, file_priv);
> @@ -401,7 +401,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
>   	if (ret)
>   		goto err_remove;
>   
> -	if (obj->funcs->open) {
> +	if (obj->funcs && obj->funcs->open) {
>   		ret = obj->funcs->open(obj, file_priv);
>   		if (ret)
>   			goto err_revoke;
> @@ -977,7 +977,7 @@ drm_gem_object_free(struct kref *kref)
>   	struct drm_gem_object *obj =
>   		container_of(kref, struct drm_gem_object, refcount);
>   
> -	if (WARN_ON(!obj->funcs->free))
> +	if (!obj->funcs || WARN_ON(!obj->funcs->free))
>   		return;
>   
>   	obj->funcs->free(obj);
> @@ -1079,7 +1079,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
>   
>   	vma->vm_private_data = obj;
>   
> -	if (obj->funcs->mmap) {
> +	if (obj->funcs && obj->funcs->mmap) {
>   		ret = obj->funcs->mmap(obj, vma);
>   		if (ret) {
>   			drm_gem_object_put(obj);
> @@ -1087,7 +1087,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
>   		}
>   		WARN_ON(!(vma->vm_flags & VM_DONTEXPAND));
>   	} else {
> -		if (obj->funcs->vm_ops)
> +		if (obj->funcs && obj->funcs->vm_ops)
>   			vma->vm_ops = obj->funcs->vm_ops;
>   		else {
>   			drm_gem_object_put(obj);
> @@ -1188,13 +1188,13 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
>   	drm_printf_indent(p, indent, "imported=%s\n",
>   			  obj->import_attach ? "yes" : "no");
>   
> -	if (obj->funcs->print_info)
> +	if (obj->funcs && obj->funcs->print_info)
>   		obj->funcs->print_info(p, indent, obj);
>   }
>   
>   int drm_gem_pin(struct drm_gem_object *obj)
>   {
> -	if (obj->funcs->pin)
> +	if (obj->funcs && obj->funcs->pin)
>   		return obj->funcs->pin(obj);
>   	else
>   		return 0;
> @@ -1202,7 +1202,7 @@ int drm_gem_pin(struct drm_gem_object *obj)
>   
>   void drm_gem_unpin(struct drm_gem_object *obj)
>   {
> -	if (obj->funcs->unpin)
> +	if (obj->funcs && obj->funcs->unpin)
>   		obj->funcs->unpin(obj);
>   }
>   
> @@ -1210,7 +1210,7 @@ int drm_gem_vmap(struct drm_gem_object *obj, struct dma_buf_map *map)
>   {
>   	int ret;
>   
> -	if (!obj->funcs->vmap)
> +	if (!obj->funcs || !obj->funcs->vmap)
>   		return -EOPNOTSUPP;
>   
>   	ret = obj->funcs->vmap(obj, map);
> @@ -1227,7 +1227,7 @@ void drm_gem_vunmap(struct drm_gem_object *obj, struct dma_buf_map *map)
>   	if (dma_buf_map_is_null(map))
>   		return;
>   
> -	if (obj->funcs->vunmap)
> +	if (obj->funcs && obj->funcs->vunmap)
>   		obj->funcs->vunmap(obj, map);
>   
>   	/* Always set the mapping to NULL. Callers may rely on this. */
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 7db55fce35d8..1566dcf417e2 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -620,7 +620,7 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
>   	if (WARN_ON(dir == DMA_NONE))
>   		return ERR_PTR(-EINVAL);
>   
> -	if (WARN_ON(!obj->funcs->get_sg_table))
> +	if (!obj->funcs || WARN_ON(!obj->funcs->get_sg_table))
>   		return ERR_PTR(-ENOSYS);
>   
>   	sgt = obj->funcs->get_sg_table(obj);
>
Daniel Vetter March 1, 2021, 10:04 a.m. UTC | #2
On Mon, Mar 1, 2021 at 10:56 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> (cc'ing amd devs)
>
> Hi
>
> Am 28.02.21 um 17:10 schrieb Pavel Turinský:
> > The checks were removed in commit d693def4fd1c ("drm: Remove obsolete GEM
> > and PRIME callbacks from struct drm_driver") and can lead to following
> > kernel oops:
>
> Thanks for reporting. All drivers are supposed to set the funcs pointer
> in their GEM objects. This looks like a radeon bug. Adding the AMD devs.

Looks like we're setting obj->funcs only in radeon_gem_object_create,
but should set it in radeon_bo_create instead so it catches internal
functions too. I think this was missed in

commit ce77038fdae385f947757a37573d90f2e83f0271
Author: Gerd Hoffmann <kraxel@redhat.com>
Date:   Mon Aug 5 16:01:06 2019 +0200

   drm/radeon: use embedded gem object

Adding Gerd.
-Daniel

>
> Best regards
> Thomas
>
> >
> > [  139.449098] BUG: kernel NULL pointer dereference, address: 0000000000000008
> > [  139.449110] #PF: supervisor read access in kernel mode
> > [  139.449113] #PF: error_code(0x0000) - not-present page
> > [  139.449116] PGD 0 P4D 0
> > [  139.449121] Oops: 0000 [#1] PREEMPT SMP PTI
> > [  139.449126] CPU: 4 PID: 1181 Comm: Xorg Not tainted 5.11.2LEdoian #2
> > [  139.449130] Hardware name: Gigabyte Technology Co., Ltd. To be filled by O.E.M./Z77-DS3H, BIOS F4 04/25/2012
> > [  139.449133] RIP: 0010:drm_gem_handle_create_tail+0xcb/0x190 [drm]
> > [  139.449185] Code: 00 48 89 ef e8 06 b4 49 f7 45 85 e4 78 77 48 8d 6b 18 4c 89 ee 48 89 ef e8 c2 f5 00 00 89 c2 85 c0 75 3e 48 8b 83 40 01 00 00 <48> 8b 40 0
> > 8 48 85 c0 74 0f 4c 89 ee 48 89 df e8 71 5d 87 f7 85 c0
> > [  139.449190] RSP: 0018:ffffbe21c194bd28 EFLAGS: 00010246
> > [  139.449194] RAX: 0000000000000000 RBX: ffff9da9b3caf078 RCX: 0000000000000000
> > [  139.449197] RDX: 0000000000000000 RSI: ffffffffc039b893 RDI: 0000000000000000
> > [  139.449199] RBP: ffff9da9b3caf090 R08: 0000000000000040 R09: ffff9da983b911c0
> > [  139.449202] R10: ffff9da984749e00 R11: ffff9da9859bfc38 R12: 0000000000000007
> > [  139.449204] R13: ffff9da9859bfc00 R14: ffff9da9859bfc50 R15: ffff9da9859bfc38
> > [  139.449207] FS:  00007f6332a56900(0000) GS:ffff9daea7b00000(0000) knlGS:0000000000000000
> > [  139.449211] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [  139.449214] CR2: 0000000000000008 CR3: 00000001319b8005 CR4: 00000000001706e0
> > [  139.449217] Call Trace:
> > [  139.449224]  drm_gem_prime_fd_to_handle+0xff/0x1d0 [drm]
> > [  139.449274]  ? drm_prime_destroy_file_private+0x20/0x20 [drm]
> > [  139.449323]  drm_ioctl_kernel+0xac/0xf0 [drm]
> > [  139.449363]  drm_ioctl+0x20f/0x3b0 [drm]
> > [  139.449403]  ? drm_prime_destroy_file_private+0x20/0x20 [drm]
> > [  139.449454]  radeon_drm_ioctl+0x49/0x80 [radeon]
> > [  139.449500]  __x64_sys_ioctl+0x84/0xc0
> > [  139.449507]  do_syscall_64+0x33/0x40
> > [  139.449514]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > [  139.449522] RIP: 0033:0x7f63330fbe6b
> > [  139.449526] Code: ff ff ff 85 c0 79 8b 49 c7 c4 ff ff ff ff 5b 5d 4c 89 e0 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f
> > 0 ff ff 73 01 c3 48 8b 0d d5 af 0c 00 f7 d8 64 89 01 48
> > [  139.449529] RSP: 002b:00007fff1e9c4438 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> > [  139.449534] RAX: ffffffffffffffda RBX: 00007fff1e9c447c RCX: 00007f63330fbe6b
> > [  139.449537] RDX: 00007fff1e9c447c RSI: 00000000c00c642e RDI: 0000000000000012
> > [  139.449539] RBP: 00000000c00c642e R08: 00007fff1e9c4520 R09: 00007f63331c7a60
> > [  139.449542] R10: 00007f6329fb9ab0 R11: 0000000000000246 R12: 000055f69810ad40
> > [  139.449544] R13: 0000000000000012 R14: 0000000000100000 R15: 00007fff1e9c4c20
> > [  139.449549] Modules linked in: 8021q garp mrp bridge stp llc nls_iso8859_1 vfat fat fuse btrfs blake2b_generic xor raid6_pq libcrc32c crypto_user tun i2c_de
> > v it87 hwmon_vid snd_seq snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio sg snd_hda_codec_hdmi virtio_balloon snd_hda_intel virtio_console snd_intel_
> > dspcfg soundwire_intel virtio_pci soundwire_generic_allocation soundwire_cadence virtio_blk snd_hda_codec intel_rapl_msr btusb intel_rapl_common virtio_net btr
> > tl net_failover uvcvideo snd_usb_audio snd_hda_core btbcm x86_pkg_temp_thermal failover soundwire_bus btintel intel_powerclamp snd_soc_core coretemp snd_usbmid
> > i_lib iTCO_wdt videobuf2_vmalloc bluetooth intel_pmc_bxt snd_hwdep kvm_intel videobuf2_memops snd_rawmidi snd_compress videobuf2_v4l2 ac97_bus snd_pcm_dmaengin
> > e iTCO_vendor_support crct10dif_pclmul at24 crc32_pclmul videobuf2_common snd_seq_device mei_hdcp snd_pcm ghash_clmulni_intel kvm videodev aesni_intel crypto_s
> > imd snd_timer snd cryptd mc ecdh_generic
> > [  139.449642]  glue_helper rfkill soundcore joydev mousedev rapl ecc intel_cstate r8169 i2c_i801 intel_uncore atl1c realtek irqbypass mdio_devres mei_me libph
> > y i2c_smbus mei mac_hid lpc_ich ext4 crc32c_generic crc16 mbcache jbd2 dm_mod ata_generic pata_acpi uas usb_storage sr_mod crc32c_intel serio_raw cdrom xhci_pc
> > i pata_jmicron xhci_pci_renesas radeon usbhid i915 intel_gtt nouveau mxm_wmi wmi video i2c_algo_bit drm_ttm_helper ttm drm_kms_helper syscopyarea sysfillrect s
> > ysimgblt fb_sys_fops cec drm agpgart
> > [  139.449707] CR2: 0000000000000008
> > [  139.449710] ---[ end trace f5ce5774498d18e1 ]---
> >
> > Signed-off-by: Pavel Turinský <ledoian@kam.mff.cuni.cz>
> > Fixes: d693def4fd1c ("drm: Remove obsolete GEM and PRIME callbacks from struct drm_driver")
> > Cc: stable@vger.kernel.org
> > ---
> >
> > This is a very symptomatic patch around issue I ran into. I do not know if the
> > funcs property should ever be NULL. I basically restored all the checks that
> > were removed in the mentioned commit. Unfortunately, I do not understand drm
> > nor will I have time to delve into it in forseeable future.
> >
> >   drivers/gpu/drm/drm_gem.c   | 20 ++++++++++----------
> >   drivers/gpu/drm/drm_prime.c |  2 +-
> >   2 files changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > index 92f89cee213e..451f290c737c 100644
> > --- a/drivers/gpu/drm/drm_gem.c
> > +++ b/drivers/gpu/drm/drm_gem.c
> > @@ -249,7 +249,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
> >       struct drm_file *file_priv = data;
> >       struct drm_gem_object *obj = ptr;
> >
> > -     if (obj->funcs->close)
> > +     if (obj->funcs && obj->funcs->close)
> >               obj->funcs->close(obj, file_priv);
> >
> >       drm_gem_remove_prime_handles(obj, file_priv);
> > @@ -401,7 +401,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
> >       if (ret)
> >               goto err_remove;
> >
> > -     if (obj->funcs->open) {
> > +     if (obj->funcs && obj->funcs->open) {
> >               ret = obj->funcs->open(obj, file_priv);
> >               if (ret)
> >                       goto err_revoke;
> > @@ -977,7 +977,7 @@ drm_gem_object_free(struct kref *kref)
> >       struct drm_gem_object *obj =
> >               container_of(kref, struct drm_gem_object, refcount);
> >
> > -     if (WARN_ON(!obj->funcs->free))
> > +     if (!obj->funcs || WARN_ON(!obj->funcs->free))
> >               return;
> >
> >       obj->funcs->free(obj);
> > @@ -1079,7 +1079,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
> >
> >       vma->vm_private_data = obj;
> >
> > -     if (obj->funcs->mmap) {
> > +     if (obj->funcs && obj->funcs->mmap) {
> >               ret = obj->funcs->mmap(obj, vma);
> >               if (ret) {
> >                       drm_gem_object_put(obj);
> > @@ -1087,7 +1087,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
> >               }
> >               WARN_ON(!(vma->vm_flags & VM_DONTEXPAND));
> >       } else {
> > -             if (obj->funcs->vm_ops)
> > +             if (obj->funcs && obj->funcs->vm_ops)
> >                       vma->vm_ops = obj->funcs->vm_ops;
> >               else {
> >                       drm_gem_object_put(obj);
> > @@ -1188,13 +1188,13 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
> >       drm_printf_indent(p, indent, "imported=%s\n",
> >                         obj->import_attach ? "yes" : "no");
> >
> > -     if (obj->funcs->print_info)
> > +     if (obj->funcs && obj->funcs->print_info)
> >               obj->funcs->print_info(p, indent, obj);
> >   }
> >
> >   int drm_gem_pin(struct drm_gem_object *obj)
> >   {
> > -     if (obj->funcs->pin)
> > +     if (obj->funcs && obj->funcs->pin)
> >               return obj->funcs->pin(obj);
> >       else
> >               return 0;
> > @@ -1202,7 +1202,7 @@ int drm_gem_pin(struct drm_gem_object *obj)
> >
> >   void drm_gem_unpin(struct drm_gem_object *obj)
> >   {
> > -     if (obj->funcs->unpin)
> > +     if (obj->funcs && obj->funcs->unpin)
> >               obj->funcs->unpin(obj);
> >   }
> >
> > @@ -1210,7 +1210,7 @@ int drm_gem_vmap(struct drm_gem_object *obj, struct dma_buf_map *map)
> >   {
> >       int ret;
> >
> > -     if (!obj->funcs->vmap)
> > +     if (!obj->funcs || !obj->funcs->vmap)
> >               return -EOPNOTSUPP;
> >
> >       ret = obj->funcs->vmap(obj, map);
> > @@ -1227,7 +1227,7 @@ void drm_gem_vunmap(struct drm_gem_object *obj, struct dma_buf_map *map)
> >       if (dma_buf_map_is_null(map))
> >               return;
> >
> > -     if (obj->funcs->vunmap)
> > +     if (obj->funcs && obj->funcs->vunmap)
> >               obj->funcs->vunmap(obj, map);
> >
> >       /* Always set the mapping to NULL. Callers may rely on this. */
> > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> > index 7db55fce35d8..1566dcf417e2 100644
> > --- a/drivers/gpu/drm/drm_prime.c
> > +++ b/drivers/gpu/drm/drm_prime.c
> > @@ -620,7 +620,7 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
> >       if (WARN_ON(dir == DMA_NONE))
> >               return ERR_PTR(-EINVAL);
> >
> > -     if (WARN_ON(!obj->funcs->get_sg_table))
> > +     if (!obj->funcs || WARN_ON(!obj->funcs->get_sg_table))
> >               return ERR_PTR(-ENOSYS);
> >
> >       sgt = obj->funcs->get_sg_table(obj);
> >
>
> --
> 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
>
Christian König March 1, 2021, 10:25 a.m. UTC | #3
Am 01.03.21 um 11:04 schrieb Daniel Vetter:
> On Mon, Mar 1, 2021 at 10:56 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> (cc'ing amd devs)
>>
>> Hi
>>
>> Am 28.02.21 um 17:10 schrieb Pavel Turinský:
>>> The checks were removed in commit d693def4fd1c ("drm: Remove obsolete GEM
>>> and PRIME callbacks from struct drm_driver") and can lead to following
>>> kernel oops:
>> Thanks for reporting. All drivers are supposed to set the funcs pointer
>> in their GEM objects. This looks like a radeon bug. Adding the AMD devs.
> Looks like we're setting obj->funcs only in radeon_gem_object_create,
> but should set it in radeon_bo_create instead so it catches internal
> functions too. I think this was missed in
>
> commit ce77038fdae385f947757a37573d90f2e83f0271
> Author: Gerd Hoffmann <kraxel@redhat.com>
> Date:   Mon Aug 5 16:01:06 2019 +0200
>
>     drm/radeon: use embedded gem object

Maybe the same problem we had for amdgpu that the function pointer 
wasn't set for imported DMA-bufs?

Regards,
Christian.

>
> Adding Gerd.
> -Daniel
>
>> Best regards
>> Thomas
>>
>>> [  139.449098] BUG: kernel NULL pointer dereference, address: 0000000000000008
>>> [  139.449110] #PF: supervisor read access in kernel mode
>>> [  139.449113] #PF: error_code(0x0000) - not-present page
>>> [  139.449116] PGD 0 P4D 0
>>> [  139.449121] Oops: 0000 [#1] PREEMPT SMP PTI
>>> [  139.449126] CPU: 4 PID: 1181 Comm: Xorg Not tainted 5.11.2LEdoian #2
>>> [  139.449130] Hardware name: Gigabyte Technology Co., Ltd. To be filled by O.E.M./Z77-DS3H, BIOS F4 04/25/2012
>>> [  139.449133] RIP: 0010:drm_gem_handle_create_tail+0xcb/0x190 [drm]
>>> [  139.449185] Code: 00 48 89 ef e8 06 b4 49 f7 45 85 e4 78 77 48 8d 6b 18 4c 89 ee 48 89 ef e8 c2 f5 00 00 89 c2 85 c0 75 3e 48 8b 83 40 01 00 00 <48> 8b 40 0
>>> 8 48 85 c0 74 0f 4c 89 ee 48 89 df e8 71 5d 87 f7 85 c0
>>> [  139.449190] RSP: 0018:ffffbe21c194bd28 EFLAGS: 00010246
>>> [  139.449194] RAX: 0000000000000000 RBX: ffff9da9b3caf078 RCX: 0000000000000000
>>> [  139.449197] RDX: 0000000000000000 RSI: ffffffffc039b893 RDI: 0000000000000000
>>> [  139.449199] RBP: ffff9da9b3caf090 R08: 0000000000000040 R09: ffff9da983b911c0
>>> [  139.449202] R10: ffff9da984749e00 R11: ffff9da9859bfc38 R12: 0000000000000007
>>> [  139.449204] R13: ffff9da9859bfc00 R14: ffff9da9859bfc50 R15: ffff9da9859bfc38
>>> [  139.449207] FS:  00007f6332a56900(0000) GS:ffff9daea7b00000(0000) knlGS:0000000000000000
>>> [  139.449211] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [  139.449214] CR2: 0000000000000008 CR3: 00000001319b8005 CR4: 00000000001706e0
>>> [  139.449217] Call Trace:
>>> [  139.449224]  drm_gem_prime_fd_to_handle+0xff/0x1d0 [drm]
>>> [  139.449274]  ? drm_prime_destroy_file_private+0x20/0x20 [drm]
>>> [  139.449323]  drm_ioctl_kernel+0xac/0xf0 [drm]
>>> [  139.449363]  drm_ioctl+0x20f/0x3b0 [drm]
>>> [  139.449403]  ? drm_prime_destroy_file_private+0x20/0x20 [drm]
>>> [  139.449454]  radeon_drm_ioctl+0x49/0x80 [radeon]
>>> [  139.449500]  __x64_sys_ioctl+0x84/0xc0
>>> [  139.449507]  do_syscall_64+0x33/0x40
>>> [  139.449514]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>> [  139.449522] RIP: 0033:0x7f63330fbe6b
>>> [  139.449526] Code: ff ff ff 85 c0 79 8b 49 c7 c4 ff ff ff ff 5b 5d 4c 89 e0 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f
>>> 0 ff ff 73 01 c3 48 8b 0d d5 af 0c 00 f7 d8 64 89 01 48
>>> [  139.449529] RSP: 002b:00007fff1e9c4438 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
>>> [  139.449534] RAX: ffffffffffffffda RBX: 00007fff1e9c447c RCX: 00007f63330fbe6b
>>> [  139.449537] RDX: 00007fff1e9c447c RSI: 00000000c00c642e RDI: 0000000000000012
>>> [  139.449539] RBP: 00000000c00c642e R08: 00007fff1e9c4520 R09: 00007f63331c7a60
>>> [  139.449542] R10: 00007f6329fb9ab0 R11: 0000000000000246 R12: 000055f69810ad40
>>> [  139.449544] R13: 0000000000000012 R14: 0000000000100000 R15: 00007fff1e9c4c20
>>> [  139.449549] Modules linked in: 8021q garp mrp bridge stp llc nls_iso8859_1 vfat fat fuse btrfs blake2b_generic xor raid6_pq libcrc32c crypto_user tun i2c_de
>>> v it87 hwmon_vid snd_seq snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio sg snd_hda_codec_hdmi virtio_balloon snd_hda_intel virtio_console snd_intel_
>>> dspcfg soundwire_intel virtio_pci soundwire_generic_allocation soundwire_cadence virtio_blk snd_hda_codec intel_rapl_msr btusb intel_rapl_common virtio_net btr
>>> tl net_failover uvcvideo snd_usb_audio snd_hda_core btbcm x86_pkg_temp_thermal failover soundwire_bus btintel intel_powerclamp snd_soc_core coretemp snd_usbmid
>>> i_lib iTCO_wdt videobuf2_vmalloc bluetooth intel_pmc_bxt snd_hwdep kvm_intel videobuf2_memops snd_rawmidi snd_compress videobuf2_v4l2 ac97_bus snd_pcm_dmaengin
>>> e iTCO_vendor_support crct10dif_pclmul at24 crc32_pclmul videobuf2_common snd_seq_device mei_hdcp snd_pcm ghash_clmulni_intel kvm videodev aesni_intel crypto_s
>>> imd snd_timer snd cryptd mc ecdh_generic
>>> [  139.449642]  glue_helper rfkill soundcore joydev mousedev rapl ecc intel_cstate r8169 i2c_i801 intel_uncore atl1c realtek irqbypass mdio_devres mei_me libph
>>> y i2c_smbus mei mac_hid lpc_ich ext4 crc32c_generic crc16 mbcache jbd2 dm_mod ata_generic pata_acpi uas usb_storage sr_mod crc32c_intel serio_raw cdrom xhci_pc
>>> i pata_jmicron xhci_pci_renesas radeon usbhid i915 intel_gtt nouveau mxm_wmi wmi video i2c_algo_bit drm_ttm_helper ttm drm_kms_helper syscopyarea sysfillrect s
>>> ysimgblt fb_sys_fops cec drm agpgart
>>> [  139.449707] CR2: 0000000000000008
>>> [  139.449710] ---[ end trace f5ce5774498d18e1 ]---
>>>
>>> Signed-off-by: Pavel Turinský <ledoian@kam.mff.cuni.cz>
>>> Fixes: d693def4fd1c ("drm: Remove obsolete GEM and PRIME callbacks from struct drm_driver")
>>> Cc: stable@vger.kernel.org
>>> ---
>>>
>>> This is a very symptomatic patch around issue I ran into. I do not know if the
>>> funcs property should ever be NULL. I basically restored all the checks that
>>> were removed in the mentioned commit. Unfortunately, I do not understand drm
>>> nor will I have time to delve into it in forseeable future.
>>>
>>>    drivers/gpu/drm/drm_gem.c   | 20 ++++++++++----------
>>>    drivers/gpu/drm/drm_prime.c |  2 +-
>>>    2 files changed, 11 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
>>> index 92f89cee213e..451f290c737c 100644
>>> --- a/drivers/gpu/drm/drm_gem.c
>>> +++ b/drivers/gpu/drm/drm_gem.c
>>> @@ -249,7 +249,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
>>>        struct drm_file *file_priv = data;
>>>        struct drm_gem_object *obj = ptr;
>>>
>>> -     if (obj->funcs->close)
>>> +     if (obj->funcs && obj->funcs->close)
>>>                obj->funcs->close(obj, file_priv);
>>>
>>>        drm_gem_remove_prime_handles(obj, file_priv);
>>> @@ -401,7 +401,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
>>>        if (ret)
>>>                goto err_remove;
>>>
>>> -     if (obj->funcs->open) {
>>> +     if (obj->funcs && obj->funcs->open) {
>>>                ret = obj->funcs->open(obj, file_priv);
>>>                if (ret)
>>>                        goto err_revoke;
>>> @@ -977,7 +977,7 @@ drm_gem_object_free(struct kref *kref)
>>>        struct drm_gem_object *obj =
>>>                container_of(kref, struct drm_gem_object, refcount);
>>>
>>> -     if (WARN_ON(!obj->funcs->free))
>>> +     if (!obj->funcs || WARN_ON(!obj->funcs->free))
>>>                return;
>>>
>>>        obj->funcs->free(obj);
>>> @@ -1079,7 +1079,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
>>>
>>>        vma->vm_private_data = obj;
>>>
>>> -     if (obj->funcs->mmap) {
>>> +     if (obj->funcs && obj->funcs->mmap) {
>>>                ret = obj->funcs->mmap(obj, vma);
>>>                if (ret) {
>>>                        drm_gem_object_put(obj);
>>> @@ -1087,7 +1087,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
>>>                }
>>>                WARN_ON(!(vma->vm_flags & VM_DONTEXPAND));
>>>        } else {
>>> -             if (obj->funcs->vm_ops)
>>> +             if (obj->funcs && obj->funcs->vm_ops)
>>>                        vma->vm_ops = obj->funcs->vm_ops;
>>>                else {
>>>                        drm_gem_object_put(obj);
>>> @@ -1188,13 +1188,13 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
>>>        drm_printf_indent(p, indent, "imported=%s\n",
>>>                          obj->import_attach ? "yes" : "no");
>>>
>>> -     if (obj->funcs->print_info)
>>> +     if (obj->funcs && obj->funcs->print_info)
>>>                obj->funcs->print_info(p, indent, obj);
>>>    }
>>>
>>>    int drm_gem_pin(struct drm_gem_object *obj)
>>>    {
>>> -     if (obj->funcs->pin)
>>> +     if (obj->funcs && obj->funcs->pin)
>>>                return obj->funcs->pin(obj);
>>>        else
>>>                return 0;
>>> @@ -1202,7 +1202,7 @@ int drm_gem_pin(struct drm_gem_object *obj)
>>>
>>>    void drm_gem_unpin(struct drm_gem_object *obj)
>>>    {
>>> -     if (obj->funcs->unpin)
>>> +     if (obj->funcs && obj->funcs->unpin)
>>>                obj->funcs->unpin(obj);
>>>    }
>>>
>>> @@ -1210,7 +1210,7 @@ int drm_gem_vmap(struct drm_gem_object *obj, struct dma_buf_map *map)
>>>    {
>>>        int ret;
>>>
>>> -     if (!obj->funcs->vmap)
>>> +     if (!obj->funcs || !obj->funcs->vmap)
>>>                return -EOPNOTSUPP;
>>>
>>>        ret = obj->funcs->vmap(obj, map);
>>> @@ -1227,7 +1227,7 @@ void drm_gem_vunmap(struct drm_gem_object *obj, struct dma_buf_map *map)
>>>        if (dma_buf_map_is_null(map))
>>>                return;
>>>
>>> -     if (obj->funcs->vunmap)
>>> +     if (obj->funcs && obj->funcs->vunmap)
>>>                obj->funcs->vunmap(obj, map);
>>>
>>>        /* Always set the mapping to NULL. Callers may rely on this. */
>>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
>>> index 7db55fce35d8..1566dcf417e2 100644
>>> --- a/drivers/gpu/drm/drm_prime.c
>>> +++ b/drivers/gpu/drm/drm_prime.c
>>> @@ -620,7 +620,7 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
>>>        if (WARN_ON(dir == DMA_NONE))
>>>                return ERR_PTR(-EINVAL);
>>>
>>> -     if (WARN_ON(!obj->funcs->get_sg_table))
>>> +     if (!obj->funcs || WARN_ON(!obj->funcs->get_sg_table))
>>>                return ERR_PTR(-ENOSYS);
>>>
>>>        sgt = obj->funcs->get_sg_table(obj);
>>>
>> --
>> 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
>>
>
Alex Deucher March 1, 2021, 5:30 p.m. UTC | #4
+ Andrey

On Mon, Mar 1, 2021 at 5:25 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
>
>
> Am 01.03.21 um 11:04 schrieb Daniel Vetter:
> > On Mon, Mar 1, 2021 at 10:56 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >> (cc'ing amd devs)
> >>
> >> Hi
> >>
> >> Am 28.02.21 um 17:10 schrieb Pavel Turinský:
> >>> The checks were removed in commit d693def4fd1c ("drm: Remove obsolete GEM
> >>> and PRIME callbacks from struct drm_driver") and can lead to following
> >>> kernel oops:
> >> Thanks for reporting. All drivers are supposed to set the funcs pointer
> >> in their GEM objects. This looks like a radeon bug. Adding the AMD devs.
> > Looks like we're setting obj->funcs only in radeon_gem_object_create,
> > but should set it in radeon_bo_create instead so it catches internal
> > functions too. I think this was missed in
> >
> > commit ce77038fdae385f947757a37573d90f2e83f0271
> > Author: Gerd Hoffmann <kraxel@redhat.com>
> > Date:   Mon Aug 5 16:01:06 2019 +0200
> >
> >     drm/radeon: use embedded gem object
>
> Maybe the same problem we had for amdgpu that the function pointer
> wasn't set for imported DMA-bufs?

Perhaps radeon needs some variant of this patch?

commit f8aab60422c371425365d386dfd51e0c6c5b1041
Author: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
Date:   Tue Dec 8 15:16:15 2020 -0500

    drm/amdgpu: Initialise drm_gem_object_funcs for imported BOs

    For BOs imported from outside of amdgpu, setting of amdgpu_gem_object_funcs
    was missing in amdgpu_dma_buf_create_obj. Fix by refactoring BO creation
    and amdgpu_gem_object_funcs setting into single function called
    from both code paths.

    Fixes: d693def4fd1c ("drm: Remove obsolete GEM and PRIME callbacks
from struct drm_driver")

    v2: Use use amdgpu_gem_object_create() directly
    v3: fix warning

    Reviewed-by: Christian König <christian.koenig@amd.com>
    Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
    Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
    Signed-off-by: Alex Deucher <alexander.deucher@amd.com>


>
> Regards,
> Christian.
>
> >
> > Adding Gerd.
> > -Daniel
> >
> >> Best regards
> >> Thomas
> >>
> >>> [  139.449098] BUG: kernel NULL pointer dereference, address: 0000000000000008
> >>> [  139.449110] #PF: supervisor read access in kernel mode
> >>> [  139.449113] #PF: error_code(0x0000) - not-present page
> >>> [  139.449116] PGD 0 P4D 0
> >>> [  139.449121] Oops: 0000 [#1] PREEMPT SMP PTI
> >>> [  139.449126] CPU: 4 PID: 1181 Comm: Xorg Not tainted 5.11.2LEdoian #2
> >>> [  139.449130] Hardware name: Gigabyte Technology Co., Ltd. To be filled by O.E.M./Z77-DS3H, BIOS F4 04/25/2012
> >>> [  139.449133] RIP: 0010:drm_gem_handle_create_tail+0xcb/0x190 [drm]
> >>> [  139.449185] Code: 00 48 89 ef e8 06 b4 49 f7 45 85 e4 78 77 48 8d 6b 18 4c 89 ee 48 89 ef e8 c2 f5 00 00 89 c2 85 c0 75 3e 48 8b 83 40 01 00 00 <48> 8b 40 0
> >>> 8 48 85 c0 74 0f 4c 89 ee 48 89 df e8 71 5d 87 f7 85 c0
> >>> [  139.449190] RSP: 0018:ffffbe21c194bd28 EFLAGS: 00010246
> >>> [  139.449194] RAX: 0000000000000000 RBX: ffff9da9b3caf078 RCX: 0000000000000000
> >>> [  139.449197] RDX: 0000000000000000 RSI: ffffffffc039b893 RDI: 0000000000000000
> >>> [  139.449199] RBP: ffff9da9b3caf090 R08: 0000000000000040 R09: ffff9da983b911c0
> >>> [  139.449202] R10: ffff9da984749e00 R11: ffff9da9859bfc38 R12: 0000000000000007
> >>> [  139.449204] R13: ffff9da9859bfc00 R14: ffff9da9859bfc50 R15: ffff9da9859bfc38
> >>> [  139.449207] FS:  00007f6332a56900(0000) GS:ffff9daea7b00000(0000) knlGS:0000000000000000
> >>> [  139.449211] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >>> [  139.449214] CR2: 0000000000000008 CR3: 00000001319b8005 CR4: 00000000001706e0
> >>> [  139.449217] Call Trace:
> >>> [  139.449224]  drm_gem_prime_fd_to_handle+0xff/0x1d0 [drm]
> >>> [  139.449274]  ? drm_prime_destroy_file_private+0x20/0x20 [drm]
> >>> [  139.449323]  drm_ioctl_kernel+0xac/0xf0 [drm]
> >>> [  139.449363]  drm_ioctl+0x20f/0x3b0 [drm]
> >>> [  139.449403]  ? drm_prime_destroy_file_private+0x20/0x20 [drm]
> >>> [  139.449454]  radeon_drm_ioctl+0x49/0x80 [radeon]
> >>> [  139.449500]  __x64_sys_ioctl+0x84/0xc0
> >>> [  139.449507]  do_syscall_64+0x33/0x40
> >>> [  139.449514]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> >>> [  139.449522] RIP: 0033:0x7f63330fbe6b
> >>> [  139.449526] Code: ff ff ff 85 c0 79 8b 49 c7 c4 ff ff ff ff 5b 5d 4c 89 e0 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f
> >>> 0 ff ff 73 01 c3 48 8b 0d d5 af 0c 00 f7 d8 64 89 01 48
> >>> [  139.449529] RSP: 002b:00007fff1e9c4438 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> >>> [  139.449534] RAX: ffffffffffffffda RBX: 00007fff1e9c447c RCX: 00007f63330fbe6b
> >>> [  139.449537] RDX: 00007fff1e9c447c RSI: 00000000c00c642e RDI: 0000000000000012
> >>> [  139.449539] RBP: 00000000c00c642e R08: 00007fff1e9c4520 R09: 00007f63331c7a60
> >>> [  139.449542] R10: 00007f6329fb9ab0 R11: 0000000000000246 R12: 000055f69810ad40
> >>> [  139.449544] R13: 0000000000000012 R14: 0000000000100000 R15: 00007fff1e9c4c20
> >>> [  139.449549] Modules linked in: 8021q garp mrp bridge stp llc nls_iso8859_1 vfat fat fuse btrfs blake2b_generic xor raid6_pq libcrc32c crypto_user tun i2c_de
> >>> v it87 hwmon_vid snd_seq snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio sg snd_hda_codec_hdmi virtio_balloon snd_hda_intel virtio_console snd_intel_
> >>> dspcfg soundwire_intel virtio_pci soundwire_generic_allocation soundwire_cadence virtio_blk snd_hda_codec intel_rapl_msr btusb intel_rapl_common virtio_net btr
> >>> tl net_failover uvcvideo snd_usb_audio snd_hda_core btbcm x86_pkg_temp_thermal failover soundwire_bus btintel intel_powerclamp snd_soc_core coretemp snd_usbmid
> >>> i_lib iTCO_wdt videobuf2_vmalloc bluetooth intel_pmc_bxt snd_hwdep kvm_intel videobuf2_memops snd_rawmidi snd_compress videobuf2_v4l2 ac97_bus snd_pcm_dmaengin
> >>> e iTCO_vendor_support crct10dif_pclmul at24 crc32_pclmul videobuf2_common snd_seq_device mei_hdcp snd_pcm ghash_clmulni_intel kvm videodev aesni_intel crypto_s
> >>> imd snd_timer snd cryptd mc ecdh_generic
> >>> [  139.449642]  glue_helper rfkill soundcore joydev mousedev rapl ecc intel_cstate r8169 i2c_i801 intel_uncore atl1c realtek irqbypass mdio_devres mei_me libph
> >>> y i2c_smbus mei mac_hid lpc_ich ext4 crc32c_generic crc16 mbcache jbd2 dm_mod ata_generic pata_acpi uas usb_storage sr_mod crc32c_intel serio_raw cdrom xhci_pc
> >>> i pata_jmicron xhci_pci_renesas radeon usbhid i915 intel_gtt nouveau mxm_wmi wmi video i2c_algo_bit drm_ttm_helper ttm drm_kms_helper syscopyarea sysfillrect s
> >>> ysimgblt fb_sys_fops cec drm agpgart
> >>> [  139.449707] CR2: 0000000000000008
> >>> [  139.449710] ---[ end trace f5ce5774498d18e1 ]---
> >>>
> >>> Signed-off-by: Pavel Turinský <ledoian@kam.mff.cuni.cz>
> >>> Fixes: d693def4fd1c ("drm: Remove obsolete GEM and PRIME callbacks from struct drm_driver")
> >>> Cc: stable@vger.kernel.org
> >>> ---
> >>>
> >>> This is a very symptomatic patch around issue I ran into. I do not know if the
> >>> funcs property should ever be NULL. I basically restored all the checks that
> >>> were removed in the mentioned commit. Unfortunately, I do not understand drm
> >>> nor will I have time to delve into it in forseeable future.
> >>>
> >>>    drivers/gpu/drm/drm_gem.c   | 20 ++++++++++----------
> >>>    drivers/gpu/drm/drm_prime.c |  2 +-
> >>>    2 files changed, 11 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> >>> index 92f89cee213e..451f290c737c 100644
> >>> --- a/drivers/gpu/drm/drm_gem.c
> >>> +++ b/drivers/gpu/drm/drm_gem.c
> >>> @@ -249,7 +249,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
> >>>        struct drm_file *file_priv = data;
> >>>        struct drm_gem_object *obj = ptr;
> >>>
> >>> -     if (obj->funcs->close)
> >>> +     if (obj->funcs && obj->funcs->close)
> >>>                obj->funcs->close(obj, file_priv);
> >>>
> >>>        drm_gem_remove_prime_handles(obj, file_priv);
> >>> @@ -401,7 +401,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
> >>>        if (ret)
> >>>                goto err_remove;
> >>>
> >>> -     if (obj->funcs->open) {
> >>> +     if (obj->funcs && obj->funcs->open) {
> >>>                ret = obj->funcs->open(obj, file_priv);
> >>>                if (ret)
> >>>                        goto err_revoke;
> >>> @@ -977,7 +977,7 @@ drm_gem_object_free(struct kref *kref)
> >>>        struct drm_gem_object *obj =
> >>>                container_of(kref, struct drm_gem_object, refcount);
> >>>
> >>> -     if (WARN_ON(!obj->funcs->free))
> >>> +     if (!obj->funcs || WARN_ON(!obj->funcs->free))
> >>>                return;
> >>>
> >>>        obj->funcs->free(obj);
> >>> @@ -1079,7 +1079,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
> >>>
> >>>        vma->vm_private_data = obj;
> >>>
> >>> -     if (obj->funcs->mmap) {
> >>> +     if (obj->funcs && obj->funcs->mmap) {
> >>>                ret = obj->funcs->mmap(obj, vma);
> >>>                if (ret) {
> >>>                        drm_gem_object_put(obj);
> >>> @@ -1087,7 +1087,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
> >>>                }
> >>>                WARN_ON(!(vma->vm_flags & VM_DONTEXPAND));
> >>>        } else {
> >>> -             if (obj->funcs->vm_ops)
> >>> +             if (obj->funcs && obj->funcs->vm_ops)
> >>>                        vma->vm_ops = obj->funcs->vm_ops;
> >>>                else {
> >>>                        drm_gem_object_put(obj);
> >>> @@ -1188,13 +1188,13 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
> >>>        drm_printf_indent(p, indent, "imported=%s\n",
> >>>                          obj->import_attach ? "yes" : "no");
> >>>
> >>> -     if (obj->funcs->print_info)
> >>> +     if (obj->funcs && obj->funcs->print_info)
> >>>                obj->funcs->print_info(p, indent, obj);
> >>>    }
> >>>
> >>>    int drm_gem_pin(struct drm_gem_object *obj)
> >>>    {
> >>> -     if (obj->funcs->pin)
> >>> +     if (obj->funcs && obj->funcs->pin)
> >>>                return obj->funcs->pin(obj);
> >>>        else
> >>>                return 0;
> >>> @@ -1202,7 +1202,7 @@ int drm_gem_pin(struct drm_gem_object *obj)
> >>>
> >>>    void drm_gem_unpin(struct drm_gem_object *obj)
> >>>    {
> >>> -     if (obj->funcs->unpin)
> >>> +     if (obj->funcs && obj->funcs->unpin)
> >>>                obj->funcs->unpin(obj);
> >>>    }
> >>>
> >>> @@ -1210,7 +1210,7 @@ int drm_gem_vmap(struct drm_gem_object *obj, struct dma_buf_map *map)
> >>>    {
> >>>        int ret;
> >>>
> >>> -     if (!obj->funcs->vmap)
> >>> +     if (!obj->funcs || !obj->funcs->vmap)
> >>>                return -EOPNOTSUPP;
> >>>
> >>>        ret = obj->funcs->vmap(obj, map);
> >>> @@ -1227,7 +1227,7 @@ void drm_gem_vunmap(struct drm_gem_object *obj, struct dma_buf_map *map)
> >>>        if (dma_buf_map_is_null(map))
> >>>                return;
> >>>
> >>> -     if (obj->funcs->vunmap)
> >>> +     if (obj->funcs && obj->funcs->vunmap)
> >>>                obj->funcs->vunmap(obj, map);
> >>>
> >>>        /* Always set the mapping to NULL. Callers may rely on this. */
> >>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> >>> index 7db55fce35d8..1566dcf417e2 100644
> >>> --- a/drivers/gpu/drm/drm_prime.c
> >>> +++ b/drivers/gpu/drm/drm_prime.c
> >>> @@ -620,7 +620,7 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
> >>>        if (WARN_ON(dir == DMA_NONE))
> >>>                return ERR_PTR(-EINVAL);
> >>>
> >>> -     if (WARN_ON(!obj->funcs->get_sg_table))
> >>> +     if (!obj->funcs || WARN_ON(!obj->funcs->get_sg_table))
> >>>                return ERR_PTR(-ENOSYS);
> >>>
> >>>        sgt = obj->funcs->get_sg_table(obj);
> >>>
> >> --
> >> 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
> >>
> >
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Alex Deucher March 8, 2021, 7:20 p.m. UTC | #5
On Mon, Mar 1, 2021 at 5:25 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
>
>
> Am 01.03.21 um 11:04 schrieb Daniel Vetter:
> > On Mon, Mar 1, 2021 at 10:56 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> >> (cc'ing amd devs)
> >>
> >> Hi
> >>
> >> Am 28.02.21 um 17:10 schrieb Pavel Turinský:
> >>> The checks were removed in commit d693def4fd1c ("drm: Remove obsolete GEM
> >>> and PRIME callbacks from struct drm_driver") and can lead to following
> >>> kernel oops:
> >> Thanks for reporting. All drivers are supposed to set the funcs pointer
> >> in their GEM objects. This looks like a radeon bug. Adding the AMD devs.
> > Looks like we're setting obj->funcs only in radeon_gem_object_create,
> > but should set it in radeon_bo_create instead so it catches internal
> > functions too. I think this was missed in
> >
> > commit ce77038fdae385f947757a37573d90f2e83f0271
> > Author: Gerd Hoffmann <kraxel@redhat.com>
> > Date:   Mon Aug 5 16:01:06 2019 +0200
> >
> >     drm/radeon: use embedded gem object
>
> Maybe the same problem we had for amdgpu that the function pointer
> wasn't set for imported DMA-bufs?

Should be fixed here:
https://patchwork.freedesktop.org/patch/423250/

Alex

>
> Regards,
> Christian.
>
> >
> > Adding Gerd.
> > -Daniel
> >
> >> Best regards
> >> Thomas
> >>
> >>> [  139.449098] BUG: kernel NULL pointer dereference, address: 0000000000000008
> >>> [  139.449110] #PF: supervisor read access in kernel mode
> >>> [  139.449113] #PF: error_code(0x0000) - not-present page
> >>> [  139.449116] PGD 0 P4D 0
> >>> [  139.449121] Oops: 0000 [#1] PREEMPT SMP PTI
> >>> [  139.449126] CPU: 4 PID: 1181 Comm: Xorg Not tainted 5.11.2LEdoian #2
> >>> [  139.449130] Hardware name: Gigabyte Technology Co., Ltd. To be filled by O.E.M./Z77-DS3H, BIOS F4 04/25/2012
> >>> [  139.449133] RIP: 0010:drm_gem_handle_create_tail+0xcb/0x190 [drm]
> >>> [  139.449185] Code: 00 48 89 ef e8 06 b4 49 f7 45 85 e4 78 77 48 8d 6b 18 4c 89 ee 48 89 ef e8 c2 f5 00 00 89 c2 85 c0 75 3e 48 8b 83 40 01 00 00 <48> 8b 40 0
> >>> 8 48 85 c0 74 0f 4c 89 ee 48 89 df e8 71 5d 87 f7 85 c0
> >>> [  139.449190] RSP: 0018:ffffbe21c194bd28 EFLAGS: 00010246
> >>> [  139.449194] RAX: 0000000000000000 RBX: ffff9da9b3caf078 RCX: 0000000000000000
> >>> [  139.449197] RDX: 0000000000000000 RSI: ffffffffc039b893 RDI: 0000000000000000
> >>> [  139.449199] RBP: ffff9da9b3caf090 R08: 0000000000000040 R09: ffff9da983b911c0
> >>> [  139.449202] R10: ffff9da984749e00 R11: ffff9da9859bfc38 R12: 0000000000000007
> >>> [  139.449204] R13: ffff9da9859bfc00 R14: ffff9da9859bfc50 R15: ffff9da9859bfc38
> >>> [  139.449207] FS:  00007f6332a56900(0000) GS:ffff9daea7b00000(0000) knlGS:0000000000000000
> >>> [  139.449211] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >>> [  139.449214] CR2: 0000000000000008 CR3: 00000001319b8005 CR4: 00000000001706e0
> >>> [  139.449217] Call Trace:
> >>> [  139.449224]  drm_gem_prime_fd_to_handle+0xff/0x1d0 [drm]
> >>> [  139.449274]  ? drm_prime_destroy_file_private+0x20/0x20 [drm]
> >>> [  139.449323]  drm_ioctl_kernel+0xac/0xf0 [drm]
> >>> [  139.449363]  drm_ioctl+0x20f/0x3b0 [drm]
> >>> [  139.449403]  ? drm_prime_destroy_file_private+0x20/0x20 [drm]
> >>> [  139.449454]  radeon_drm_ioctl+0x49/0x80 [radeon]
> >>> [  139.449500]  __x64_sys_ioctl+0x84/0xc0
> >>> [  139.449507]  do_syscall_64+0x33/0x40
> >>> [  139.449514]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> >>> [  139.449522] RIP: 0033:0x7f63330fbe6b
> >>> [  139.449526] Code: ff ff ff 85 c0 79 8b 49 c7 c4 ff ff ff ff 5b 5d 4c 89 e0 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e fa b8 10 00 00 00 0f 05 <48> 3d 01 f
> >>> 0 ff ff 73 01 c3 48 8b 0d d5 af 0c 00 f7 d8 64 89 01 48
> >>> [  139.449529] RSP: 002b:00007fff1e9c4438 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> >>> [  139.449534] RAX: ffffffffffffffda RBX: 00007fff1e9c447c RCX: 00007f63330fbe6b
> >>> [  139.449537] RDX: 00007fff1e9c447c RSI: 00000000c00c642e RDI: 0000000000000012
> >>> [  139.449539] RBP: 00000000c00c642e R08: 00007fff1e9c4520 R09: 00007f63331c7a60
> >>> [  139.449542] R10: 00007f6329fb9ab0 R11: 0000000000000246 R12: 000055f69810ad40
> >>> [  139.449544] R13: 0000000000000012 R14: 0000000000100000 R15: 00007fff1e9c4c20
> >>> [  139.449549] Modules linked in: 8021q garp mrp bridge stp llc nls_iso8859_1 vfat fat fuse btrfs blake2b_generic xor raid6_pq libcrc32c crypto_user tun i2c_de
> >>> v it87 hwmon_vid snd_seq snd_hda_codec_realtek snd_hda_codec_generic ledtrig_audio sg snd_hda_codec_hdmi virtio_balloon snd_hda_intel virtio_console snd_intel_
> >>> dspcfg soundwire_intel virtio_pci soundwire_generic_allocation soundwire_cadence virtio_blk snd_hda_codec intel_rapl_msr btusb intel_rapl_common virtio_net btr
> >>> tl net_failover uvcvideo snd_usb_audio snd_hda_core btbcm x86_pkg_temp_thermal failover soundwire_bus btintel intel_powerclamp snd_soc_core coretemp snd_usbmid
> >>> i_lib iTCO_wdt videobuf2_vmalloc bluetooth intel_pmc_bxt snd_hwdep kvm_intel videobuf2_memops snd_rawmidi snd_compress videobuf2_v4l2 ac97_bus snd_pcm_dmaengin
> >>> e iTCO_vendor_support crct10dif_pclmul at24 crc32_pclmul videobuf2_common snd_seq_device mei_hdcp snd_pcm ghash_clmulni_intel kvm videodev aesni_intel crypto_s
> >>> imd snd_timer snd cryptd mc ecdh_generic
> >>> [  139.449642]  glue_helper rfkill soundcore joydev mousedev rapl ecc intel_cstate r8169 i2c_i801 intel_uncore atl1c realtek irqbypass mdio_devres mei_me libph
> >>> y i2c_smbus mei mac_hid lpc_ich ext4 crc32c_generic crc16 mbcache jbd2 dm_mod ata_generic pata_acpi uas usb_storage sr_mod crc32c_intel serio_raw cdrom xhci_pc
> >>> i pata_jmicron xhci_pci_renesas radeon usbhid i915 intel_gtt nouveau mxm_wmi wmi video i2c_algo_bit drm_ttm_helper ttm drm_kms_helper syscopyarea sysfillrect s
> >>> ysimgblt fb_sys_fops cec drm agpgart
> >>> [  139.449707] CR2: 0000000000000008
> >>> [  139.449710] ---[ end trace f5ce5774498d18e1 ]---
> >>>
> >>> Signed-off-by: Pavel Turinský <ledoian@kam.mff.cuni.cz>
> >>> Fixes: d693def4fd1c ("drm: Remove obsolete GEM and PRIME callbacks from struct drm_driver")
> >>> Cc: stable@vger.kernel.org
> >>> ---
> >>>
> >>> This is a very symptomatic patch around issue I ran into. I do not know if the
> >>> funcs property should ever be NULL. I basically restored all the checks that
> >>> were removed in the mentioned commit. Unfortunately, I do not understand drm
> >>> nor will I have time to delve into it in forseeable future.
> >>>
> >>>    drivers/gpu/drm/drm_gem.c   | 20 ++++++++++----------
> >>>    drivers/gpu/drm/drm_prime.c |  2 +-
> >>>    2 files changed, 11 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> >>> index 92f89cee213e..451f290c737c 100644
> >>> --- a/drivers/gpu/drm/drm_gem.c
> >>> +++ b/drivers/gpu/drm/drm_gem.c
> >>> @@ -249,7 +249,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
> >>>        struct drm_file *file_priv = data;
> >>>        struct drm_gem_object *obj = ptr;
> >>>
> >>> -     if (obj->funcs->close)
> >>> +     if (obj->funcs && obj->funcs->close)
> >>>                obj->funcs->close(obj, file_priv);
> >>>
> >>>        drm_gem_remove_prime_handles(obj, file_priv);
> >>> @@ -401,7 +401,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
> >>>        if (ret)
> >>>                goto err_remove;
> >>>
> >>> -     if (obj->funcs->open) {
> >>> +     if (obj->funcs && obj->funcs->open) {
> >>>                ret = obj->funcs->open(obj, file_priv);
> >>>                if (ret)
> >>>                        goto err_revoke;
> >>> @@ -977,7 +977,7 @@ drm_gem_object_free(struct kref *kref)
> >>>        struct drm_gem_object *obj =
> >>>                container_of(kref, struct drm_gem_object, refcount);
> >>>
> >>> -     if (WARN_ON(!obj->funcs->free))
> >>> +     if (!obj->funcs || WARN_ON(!obj->funcs->free))
> >>>                return;
> >>>
> >>>        obj->funcs->free(obj);
> >>> @@ -1079,7 +1079,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
> >>>
> >>>        vma->vm_private_data = obj;
> >>>
> >>> -     if (obj->funcs->mmap) {
> >>> +     if (obj->funcs && obj->funcs->mmap) {
> >>>                ret = obj->funcs->mmap(obj, vma);
> >>>                if (ret) {
> >>>                        drm_gem_object_put(obj);
> >>> @@ -1087,7 +1087,7 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
> >>>                }
> >>>                WARN_ON(!(vma->vm_flags & VM_DONTEXPAND));
> >>>        } else {
> >>> -             if (obj->funcs->vm_ops)
> >>> +             if (obj->funcs && obj->funcs->vm_ops)
> >>>                        vma->vm_ops = obj->funcs->vm_ops;
> >>>                else {
> >>>                        drm_gem_object_put(obj);
> >>> @@ -1188,13 +1188,13 @@ void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
> >>>        drm_printf_indent(p, indent, "imported=%s\n",
> >>>                          obj->import_attach ? "yes" : "no");
> >>>
> >>> -     if (obj->funcs->print_info)
> >>> +     if (obj->funcs && obj->funcs->print_info)
> >>>                obj->funcs->print_info(p, indent, obj);
> >>>    }
> >>>
> >>>    int drm_gem_pin(struct drm_gem_object *obj)
> >>>    {
> >>> -     if (obj->funcs->pin)
> >>> +     if (obj->funcs && obj->funcs->pin)
> >>>                return obj->funcs->pin(obj);
> >>>        else
> >>>                return 0;
> >>> @@ -1202,7 +1202,7 @@ int drm_gem_pin(struct drm_gem_object *obj)
> >>>
> >>>    void drm_gem_unpin(struct drm_gem_object *obj)
> >>>    {
> >>> -     if (obj->funcs->unpin)
> >>> +     if (obj->funcs && obj->funcs->unpin)
> >>>                obj->funcs->unpin(obj);
> >>>    }
> >>>
> >>> @@ -1210,7 +1210,7 @@ int drm_gem_vmap(struct drm_gem_object *obj, struct dma_buf_map *map)
> >>>    {
> >>>        int ret;
> >>>
> >>> -     if (!obj->funcs->vmap)
> >>> +     if (!obj->funcs || !obj->funcs->vmap)
> >>>                return -EOPNOTSUPP;
> >>>
> >>>        ret = obj->funcs->vmap(obj, map);
> >>> @@ -1227,7 +1227,7 @@ void drm_gem_vunmap(struct drm_gem_object *obj, struct dma_buf_map *map)
> >>>        if (dma_buf_map_is_null(map))
> >>>                return;
> >>>
> >>> -     if (obj->funcs->vunmap)
> >>> +     if (obj->funcs && obj->funcs->vunmap)
> >>>                obj->funcs->vunmap(obj, map);
> >>>
> >>>        /* Always set the mapping to NULL. Callers may rely on this. */
> >>> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> >>> index 7db55fce35d8..1566dcf417e2 100644
> >>> --- a/drivers/gpu/drm/drm_prime.c
> >>> +++ b/drivers/gpu/drm/drm_prime.c
> >>> @@ -620,7 +620,7 @@ struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
> >>>        if (WARN_ON(dir == DMA_NONE))
> >>>                return ERR_PTR(-EINVAL);
> >>>
> >>> -     if (WARN_ON(!obj->funcs->get_sg_table))
> >>> +     if (!obj->funcs || WARN_ON(!obj->funcs->get_sg_table))
> >>>                return ERR_PTR(-ENOSYS);
> >>>
> >>>        sgt = obj->funcs->get_sg_table(obj);
> >>>
> >> --
> >> 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
> >>
> >
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Pavel Turinský March 10, 2021, 1:02 p.m. UTC | #6
On 08. 03. 21 20:20, Alex Deucher wrote:
> Should be fixed here:
> https://patchwork.freedesktop.org/patch/423250/
> 
> Alex

Thanks, it works for me! I tried only the first patch from the series,
the rest didn't look relevant to me. I can try that too.

I didn't test it thoroughly (I don't know how), but the issue with being
unable to start X.org is gone now and the kernel doesn't complain.

Pavel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 92f89cee213e..451f290c737c 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -249,7 +249,7 @@  drm_gem_object_release_handle(int id, void *ptr, void *data)
 	struct drm_file *file_priv = data;
 	struct drm_gem_object *obj = ptr;
 
-	if (obj->funcs->close)
+	if (obj->funcs && obj->funcs->close)
 		obj->funcs->close(obj, file_priv);
 
 	drm_gem_remove_prime_handles(obj, file_priv);
@@ -401,7 +401,7 @@  drm_gem_handle_create_tail(struct drm_file *file_priv,
 	if (ret)
 		goto err_remove;
 
-	if (obj->funcs->open) {
+	if (obj->funcs && obj->funcs->open) {
 		ret = obj->funcs->open(obj, file_priv);
 		if (ret)
 			goto err_revoke;
@@ -977,7 +977,7 @@  drm_gem_object_free(struct kref *kref)
 	struct drm_gem_object *obj =
 		container_of(kref, struct drm_gem_object, refcount);
 
-	if (WARN_ON(!obj->funcs->free))
+	if (!obj->funcs || WARN_ON(!obj->funcs->free))
 		return;
 
 	obj->funcs->free(obj);
@@ -1079,7 +1079,7 @@  int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
 
 	vma->vm_private_data = obj;
 
-	if (obj->funcs->mmap) {
+	if (obj->funcs && obj->funcs->mmap) {
 		ret = obj->funcs->mmap(obj, vma);
 		if (ret) {
 			drm_gem_object_put(obj);
@@ -1087,7 +1087,7 @@  int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
 		}
 		WARN_ON(!(vma->vm_flags & VM_DONTEXPAND));
 	} else {
-		if (obj->funcs->vm_ops)
+		if (obj->funcs && obj->funcs->vm_ops)
 			vma->vm_ops = obj->funcs->vm_ops;
 		else {
 			drm_gem_object_put(obj);
@@ -1188,13 +1188,13 @@  void drm_gem_print_info(struct drm_printer *p, unsigned int indent,
 	drm_printf_indent(p, indent, "imported=%s\n",
 			  obj->import_attach ? "yes" : "no");
 
-	if (obj->funcs->print_info)
+	if (obj->funcs && obj->funcs->print_info)
 		obj->funcs->print_info(p, indent, obj);
 }
 
 int drm_gem_pin(struct drm_gem_object *obj)
 {
-	if (obj->funcs->pin)
+	if (obj->funcs && obj->funcs->pin)
 		return obj->funcs->pin(obj);
 	else
 		return 0;
@@ -1202,7 +1202,7 @@  int drm_gem_pin(struct drm_gem_object *obj)
 
 void drm_gem_unpin(struct drm_gem_object *obj)
 {
-	if (obj->funcs->unpin)
+	if (obj->funcs && obj->funcs->unpin)
 		obj->funcs->unpin(obj);
 }
 
@@ -1210,7 +1210,7 @@  int drm_gem_vmap(struct drm_gem_object *obj, struct dma_buf_map *map)
 {
 	int ret;
 
-	if (!obj->funcs->vmap)
+	if (!obj->funcs || !obj->funcs->vmap)
 		return -EOPNOTSUPP;
 
 	ret = obj->funcs->vmap(obj, map);
@@ -1227,7 +1227,7 @@  void drm_gem_vunmap(struct drm_gem_object *obj, struct dma_buf_map *map)
 	if (dma_buf_map_is_null(map))
 		return;
 
-	if (obj->funcs->vunmap)
+	if (obj->funcs && obj->funcs->vunmap)
 		obj->funcs->vunmap(obj, map);
 
 	/* Always set the mapping to NULL. Callers may rely on this. */
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 7db55fce35d8..1566dcf417e2 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -620,7 +620,7 @@  struct sg_table *drm_gem_map_dma_buf(struct dma_buf_attachment *attach,
 	if (WARN_ON(dir == DMA_NONE))
 		return ERR_PTR(-EINVAL);
 
-	if (WARN_ON(!obj->funcs->get_sg_table))
+	if (!obj->funcs || WARN_ON(!obj->funcs->get_sg_table))
 		return ERR_PTR(-ENOSYS);
 
 	sgt = obj->funcs->get_sg_table(obj);