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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <B9CB6BD4-A013-41AD-8094-C0B45A8201BF@live.com>
Date: Thu, 20 Feb 2025 10:11:58 +0000
From: Aditya Garg <gargaditya08@...e.com>
To: Thomas Zimmermann <tzimmermann@...e.de>
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


> 
> 
>> + 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;
 	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);
+
+	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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ