Skip to content

Commit

Permalink
firmware: qcom: scm: Cleanup global '__scm' on probe failures
Browse files Browse the repository at this point in the history
If SCM driver fails the probe, it should not leave global '__scm'
variable assigned, because external users of this driver will assume the
probe finished successfully.  For example TZMEM parts ('__scm->mempool')
are initialized later in the probe, but users of it (__scm_smc_call())
rely on the '__scm' variable.

This fixes theoretical NULL pointer exception, triggered via introducing
probe deferral in SCM driver with call trace:

  qcom_tzmem_alloc+0x70/0x1ac (P)
  qcom_tzmem_alloc+0x64/0x1ac (L)
  qcom_scm_assign_mem+0x78/0x194
  qcom_rmtfs_mem_probe+0x2d4/0x38c
  platform_probe+0x68/0xc8

Fixes: 40289e3 ("firmware: qcom: scm: enable the TZ mem allocator")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Link: https://lore.kernel.org/r/20241209-qcom-scm-missing-barriers-and-all-sort-of-srap-v2-4-9061013c8d92@linaro.org
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
  • Loading branch information
Krzysztof Kozlowski authored and Bjorn Andersson committed Jan 7, 2025
1 parent b628510 commit 1e76b54
Showing 1 changed file with 29 additions and 13 deletions.
42 changes: 29 additions & 13 deletions drivers/firmware/qcom/qcom_scm.c
Original file line number Diff line number Diff line change
Expand Up @@ -2038,13 +2038,17 @@ static int qcom_scm_probe(struct platform_device *pdev)

irq = platform_get_irq_optional(pdev, 0);
if (irq < 0) {
if (irq != -ENXIO)
return irq;
if (irq != -ENXIO) {
ret = irq;
goto err;
}
} else {
ret = devm_request_threaded_irq(__scm->dev, irq, NULL, qcom_scm_irq_handler,
IRQF_ONESHOT, "qcom-scm", __scm);
if (ret < 0)
return dev_err_probe(scm->dev, ret, "Failed to request qcom-scm irq\n");
if (ret < 0) {
dev_err_probe(scm->dev, ret, "Failed to request qcom-scm irq\n");
goto err;
}
}

__get_convention();
Expand All @@ -2063,24 +2067,30 @@ static int qcom_scm_probe(struct platform_device *pdev)
qcom_scm_disable_sdi();

ret = of_reserved_mem_device_init(__scm->dev);
if (ret && ret != -ENODEV)
return dev_err_probe(__scm->dev, ret,
"Failed to setup the reserved memory region for TZ mem\n");
if (ret && ret != -ENODEV) {
dev_err_probe(__scm->dev, ret,
"Failed to setup the reserved memory region for TZ mem\n");
goto err;
}

ret = qcom_tzmem_enable(__scm->dev);
if (ret)
return dev_err_probe(__scm->dev, ret,
"Failed to enable the TrustZone memory allocator\n");
if (ret) {
dev_err_probe(__scm->dev, ret,
"Failed to enable the TrustZone memory allocator\n");
goto err;
}

memset(&pool_config, 0, sizeof(pool_config));
pool_config.initial_size = 0;
pool_config.policy = QCOM_TZMEM_POLICY_ON_DEMAND;
pool_config.max_size = SZ_256K;

__scm->mempool = devm_qcom_tzmem_pool_new(__scm->dev, &pool_config);
if (IS_ERR(__scm->mempool))
return dev_err_probe(__scm->dev, PTR_ERR(__scm->mempool),
"Failed to create the SCM memory pool\n");
if (IS_ERR(__scm->mempool)) {
dev_err_probe(__scm->dev, PTR_ERR(__scm->mempool),
"Failed to create the SCM memory pool\n");
goto err;
}

/*
* Initialize the QSEECOM interface.
Expand All @@ -2096,6 +2106,12 @@ static int qcom_scm_probe(struct platform_device *pdev)
WARN(ret < 0, "failed to initialize qseecom: %d\n", ret);

return 0;

err:
/* Paired with smp_load_acquire() in qcom_scm_is_available(). */
smp_store_release(&__scm, NULL);

return ret;
}

static void qcom_scm_shutdown(struct platform_device *pdev)
Expand Down

0 comments on commit 1e76b54

Please sign in to comment.