| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <be52d165-eea7-4e93-aa12-23490d6170cc@suse.de>
Date: Thu, 20 Feb 2025 11:15:56 +0100
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Aditya Garg <gargaditya08@...e.com>
Cc: "maarten.lankhorst@...ux.intel.com" <maarten.lankhorst@...ux.intel.com>,
"mripard@...nel.org" <mripard@...nel.org>,
"airlied@...il.com" <airlied@...il.com>, "simona@...ll.ch"
<simona@...ll.ch>, Kerem Karabay <kekrby@...il.com>,
Atharva Tiwari <evepolonium@...il.com>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH 2/2] drm/tiny: add driver for Apple Touch Bars in x86 Macs
Hi
Am 20.02.25 um 11:11 schrieb Aditya Garg:
> Hi
>
>
>>
>>> + ret = drm_dev_register(drm, 0);
>>> + if (ret)
>>> + return dev_err_probe(dev, ret, "Failed to register DRM device\n");
>> This call does not belong to the mode-setting pipeline and belongs into appletbdrm_probe().
>>
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int appletbdrm_probe(struct usb_interface *intf,
>>> + const struct usb_device_id *id)
>>> +{
>>> + struct usb_endpoint_descriptor *bulk_in, *bulk_out;
>>> + struct device *dev = &intf->dev;
>>> + struct appletbdrm_device *adev;
>>> + int ret;
>>> +
>>> + ret = usb_find_common_endpoints(intf->cur_altsetting, &bulk_in, &bulk_out, NULL, NULL);
>>> + if (ret)
>>> + return dev_err_probe(dev, ret, "Failed to find bulk endpoints\n");
>>> +
>>> + adev = devm_drm_dev_alloc(dev, &appletbdrm_drm_driver, struct appletbdrm_device, drm);
>>> + if (IS_ERR(adev))
>>> + return PTR_ERR(adev);
>>> +
>>> + adev->dev = dev;
>>> + adev->in_ep = bulk_in->bEndpointAddress;
>>> + adev->out_ep = bulk_out->bEndpointAddress;
>>> +
>>> + usb_set_intfdata(intf, adev);
>> Rather set the DRM device here and fetch it in the callbacks below. At some point, we might be able to share some of those helpers among drivers.
>>
> FWIW
>
> Moving register drm device here results in these errors in journalctl:
>
> Feb 20 15:02:46 MacBook kernel: appletbdrm: loading out-of-tree module taints kernel.
> Feb 20 15:02:46 MacBook kernel: appletbdrm: module verification failed: signature and/or required key missing - tainting kernel
> Feb 20 15:02:46 MacBook kernel: BUG: kernel NULL pointer dereference, address: 0000000000000030
> Feb 20 15:02:46 MacBook kernel: #PF: supervisor read access in kernel mode
> Feb 20 15:02:46 MacBook kernel: #PF: error_code(0x0000) - not-present page
> Feb 20 15:02:46 MacBook kernel: PGD 0 P4D 0
> Feb 20 15:02:46 MacBook kernel: Oops: Oops: 0000 [#1] PREEMPT SMP PTI
> Feb 20 15:02:46 MacBook kernel: CPU: 10 UID: 0 PID: 3530 Comm: modprobe Tainted: G C OE 6.13.3-1-t2-noble #1
> Feb 20 15:02:46 MacBook kernel: Tainted: [C]=CRAP, [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
> Feb 20 15:02:46 MacBook kernel: Hardware name: Apple Inc. MacBookPro16,1/Mac-E1008331FDC96864, BIOS 2069.80.3.0.0 (iBridge: 22.16.13051.0.0,0) 01/07/2025
> Feb 20 15:02:46 MacBook kernel: RIP: 0010:drm_dev_register+0x1c/0x290
> Feb 20 15:02:46 MacBook kernel: Code: 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44 00 00 55 48 89 e5 41 57 41 56 41 55 49 89 f5 41 54 53 48 89 fb 48 83 ec 08 <4c> 8b 77 30 49 83 3e 00 0f 84 09 02 00 00 48 83 7b 20 00 0f 84 0e
> Feb 20 15:02:46 MacBook kernel: RSP: 0018:ffffbf4344cbb670 EFLAGS: 00010282
> Feb 20 15:02:46 MacBook kernel: RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
> Feb 20 15:02:46 MacBook kernel: RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> Feb 20 15:02:46 MacBook kernel: RBP: ffffbf4344cbb6a0 R08: 0000000000000000 R09: 0000000000000000
> Feb 20 15:02:46 MacBook kernel: R10: 0000000000000000 R11: 0000000000000000 R12: ffff992a8f114020
> Feb 20 15:02:46 MacBook kernel: R13: 0000000000000000 R14: ffff992a8f115ad8 R15: ffff992a8f114000
> Feb 20 15:02:46 MacBook kernel: FS: 000073572877c080(0000) GS:ffff992deed00000(0000) knlGS:0000000000000000
> Feb 20 15:02:46 MacBook kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> Feb 20 15:02:46 MacBook kernel: CR2: 0000000000000030 CR3: 000000011dd12003 CR4: 00000000003726f0
> Feb 20 15:02:46 MacBook kernel: Call Trace:
> Feb 20 15:02:46 MacBook kernel: <TASK>
> Feb 20 15:02:46 MacBook kernel: ? show_regs+0x6c/0x80
> Feb 20 15:02:46 MacBook kernel: ? __die+0x24/0x80
> Feb 20 15:02:46 MacBook kernel: ? page_fault_oops+0x175/0x5d0
> Feb 20 15:02:46 MacBook kernel: ? do_user_addr_fault+0x4b2/0x870
> Feb 20 15:02:46 MacBook kernel: ? exc_page_fault+0x85/0x1c0
> Feb 20 15:02:46 MacBook kernel: ? asm_exc_page_fault+0x27/0x30
> Feb 20 15:02:46 MacBook kernel: ? drm_dev_register+0x1c/0x290
> Feb 20 15:02:46 MacBook kernel: appletbdrm_probe+0x4eb/0x5f0 [appletbdrm]
> Feb 20 15:02:46 MacBook kernel: usb_probe_interface+0x168/0x3d0
> Feb 20 15:02:46 MacBook kernel: really_probe+0xee/0x3c0
> Feb 20 15:02:46 MacBook kernel: __driver_probe_device+0x8c/0x180
> Feb 20 15:02:46 MacBook kernel: driver_probe_device+0x24/0xd0
> Feb 20 15:02:46 MacBook kernel: __driver_attach+0x10b/0x210
> Feb 20 15:02:46 MacBook kernel: ? __pfx___driver_attach+0x10/0x10
> Feb 20 15:02:46 MacBook kernel: bus_for_each_dev+0x8a/0xf0
> Feb 20 15:02:46 MacBook kernel: driver_attach+0x1e/0x30
> Feb 20 15:02:46 MacBook kernel: bus_add_driver+0x14e/0x290
> Feb 20 15:02:46 MacBook kernel: driver_register+0x5e/0x130
> Feb 20 15:02:46 MacBook kernel: usb_register_driver+0x87/0x170
> Feb 20 15:02:46 MacBook kernel: ? __pfx_appletbdrm_usb_driver_init+0x10/0x10 [appletbdrm]
> Feb 20 15:02:46 MacBook kernel: appletbdrm_usb_driver_init+0x23/0xff0 [appletbdrm]
> Feb 20 15:02:46 MacBook kernel: do_one_initcall+0x5b/0x340
> Feb 20 15:02:46 MacBook kernel: do_init_module+0x97/0x2a0
> Feb 20 15:02:46 MacBook kernel: load_module+0x2293/0x25c0
> Feb 20 15:02:46 MacBook kernel: init_module_from_file+0x97/0xe0
> Feb 20 15:02:46 MacBook kernel: ? init_module_from_file+0x97/0xe0
> Feb 20 15:02:46 MacBook kernel: idempotent_init_module+0x110/0x300
> Feb 20 15:02:46 MacBook kernel: __x64_sys_finit_module+0x77/0x100
> Feb 20 15:02:46 MacBook kernel: x64_sys_call+0x1f37/0x2650
> Feb 20 15:02:46 MacBook kernel: do_syscall_64+0x7e/0x170
> Feb 20 15:02:46 MacBook kernel: ? ksys_read+0x72/0xf0
> Feb 20 15:02:46 MacBook kernel: ? arch_exit_to_user_mode_prepare.isra.0+0x22/0xd0
> Feb 20 15:02:46 MacBook kernel: ? syscall_exit_to_user_mode+0x38/0x1d0
> Feb 20 15:02:46 MacBook kernel: ? do_syscall_64+0x8a/0x170
> Feb 20 15:02:46 MacBook kernel: ? __do_sys_newfstatat+0x44/0x90
> Feb 20 15:02:46 MacBook kernel: ? ext4_llseek+0xc0/0x120
> Feb 20 15:02:46 MacBook kernel: ? arch_exit_to_user_mode_prepare.isra.0+0x22/0xd0
> Feb 20 15:02:46 MacBook kernel: ? syscall_exit_to_user_mode+0x38/0x1d0
> Feb 20 15:02:46 MacBook kernel: ? do_syscall_64+0x8a/0x170
> Feb 20 15:02:46 MacBook kernel: ? do_syscall_64+0x8a/0x170
> Feb 20 15:02:46 MacBook kernel: ? count_memcg_events.constprop.0+0x2a/0x50
> Feb 20 15:02:46 MacBook kernel: ? handle_mm_fault+0xaf/0x2e0
> Feb 20 15:02:46 MacBook kernel: ? do_user_addr_fault+0x5d5/0x870
> Feb 20 15:02:46 MacBook kernel: ? arch_exit_to_user_mode_prepare.isra.0+0x22/0xd0
> Feb 20 15:02:46 MacBook kernel: ? irqentry_exit_to_user_mode+0x2d/0x1d0
> Feb 20 15:02:46 MacBook kernel: ? irqentry_exit+0x43/0x50
> Feb 20 15:02:46 MacBook kernel: ? exc_page_fault+0x96/0x1c0
> Feb 20 15:02:46 MacBook kernel: entry_SYSCALL_64_after_hwframe+0x76/0x7e
> Feb 20 15:02:46 MacBook kernel: RIP: 0033:0x735727f2725d
> Feb 20 15:02:46 MacBook kernel: Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 8b bb 0d 00 f7 d8 64 89 01 48
> Feb 20 15:02:46 MacBook kernel: RSP: 002b:00007fffd9f88d18 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
> Feb 20 15:02:46 MacBook kernel: RAX: ffffffffffffffda RBX: 000062610c6eb8e0 RCX: 0000735727f2725d
> Feb 20 15:02:46 MacBook kernel: RDX: 0000000000000000 RSI: 00006260e7b3be52 RDI: 0000000000000003
> Feb 20 15:02:46 MacBook kernel: RBP: 00007fffd9f88dd0 R08: 0000000000000040 R09: 00007fffd9f88e50
> Feb 20 15:02:46 MacBook kernel: R10: 0000735728003b20 R11: 0000000000000246 R12: 00006260e7b3be52
> Feb 20 15:02:46 MacBook kernel: R13: 0000000000040000 R14: 000062610c6e4920 R15: 0000000000000000
> Feb 20 15:02:46 MacBook kernel: </TASK>
>
> The following change was done:
>
> @@ -13,6 +13,7 @@
>
> #include <drm/drm_atomic.h>
> #include <drm/drm_atomic_helper.h>
> +#include <drm/drm_client_setup.h>
> #include <drm/drm_crtc.h>
> #include <drm/drm_damage_helper.h>
> #include <drm/drm_drv.h>
> @@ -596,7 +597,6 @@ static int appletbdrm_setup_mode_config(struct appletbdrm_device *adev)
> * as the height is actually the width of the framebuffer and vice
> * versa
> */
> -
> drm->mode_config.min_width = 0;
> drm->mode_config.min_height = 0;
> drm->mode_config.max_width = max(adev->height, DRM_SHADOW_PLANE_MAX_WIDTH);
> @@ -635,10 +635,6 @@ static int appletbdrm_setup_mode_config(struct appletbdrm_device *adev)
>
> drm_mode_config_reset(drm);
>
> - ret = drm_dev_register(drm, 0);
> - if (ret)
> - return dev_err_probe(dev, ret, "Failed to register DRM device\n");
> -
> return 0;
> }
>
> @@ -648,6 +644,7 @@ static int appletbdrm_probe(struct usb_interface *intf,
> struct usb_endpoint_descriptor *bulk_in, *bulk_out;
> struct device *dev = &intf->dev;
> struct appletbdrm_device *adev;
> + struct drm_device *drm;
Because you apparently never initialize this variable.
> int ret;
>
> ret = usb_find_common_endpoints(intf->cur_altsetting, &bulk_in, &bulk_out, NULL, NULL);
> @@ -676,7 +673,17 @@ static int appletbdrm_probe(struct usb_interface *intf,
> if (ret)
> return dev_err_probe(dev, ret, "Failed to clear display\n");
>
> - return appletbdrm_setup_mode_config(adev);
> + ret = appletbdrm_setup_mode_config(adev);
> + if (ret)
> + return ret;
> +
> + ret = drm_dev_register(drm, 0);
> + if (ret)
> + return dev_err_probe(dev, ret, "Failed to register DRM device\n");
> +
> + drm_client_setup(drm, NULL);
You won't need a DRM client on the touch bar. Just clearing the display
should be enough.
Best regards
Thomas
> +
> + return 0;
> }
>
> static void appletbdrm_disconnect(struct usb_interface *intf)
>
>>> +
>>> + ret = appletbdrm_get_information(adev);
>>> + if (ret)
>>> + return dev_err_probe(dev, ret, "Failed to get display information\n");
>>> +
>>> + ret = appletbdrm_signal_readiness(adev);
>>> + if (ret)
>>> + return dev_err_probe(dev, ret, "Failed to signal readiness\n");
>>> +
>>> + ret = appletbdrm_clear_display(adev);
>>> + if (ret)
>>> + return dev_err_probe(dev, ret, "Failed to clear display\n");
>> Clearing the display is not something usually done in probe. But I guess there's no better place. I'd do this as the final call in probe; after registering the device. That way, it acts a bit like an in-kernel DRM client.
>>
>> Best regards
>> Thomas
>>
>>> +
>>> + return appletbdrm_setup_mode_config(adev);
>>> +}
>>> +
>>> +static void appletbdrm_disconnect(struct usb_interface *intf)
>>> +{
>>> + struct appletbdrm_device *adev = usb_get_intfdata(intf);
>>> + struct drm_device *drm = &adev->drm;
>>> +
>>> + drm_dev_unplug(drm);
>>> + drm_atomic_helper_shutdown(drm);
>>> +}
>>> +
>>> +static void appletbdrm_shutdown(struct usb_interface *intf)
>>> +{
>>> + struct appletbdrm_device *adev = usb_get_intfdata(intf);
>>> +
>>> + /*
>>> + * The framebuffer needs to be cleared on shutdown since its content
>>> + * persists across boots
>>> + */
>>> + drm_atomic_helper_shutdown(&adev->drm);
>>> +}
>>> +
>>> +static const struct usb_device_id appletbdrm_usb_id_table[] = {
>>> + { USB_DEVICE_INTERFACE_CLASS(0x05ac, 0x8302, USB_CLASS_AUDIO_VIDEO) },
>>> + {}
>>> +};
>>> +MODULE_DEVICE_TABLE(usb, appletbdrm_usb_id_table);
>>> +
>>> +static struct usb_driver appletbdrm_usb_driver = {
>>> + .name = "appletbdrm",
>>> + .probe = appletbdrm_probe,
>>> + .disconnect = appletbdrm_disconnect,
>>> + .shutdown = appletbdrm_shutdown,
>>> + .id_table = appletbdrm_usb_id_table,
>>> +};
>>> +module_usb_driver(appletbdrm_usb_driver);
>>> +
>>> +MODULE_AUTHOR("Kerem Karabay <kekrby@...il.com>");
>>> +MODULE_DESCRIPTION("Apple Touch Bar DRM Driver");
>>> +MODULE_LICENSE("GPL");
> Regards
> Aditya
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
Powered by blists - more mailing lists