[PATCH] platform/x86: dell: dcdbas: Force synchronous probe

[PATCH] platform/x86: dell: dcdbas: Force synchronous probe

[Paul Menzel]

dcdbas registers its platform driver and its own platform device in
dcdbas_init(), and only dcdbas_probe() sets dcdbas_pdev, the device that
dcdbas_smi_alloc() and dcdbas_smi_free() use for the DMA allocation.
Since commit 77089467fc79 (platform/x86/dell: add buffer
allocation/free functions for SMI calls), dell_smbios calls
dcdbas_smi_alloc() from its module init through init_dell_smbios_smm().

That relies on the probe having run when dcdbas_init() returns. With
asynchronous probing, requested with module.async_probe=1,
dcdbas.async_probe=1 or driver_async_probe=dcdbas, the probe is queued
instead, and if dell_smbios is loaded before it runs,
dcdbas_smi_alloc() passes &dcdbas_pdev->dev, with dcdbas_pdev still
NULL, to dma_alloc_coherent():

    BUG: kernel NULL pointer dereference, address: 0000000000000268
    #PF: supervisor read access in kernel mode
    #PF: error_code(0x0000) - not-present page
    PGD 0 P4D 0
    Oops: Oops: 0000 [#1] SMP NOPTI
    CPU: 2 UID: 0 PID: 295 Comm: (udev-worker) Not tainted 7.4.0-rc1-ortiz-bt4 #1 PREEMPT(lazy)
    Hardware name: Dell Inc. XPS 13 9310/0GG9PT, BIOS 3.17.0 07/12/2023
    RIP: 0010:dma_alloc_attrs+0x22/0x220
    [...]
    RDI: 0000000000000010 [...]
    Call Trace:
     <TASK>
     dcdbas_smi_alloc+0x2d/0xc0 [dcdbas]
     init_dell_smbios_smm+0x1e/0x190 [dell_smbios]
     dell_smbios_init+0x10f/0xf10 [dell_smbios]
     do_one_initcall+0x89/0x3d0
     do_init_module+0x6a/0x290
     init_module_from_file+0xc8/0x180
     idempotent_init_module+0x23e/0x360
     __x64_sys_finit_module+0x7f/0x110
     do_syscall_64+0xc1/0x560
     entry_SYSCALL_64_after_hwframe+0x76/0x7e

On a Dell XPS 13 9310 booted with module.async_probe=1, udev loads both
modules early, and this happened in about one of 15 boots.

The probe of the device dcdbas creates itself is part of setting up the
interface the module exports, so force it to be synchronous, like
commit 31e11af34e2c ("serial: 8250: force synchronous probe for the ISA
and PNP drivers") did for a similar race.

Tested in QEMU (x86_64_defconfig + kvm_guest.config, DCDBAS=m,
DELL_SMBIOS=m, DELL_SMBIOS_SMM=y, SMBIOS OEM string "Dell System"): one
SCHED_FIFO process on one vCPU loads dcdbas with async_probe=1 and then
dell_smbios, so the queued probe cannot run in between.

  without this patch: oops in dcdbas_smi_alloc() in the first iteration, 4 of 4 boots
  with this patch:    6 x 200 load/unload iterations, no oops

The two ideas below were discarded:

-   Setting dcdbas_pdev in init: dell_smbios would then use the device
    while it might still be probing.

-   platform_create_bundle(): a failed probe would fail the module load.
    Because of the symbol dependency, dell_smbios would then fail to
    load as well, including its WMI backend.

Fixes: 77089467fc79 ("platform/x86/dell: add buffer allocation/free functions for SMI calls")
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---

Notes:
    Analysis, reproducer and patch drafted with claude-opus-5-5.

 drivers/platform/x86/dell/dcdbas.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/platform/x86/dell/dcdbas.c b/drivers/platform/x86/dell/dcdbas.c
index 678f44252a453..5ce3a3f78980d 100644
--- a/drivers/platform/x86/dell/dcdbas.c
+++ b/drivers/platform/x86/dell/dcdbas.c
@@ -708,6 +708,11 @@ static void dcdbas_remove(struct platform_device *dev)
 static struct platform_driver dcdbas_driver = {
 	.driver		= {
 		.name	= DRIVER_NAME,
+		/*
+		 * dcdbas_probe() sets dcdbas_pdev, which the exported
+		 * dcdbas_smi_alloc() needs as soon as dcdbas_init() returns.
+		 */
+		.probe_type = PROBE_FORCE_SYNCHRONOUS,
 	},
 	.probe		= dcdbas_probe,
 	.remove		= dcdbas_remove,
-- 
2.54.0