crypto: Remove redundant error messages on IRQ request failure
Subsystems (21)
cover crypto: Remove redundant error messages on IRQ request failure
Commit 55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()") added automatic error logging to devm_request_threaded_irq() and devm_request_any_context_irq() via the new devm_request_result() helper, which prints device name, IRQ number, handler functions, and error code on failure.
Since devm_request_irq() is a static inline wrapper around devm_request_threaded_irq(), it also benefits from this automatic logging.
Remove the now-redundant dev_err() and dev_err_probe() calls in crypto drivers that follow these devm_request_*_irq() functions, as the core now provides more detailed diagnostic information on failure.
Pan Chuang (12):
crypto: allwinner - Remove redundant dev_err()
crypto: amlogic - Remove redundant dev_err()
crypto: aspeed - Remove redundant dev_err()
crypto: Remove redundant dev_err()/dev_err_probe()
crypto: caam - Remove redundant dev_err()
crypto: ccree - Remove redundant dev_err()
crypto: sl3516 - Remove redundant dev_err()
crypto: safexcel - Remove redundant dev_err()
crypto: keembay - Remove redundant dev_err()
crypto: octeontx2 - Remove redundant dev_err()
crypto: rockchip - Remove redundant dev_err()
crypto: stm32 - Remove redundant dev_err()
drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c | 4 +--- drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.c | 4 +--- drivers/crypto/amlogic/amlogic-gxl-core.c | 4 +--- drivers/crypto/aspeed/aspeed-acry.c | 4 +--- drivers/crypto/aspeed/aspeed-hace.c | 4 +--- drivers/crypto/atmel-aes.c | 4 +--- drivers/crypto/atmel-sha.c | 4 +--- drivers/crypto/atmel-tdes.c | 4 +--- drivers/crypto/caam/jr.c | 2 -- drivers/crypto/ccree/cc_driver.c | 4 +--- drivers/crypto/gemini/sl3516-ce-core.c | 4 +--- drivers/crypto/img-hash.c | 4 +--- drivers/crypto/inside-secure/safexcel.c | 4 +--- drivers/crypto/intel/keembay/keembay-ocs-aes-core.c | 4 +--- drivers/crypto/intel/keembay/keembay-ocs-ecc.c | 4 +--- drivers/crypto/intel/keembay/keembay-ocs-hcu-core.c | 4 +--- drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c | 5 +---- drivers/crypto/mxs-dcp.c | 8 ++------ drivers/crypto/omap-aes.c | 4 +--- drivers/crypto/omap-des.c | 4 +--- drivers/crypto/omap-sham.c | 5 +---- drivers/crypto/rockchip/rk3288_crypto.c | 4 +--- drivers/crypto/sahara.c | 3 +-- drivers/crypto/stm32/stm32-cryp.c | 4 +--- drivers/crypto/stm32/stm32-hash.c | 4 +--- 25 files changed, 25 insertions(+), 78 deletions(-)
Signature · 1 line
2.34.1
PATCH 1/12 crypto: allwinner - Remove redundant dev_err()
Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"), devm_request_irq() automatically logs detailed error messages on failure. Remove the now-redundant driver-specific dev_err() calls.
--- drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c | 4 +--- drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.c | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-)
drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c +1 -3
@@ -976,10 +976,8 @@ static int sun8i_ce_probe(struct platform_device *pdev) err =devm_request_irq (&pdev->dev, irq, ce_irq_handler,0 ,"sun8i-ce-ns" , ce); -if (err) { -dev_err (ce->dev,"Cannot request CryptoEngine Non-secure IRQ (err=%d)\n" , err); +if (err) goto error_pm; - } err =sun8i_ce_register_algs (ce);if (err)
drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.c +1 -3
@@ -824,10 +824,8 @@ static int sun8i_ss_probe(struct platform_device *pdev) goto error_pm; err =devm_request_irq (&pdev->dev, irq, ss_irq_handler,0 ,"sun8i-ss" , ss); -if (err) { -dev_err (ss->dev,"Cannot request SecuritySystem IRQ (err=%d)\n" , err); +if (err) goto error_irq; - } err =sun8i_ss_register_algs (ss);if (err)
Signature · 1 line
2.34.1
[email protected]
AI reviewRe: [PATCH 01/12] crypto: allwinner - Remove redundant dev_err()
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Uninitialized list dereference (kernel panic) due to ignored TRNG registration failure.
- [High] List corruption and kernel panic due to dangling pointers in global algorithm arrays during failed re-binds. --
--- Patch [1]: [PATCH 01/12] crypto: allwinner - Remove redundant dev_err() --- commit 19d9808368b2fa17e987058da2afc72772963840
Author: Pan Chuang <[email protected]>
Subject: crypto: allwinner - Remove redundant dev_err()
This patch removes redundant driver-specific dev_err() messages after devm_request_irq() calls. Since commit 55b48e23f5c4, devm_request_irq() automatically logs detailed error messages on failure, making these additional prints unnecessary.
Quoted 6 lines
drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c +0
-0@@ -1039,10 +1039,8 @@ static int sun8i_ce_probe(struct platform_device *pdev)
drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c static int sun8i_ce_probe(struct platform_device *pdev) · line 1046
Quoted 10 lines
drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c +1
-3err =devm_request_irq (&pdev->dev, irq, ce_irq_handler,0 ,"sun8i-ce-ns" , ce); -if (err) { -dev_err (ce->dev,"Cannot request CryptoEngine Non-secure IRQ (err=%d)\n" , err); +if (err) goto error_pm; - } err =sun8i_ce_register_algs (ce);if (err)
[Severity: High]
This is a pre-existing issue, but does probe proceed successfully even if hardware RNG registration fails?
A bit further down in sun8i_ce_probe() in drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c, the return value of sun8i_ce_hwrng_register() is ignored:
#ifdef CONFIG_CRYPTO_DEV_SUN8I_CE_TRNG sun8i_ce_hwrng_register(ce);
#endif
If registration fails (e.g., due to a duplicate name via -EEXIST from a mock device, or sysfs unbind/bind races), probe proceeds. Later, upon unbinding or module removal, sun8i_ce_remove() unconditionally calls sun8i_ce_hwrng_unregister() which triggers:
drivers/crypto/allwinner/sun8i-ce/sun8i-ce-trng.c:sun8i_ce_hwrng_unregister() {
...
if (ce->variant->trng == CE_ID_NOTSUPP)
return;
hwrng_unregister(&ce->trng);
...
}Because the device was never successfully added to the core list, its list_head remains zero-initialized (from devm_kzalloc). Calling hwrng_unregister() executes list_del(), which attempts to dereference the NULL prev and next pointers, causing an immediate kernel oops.
Could the return value of sun8i_ce_hwrng_register() be checked and handled appropriately?
[Severity: High]
This is another pre-existing issue, but does sun8i_ce_unregister_algs() leave dangling pointers in the global ce_algs array?
When algorithms are unregistered, the device instance pointers are not cleared:
drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c:sun8i_ce_unregister_algs() {
...
case CRYPTO_ALG_TYPE_SKCIPHER:
dev_info(ce->dev, "Unregister %d %s\n", i,
ce_algs[i].alg.skcipher.base.base.cra_name);
crypto_engine_unregister_skcipher(&ce_algs[i].alg.skcipher);
break;
...
}If a device is successfully bound and then unbound, these global arrays are left with dangling pointers to the freed device structure. On a subsequent probe of the device, if algorithm registration fails partway through (such as if sun8i_ce_register_algs() fails as seen in the diff context above), the error path calls the unregister function.
The loop sees the dangling pointers from the previous successful bind and attempts to call crypto_engine_unregister_skcipher() on them again. This causes a double-unregister of the same algorithm template, resulting in list corruption.
Could ce_algs[i].ce be set to NULL after unregistering each algorithm?
Quoted 6 lines
drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.c +0
-0@@ -869,10 +869,8 @@ static int sun8i_ss_probe(struct platform_device *pdev) goto error_pm;
drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.c static int sun8i_ss_probe(struct platform_device *pdev) · line 876
Quoted 10 lines
drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.c +1
-3err =devm_request_irq (&pdev->dev, irq, ss_irq_handler,0 ,"sun8i-ss" , ss); -if (err) { -dev_err (ss->dev,"Cannot request SecuritySystem IRQ (err=%d)\n" , err); +if (err) goto error_irq; - } err =sun8i_ss_register_algs (ss);if (err)
[Severity: High]
This is a pre-existing issue, but similar to the sun8i-ce driver above, can sun8i_ss_unregister_algs() cause a double-free?
drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.c:sun8i_ss_unregister_algs() {
...
case CRYPTO_ALG_TYPE_SKCIPHER:
dev_info(ss->dev, "Unregister %d %s\n", i,
ss_algs[i].alg.skcipher.base.base.cra_name);
crypto_engine_unregister_skcipher(&ss_algs[i].alg.skcipher);
break;
...
}Failing to clear ss_algs[i].ss to NULL after unregistering the algorithm leaves a dangling pointer for future probes, which can lead to list corruption and a double-free in the crypto core during failed re-binds.
Could ss_algs[i].ss be set to NULL after unregistering each algorithm here as well?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
PATCH 2/12 crypto: amlogic - Remove redundant dev_err()
In reply to Pan ChuangSince commit 55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"), devm_request_irq() automatically logs detailed error messages on failure. Remove the now-redundant driver-specific dev_err() calls.
--- drivers/crypto/amlogic/amlogic-gxl-core.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/crypto/amlogic/amlogic-gxl-core.c +1 -3
@@ -257,10 +257,8 @@ static int meson_crypto_probe(struct platform_device *pdev) err =devm_request_irq (&pdev->dev, mc->irqs[i], meson_irq_handler,0 ,"gxl-crypto" , mc); -if (err <0 ) { -dev_err (mc->dev,"Cannot request IRQ for flow %d\n" , i); +if (err <0 )return err; - } } err =clk_prepare_enable (mc->busclk);
Signature · 1 line
2.34.1
[email protected]
AI reviewRe: [PATCH 02/12] crypto: amlogic - Remove redundant dev_err()
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] IRQs are requested via devm_request_irq before the device clock is enabled and the channel list is allocated.
- [High] The driver modifies a shared static global array of algorithm templates during probe, preventing multiple device instances and corrupting crypto API state.
- [High] DMA mappings are leaked in meson_cipher when scatterlist mapping fails or exceeds the maximum descriptor count.
- [Medium] The IRQ handler unconditionally returns IRQ_HANDLED even for spurious interrupts.
--
--- Patch [2]: [PATCH 02/12] crypto: amlogic - Remove redundant dev_err() --- commit 82c5002d2a0028a21e2688dd2106e4d410487f9e
Author: Pan Chuang <[email protected]>
crypto: amlogic - Remove redundant dev_err()
Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"), devm_request_irq() automatically logs detailed error messages on failure. Remove the now-redundant driver-specific dev_err() calls.
drivers/crypto/amlogic/amlogic-gxl-core.c static int meson_crypto_probe(struct platform_device *pdev) · line 261
Quoted 12 lines
drivers/crypto/amlogic/amlogic-gxl-core.c +1
-2@@ -257,10 +257,8 @@ static int meson_crypto_probe(struct platform_device *pdev) err =devm_request_irq (&pdev->dev, mc->irqs[i], meson_irq_handler,0 ,"gxl-crypto" , mc); -if (err <0 ) { -dev_err (mc->dev,"Cannot request IRQ for flow %d\n" , i); +if (err <0 )return err;
[Severity: High]
This is a pre-existing issue, but does requesting the interrupt at this point risk a crash if an interrupt fires immediately?
If an interrupt fires between devm_request_irq() and the subsequent calls to clk_prepare_enable() or meson_allocate_chanlist(), the handler will run before resources are initialized.
In drivers/crypto/amlogic/amlogic-gxl-core.c:meson_irq_handler():
p = readl(mc->base + ((0x04 + flow) << 2));
if (p) {
writel_relaxed(0xF, mc->base + ((0x4 + flow) << 2));
mc->chanlist[flow].status = 1;The handler performs a readl() on unclocked device registers, which can cause a synchronous external abort or bus hang. It also accesses mc->chanlist without checking if it is allocated, which could cause a null pointer dereference.
[Severity: Medium]
This is a pre-existing issue, but does meson_irq_handler() unconditionally return IRQ_HANDLED even for spurious interrupts?
Looking at drivers/crypto/amlogic/amlogic-gxl-core.c:meson_irq_handler():
dev_err(mc->dev, "%s %d from unknown irq\n", __func__, irq);
return IRQ_HANDLED;If an unhandled interrupt occurs, returning IRQ_HANDLED prevents the core interrupt subsystem's "nobody cared" logic from disabling a stuck interrupt line. Could this result in an interrupt storm locking up a processor if the hardware malfunctions?
drivers/crypto/amlogic/amlogic-gxl-core.c static int meson_crypto_probe(struct platform_device *pdev) · line 264
Quoted 4 lines
drivers/crypto/amlogic/amlogic-gxl-core.c +0
-1- } } err =clk_prepare_enable (mc->busclk);
[Severity: High]
This is a pre-existing issue, but does the driver modify a shared static global array during probe?
Looking at meson_register_algs() which is called shortly after in meson_crypto_probe():
In drivers/crypto/amlogic/amlogic-gxl-core.c:meson_register_algs():
mc_algs[i].mc = mc;
switch (mc_algs[i].type) {
case CRYPTO_ALG_TYPE_SKCIPHER:
err = crypto_engine_register_skcipher(&mc_algs[i].alg.skcipher);Modifying the shared global static array mc_algs with device-specific pointers prevents multiple device instances from functioning correctly. Probing multiple instances or unbinding and rebinding the device will mutate this shared state, which might corrupt the linked lists within the kernel's crypto subsystem.
[Severity: High]
This is a pre-existing issue, but does meson_cipher() leak direct memory access mappings when scatterlist mapping fails?
In drivers/crypto/amlogic/amlogic-gxl-cipher.c:meson_cipher():
if (!nr_sgs || nr_sgs > MAXDESC - 3) {
dev_err(mc->dev, "Invalid SG count %d\n", nr_sgs);
err = -EINVAL;
goto theend;
}And in the error path:
theend:
kfree_sensitive(bkeyiv);
kfree_sensitive(backup_iv);
return err;The error path frees memory but does not call dma_unmap_single() for the previously mapped phykeyiv, nor dma_unmap_sg() for the already mapped areq->src. Can this permanently leak mapping resources until exhaustion if fragmented scatterlists are submitted?
Signature · 6 lines
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2 _______________________________________________ linux-amlogic mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-amlogic
PATCH 3/12 crypto: aspeed - Remove redundant dev_err()
In reply to Pan ChuangSince commit 55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"), devm_request_irq() automatically logs detailed error messages on failure. Remove the now-redundant driver-specific dev_err() calls.
--- drivers/crypto/aspeed/aspeed-acry.c | 4 +--- drivers/crypto/aspeed/aspeed-hace.c | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-)
drivers/crypto/aspeed/aspeed-acry.c +1 -3
@@ -732,10 +732,8 @@ static int aspeed_acry_probe(struct platform_device *pdev) rc =devm_request_irq (dev, acry_dev->irq, aspeed_acry_irq,0 ,dev_name (dev), acry_dev); -if (rc) { -dev_err (dev,"Failed to request irq.\n" ); +if (rc)return rc; - } acry_dev->clk =devm_clk_get_enabled (dev,NULL );if (IS_ERR (acry_dev->clk)) {
drivers/crypto/aspeed/aspeed-hace.c +1 -3
@@ -131,10 +131,8 @@ static int aspeed_hace_probe(struct platform_device *pdev) rc =devm_request_irq (&pdev->dev, hace_dev->irq, aspeed_hace_irq,0 ,dev_name (&pdev->dev), hace_dev); -if (rc) { -dev_err (&pdev->dev,"Failed to request interrupt\n" ); +if (rc)return rc; - }/* Get clk and enable it */ hace_dev->clk =devm_clk_get (&pdev->dev,NULL );
Signature · 1 line
2.34.1
Re: [PATCH 03/12] crypto: aspeed - Remove redundant dev_err()
Quoted 6 lines · Pan Chuang
Since commit
55b48e23f5c4("genirq/devres: Add error handling in devm_request_*_irq()"),devm_request_irq()automatically logs detailed error messages on failure. Remove the now-redundant driver-specificdev_err()calls.Signed-off-by Pan Chuang <[email protected]>
PATCH 4/12 crypto: Remove redundant dev_err()/dev_err_probe()
In reply to Pan ChuangSince commit 55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"), devm_request_irq() automatically logs detailed error messages on failure. Remove the now-redundant driver-specific dev_err() and dev_err_probe() calls.
--- drivers/crypto/atmel-aes.c | 4 +--- drivers/crypto/atmel-sha.c | 4 +--- drivers/crypto/atmel-tdes.c | 4 +--- drivers/crypto/img-hash.c | 4 +--- drivers/crypto/mxs-dcp.c | 8 ++------ drivers/crypto/omap-aes.c | 4 +--- drivers/crypto/omap-des.c | 4 +--- drivers/crypto/omap-sham.c | 5 +---- drivers/crypto/sahara.c | 3 +-- 9 files changed, 10 insertions(+), 30 deletions(-)
drivers/crypto/atmel-aes.c +1 -3
@@ -2364,10 +2364,8 @@ static int atmel_aes_probe(struct platform_device *pdev) err =devm_request_irq (&pdev->dev, aes_dd->irq, atmel_aes_irq,IRQF_SHARED ,"atmel-aes" , aes_dd); -if (err) { -dev_err (dev,"unable to request aes irq.\n" ); +if (err) goto err_tasklet_kill; - }/* Initializing the clock */ aes_dd->iclk =devm_clk_get_prepared (&pdev->dev,"aes_clk" );
drivers/crypto/atmel-sha.c +1 -3
@@ -2604,10 +2604,8 @@ static int atmel_sha_probe(struct platform_device *pdev) err =devm_request_irq (&pdev->dev, sha_dd->irq, atmel_sha_irq,IRQF_SHARED ,"atmel-sha" , sha_dd); -if (err) { -dev_err (dev,"unable to request sha irq.\n" ); +if (err) goto err_tasklet_kill; - }/* Initializing the clock */ sha_dd->iclk =devm_clk_get_prepared (&pdev->dev,"sha_clk" );
drivers/crypto/atmel-tdes.c +1 -3
@@ -978,10 +978,8 @@ static int atmel_tdes_probe(struct platform_device *pdev) err =devm_request_irq (&pdev->dev, tdes_dd->irq, atmel_tdes_irq,IRQF_SHARED ,"atmel-tdes" , tdes_dd); -if (err) { -dev_err (dev,"unable to request tdes irq.\n" ); +if (err) goto err_tasklet_kill; - }/* Initializing the clock */ tdes_dd->iclk =devm_clk_get (&pdev->dev,"tdes_clk" );
drivers/crypto/img-hash.c +1 -3
@@ -966,10 +966,8 @@ static int img_hash_probe(struct platform_device *pdev) err =devm_request_irq (dev, irq, img_irq_handler,0 ,dev_name (dev), hdev); -if (err) { -dev_err (dev,"unable to request irq\n" ); +if (err) goto res_err; - }dev_dbg (dev,"using IRQ channel %d\n" , irq); hdev->hash_clk =devm_clk_get_enabled (&pdev->dev,"hash" );
drivers/crypto/mxs-dcp.c +2 -6
@@ -1085,17 +1085,13 @@ static int mxs_dcp_probe(struct platform_device *pdev) ret =devm_request_irq (dev, dcp_vmi_irq, mxs_dcp_irq,0 ,"dcp-vmi-irq" , sdcp); -if (ret) { -dev_err (dev,"Failed to claim DCP VMI IRQ!\n" ); +if (ret)return ret; - } ret =devm_request_irq (dev, dcp_irq, mxs_dcp_irq,0 ,"dcp-irq" , sdcp); -if (ret) { -dev_err (dev,"Failed to claim DCP IRQ!\n" ); +if (ret)return ret; - }/* Allocate coherent helper block. */ sdcp->coh = devm_kzalloc(dev, sizeof(*sdcp->coh) + DCP_ALIGNMENT,
drivers/crypto/omap-aes.c +1 -3
@@ -1172,10 +1172,8 @@ static int omap_aes_probe(struct platform_device *pdev) err =devm_request_irq (dev, irq, omap_aes_irq,0 ,dev_name (dev), dd); -if (err) { -dev_err (dev,"Unable to grab omap-aes IRQ\n" ); +if (err) goto err_irq; - } }spin_lock_init (&dd->lock);
drivers/crypto/omap-des.c +1 -3
@@ -1017,10 +1017,8 @@ static int omap_des_probe(struct platform_device *pdev) err =devm_request_irq (dev, irq, omap_des_irq,0 ,dev_name (dev), dd); -if (err) { -dev_err (dev,"Unable to grab omap-des IRQ\n" ); +if (err) goto err_irq; - } }
drivers/crypto/omap-sham.c +1 -4
@@ -2092,11 +2092,8 @@ static int omap_sham_probe(struct platform_device *pdev) err =devm_request_irq (dev, dd->irq, dd->pdata->intr_hdlr,IRQF_TRIGGER_NONE ,dev_name (dev), dd); -if (err) { -dev_err (dev,"unable to request irq %d, err = %d\n" , - dd->irq, err); +if (err) goto data_err; - }dma_cap_zero (mask);dma_cap_set (DMA_SLAVE , mask);
drivers/crypto/sahara.c +1 -2
@@ -1305,8 +1305,7 @@ static int sahara_probe(struct platform_device *pdev) err =devm_request_irq (&pdev->dev, irq, sahara_irq_handler,0 ,dev_name (&pdev->dev), dev);if (err) -return dev_err_probe (&pdev->dev, err, -"failed to request irq\n" ); +return err;/* clocks */ dev->clk_ipg =devm_clk_get_enabled (&pdev->dev,"ipg" );
Signature · 1 line
2.34.1
[email protected]
AI reviewRe: [PATCH 04/12] crypto: Remove redundant dev_err()/dev_err_probe()
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Premature IRQ registration before clocks and driver state are initialized --
--- Patch [4]: [PATCH 04/12] crypto: Remove redundant dev_err()/dev_err_probe() --- commit 1a1461364ed2dcfb8ce0bfe3945b90a7c3d3835d
Author: Pan Chuang <[email protected]>
Subject: crypto: Remove redundant dev_err()/dev_err_probe()
Since devm_request_irq() automatically logs detailed error messages on failure, this patch removes the now-redundant driver-specific dev_err() and dev_err_probe() calls across multiple crypto drivers.
drivers/crypto/atmel-aes.c static int atmel_aes_probe(struct platform_device *pdev) · line 2371
Quoted 8 lines
drivers/crypto/atmel-aes.c +0
-0@@ -2369,10 +2369,8 @@ static int atmel_aes_probe(struct platform_device *pdev) err =devm_request_irq (&pdev->dev, aes_dd->irq, atmel_aes_irq,IRQF_SHARED ,"atmel-aes" , aes_dd);
[Severity: High]
This is a pre-existing issue, but can this premature IRQ request cause a crash?
Since IRQF_SHARED is used here, if another device sharing this IRQ line triggers an interrupt before the aes_clk is enabled, atmel_aes_irq() will unconditionally read from the AES_ISR register:
atmel_aes_irq()
reg = atmel_aes_read(aes_dd, AES_ISR);Could this result in a bus fault because the hardware is still unclocked?
Quoted 8 lines
drivers/crypto/atmel-aes.c +1
-3-if (err) { -dev_err (dev,"unable to request aes irq.\n" ); +if (err) goto err_tasklet_kill; - }/* Initializing the clock */ aes_dd->iclk =devm_clk_get_prepared (&pdev->dev,"aes_clk" );
[ ... ]
drivers/crypto/mxs-dcp.c static int mxs_dcp_probe(struct platform_device *pdev) · line 1087
Quoted 8 lines
drivers/crypto/mxs-dcp.c +0
-0@@ -1085,17 +1085,13 @@ static int mxs_dcp_probe(struct platform_device *pdev) ret =devm_request_irq (dev, dcp_vmi_irq, mxs_dcp_irq,0 ,"dcp-vmi-irq" , sdcp);
[Severity: High]
This is a pre-existing issue, but does this request the IRQ too early?
If a pending interrupt from the bootloader fires immediately here, mxs_dcp_irq() might access uninitialized data structures, as the locks in sdcp->completion aren't initialized until later in the mxs_dcp_probe() function.
Quoted 5 lines
drivers/crypto/mxs-dcp.c +1
-3-if (ret) { -dev_err (dev,"Failed to claim DCP VMI IRQ!\n" ); +if (ret)return ret; - }
[ ... ]
drivers/crypto/omap-aes.c static int omap_aes_probe(struct platform_device *pdev) · line 1174
Quoted 8 lines
drivers/crypto/omap-aes.c +0
-0@@ -1172,10 +1172,8 @@ static int omap_aes_probe(struct platform_device *pdev) err =devm_request_irq (dev, irq, omap_aes_irq,0 ,dev_name (dev), dd);
[Severity: High]
This is a pre-existing issue, but could this lead to a BUG_ON crash?
The IRQ is requested after the device is suspended via pm_runtime_put_sync(), and dd->in_sg is still NULL. If a pending interrupt fires right after this registration, could the handler try to access the suspended device or trip a BUG_ON over the NULL scatterlist?
Quoted 6 lines
drivers/crypto/omap-aes.c +1
-3-if (err) { -dev_err (dev,"Unable to grab omap-aes IRQ\n" ); +if (err) goto err_irq; - } }
[ ... ]
drivers/crypto/sahara.c static int sahara_probe(struct platform_device *pdev) · line 1306
Quoted 7 lines
drivers/crypto/sahara.c +0
-0@@ -1305,8 +1305,7 @@ static int sahara_probe(struct platform_device *pdev) err =devm_request_irq (&pdev->dev, irq, sahara_irq_handler,0 ,dev_name (&pdev->dev), dev);
[Severity: High]
This is a pre-existing issue, but can the handler execute before the clocks are ready?
The ipg and ahb clocks are retrieved and enabled after this IRQ registration. If a bootloader left an interrupt pending, could sahara_irq_handler() attempt to access unclocked hardware registers?
Quoted 7 lines
drivers/crypto/sahara.c +1
-2if (err) -return dev_err_probe (&pdev->dev, err, -"failed to request irq\n" ); +return err;/* clocks */ dev->clk_ipg =devm_clk_get_enabled (&pdev->dev,"ipg" );
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
PATCH 5/12 crypto: caam - Remove redundant dev_err()
In reply to Pan ChuangSince commit 55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"), devm_request_irq() automatically logs detailed error messages on failure. Remove the now-redundant driver-specific dev_err() calls.
--- drivers/crypto/caam/jr.c | 2 -- 1 file changed, 2 deletions(-)
drivers/crypto/caam/jr.c +0 -2
@@ -566,8 +566,6 @@ static int caam_jr_init(struct device *dev) error =devm_request_irq (dev, jrp->irq, caam_jr_interrupt,IRQF_SHARED ,dev_name (dev), dev);if (error) { -dev_err (dev,"can't connect JobR %d interrupt (%d)\n" , - jrp->ridx, jrp->irq);tasklet_kill (&jrp->irqtask); }
Signature · 1 line
2.34.1
PATCH 6/12 crypto: ccree - Remove redundant dev_err()
In reply to Pan ChuangSince commit 55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"), devm_request_irq() automatically logs detailed error messages on failure. Remove the now-redundant driver-specific dev_err() calls.
--- drivers/crypto/ccree/cc_driver.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/crypto/ccree/cc_driver.c +1 -3
@@ -471,10 +471,8 @@ static int init_cc_resources(struct platform_device *plat_dev)/* register the driver isr function */ rc =devm_request_irq (dev, irq, cc_isr,IRQF_SHARED ,"ccree" , new_drvdata); -if (rc) { -dev_err (dev,"Could not register to interrupt %d\n" , irq); +if (rc) goto post_pm_err; - }dev_dbg (dev,"Registered to IRQ: %d\n" , irq);init_cc_cache_params (new_drvdata);
Signature · 1 line
2.34.1
PATCH 7/12 crypto: sl3516 - Remove redundant dev_err()
In reply to Pan ChuangSince commit 55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"), devm_request_irq() automatically logs detailed error messages on failure. Remove the now-redundant driver-specific dev_err() calls.
--- drivers/crypto/gemini/sl3516-ce-core.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/crypto/gemini/sl3516-ce-core.c +1 -3
@@ -414,10 +414,8 @@ static int sl3516_ce_probe(struct platform_device *pdev)return irq; err =devm_request_irq (&pdev->dev, irq, ce_irq_handler,0 ,"crypto" , ce); -if (err) { -dev_err (ce->dev,"Cannot request Crypto Engine IRQ (err=%d)\n" , err); +if (err)return err; - } ce->reset =devm_reset_control_get (&pdev->dev,NULL );if (IS_ERR (ce->reset))
Signature · 1 line
2.34.1
Re: [PATCH 07/12] crypto: sl3516 - Remove redundant dev_err()
Quoted 6 lines · Pan Chuang
Since commit
55b48e23f5c4("genirq/devres: Add error handling in devm_request_*_irq()"),devm_request_irq()automatically logs detailed error messages on failure. Remove the now-redundant driver-specificdev_err()calls.Signed-off-by Pan Chuang <[email protected]>
Yours,
Linus Walleij
PATCH 8/12 crypto: safexcel - Remove redundant dev_err()
In reply to Pan ChuangSince commit 55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"), devm_request_threaded_irq() automatically logs detailed error messages on failure. Remove the now-redundant driver-specific dev_err() calls.
--- drivers/crypto/inside-secure/safexcel.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/crypto/inside-secure/safexcel.c +1 -3
@@ -1172,10 +1172,8 @@ static int safexcel_request_ring_irq(void *pdev, int irqid, ret =devm_request_threaded_irq (dev, irq, handler, threaded_handler,IRQF_ONESHOT ,dev_name (dev), ring_irq_priv); -if (ret) { -dev_err (dev,"unable to request IRQ %d\n" , irq); +if (ret)return ret; - }/* Set affinity */ cpu =cpumask_local_spread (ring_id,NUMA_NO_NODE );
Signature · 1 line
2.34.1
PATCH 9/12 crypto: keembay - Remove redundant dev_err()
In reply to Pan ChuangSince commit 55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"), devm_request_threaded_irq() automatically logs detailed error messages on failure. Remove the now-redundant driver-specific dev_err() calls.
--- drivers/crypto/intel/keembay/keembay-ocs-aes-core.c | 4 +--- drivers/crypto/intel/keembay/keembay-ocs-ecc.c | 4 +--- drivers/crypto/intel/keembay/keembay-ocs-hcu-core.c | 4 +--- 3 files changed, 3 insertions(+), 9 deletions(-)
drivers/crypto/intel/keembay/keembay-ocs-aes-core.c +1 -3
@@ -1609,10 +1609,8 @@ static int kmb_ocs_aes_probe(struct platform_device *pdev) rc =devm_request_threaded_irq (dev, aes_dev->irq, ocs_aes_irq_handler,NULL ,0 ,"keembay-ocs-aes" , aes_dev); -if (rc <0 ) { -dev_err (dev,"Could not request IRQ\n" ); +if (rc <0 )return rc; - }INIT_LIST_HEAD (&aes_dev->list);spin_lock (&ocs_aes.lock);
drivers/crypto/intel/keembay/keembay-ocs-ecc.c +1 -3
@@ -900,10 +900,8 @@ static int kmb_ocs_ecc_probe(struct platform_device *pdev) rc =devm_request_threaded_irq (dev, ecc_dev->irq, ocs_ecc_irq_handler,NULL ,0 ,"keembay-ocs-ecc" , ecc_dev); -if (rc <0 ) { -dev_err (dev,"Could not request IRQ\n" ); +if (rc <0 ) goto list_del; - }/* Add device to the list of OCS ECC devices. */ spin_lock (&ocs_ecc.lock);
drivers/crypto/intel/keembay/keembay-ocs-hcu-core.c +1 -3
@@ -1204,10 +1204,8 @@ static int kmb_ocs_hcu_probe(struct platform_device *pdev) rc =devm_request_threaded_irq (&pdev->dev, hcu_dev->irq, ocs_hcu_irq_handler,NULL ,0 ,"keembay-ocs-hcu" , hcu_dev); -if (rc <0 ) { -dev_err (dev,"Could not request IRQ.\n" ); +if (rc <0 )return rc; - }INIT_LIST_HEAD (&hcu_dev->list);
Signature · 1 line
2.34.1
PATCH 10/12 crypto: octeontx2 - Remove redundant dev_err()
In reply to Pan ChuangSince commit 55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"), devm_request_irq() automatically logs detailed error messages on failure. Remove the now-redundant driver-specific dev_err() calls.
--- drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c +1 -4
@@ -430,11 +430,8 @@ static int cptpf_register_afpf_mbox_intr(struct otx2_cptpf_dev *cptpf)/* Register AF-PF mailbox interrupt handler */ ret =devm_request_irq (dev, irq, otx2_cptpf_afpf_mbox_intr,0 ,"CPTAFPF Mbox" , cptpf); -if (ret) { -dev_err (dev, -"IRQ registration failed for PFAF mbox irq\n" ); +if (ret)return ret; - }/* Clear interrupt if any, to avoid spurious interrupts */ otx2_cpt_write64 (cptpf->reg_base,BLKADDR_RVUM ,0 ,RVU_PF_INT ,0x1ULL );/* Enable AF-PF interrupt */
Signature · 1 line
2.34.1
PATCH 11/12 crypto: rockchip - Remove redundant dev_err()
In reply to Pan ChuangSince commit 55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"), devm_request_irq() automatically logs detailed error messages on failure. Remove the now-redundant driver-specific dev_err() calls.
--- drivers/crypto/rockchip/rk3288_crypto.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/crypto/rockchip/rk3288_crypto.c +1 -3
@@ -365,10 +365,8 @@ static int rk_crypto_probe(struct platform_device *pdev) rk_crypto_irq_handle,IRQF_SHARED , "rk-crypto", pdev); -if (err) { -dev_err (&pdev->dev,"irq request failed.\n" ); +if (err) goto err_crypto; - } crypto_info->engine =crypto_engine_alloc_init (&pdev->dev, true);if (!crypto_info->engine) {
Signature · 1 line
2.34.1
PATCH 12/12 crypto: stm32 - Remove redundant dev_err()
In reply to Pan ChuangSince commit 55b48e23f5c4 ("genirq/devres: Add error handling in devm_request_*_irq()"), devm_request_threaded_irq() automatically logs detailed error messages on failure. Remove the now-redundant driver-specific dev_err() calls.
--- drivers/crypto/stm32/stm32-cryp.c | 4 +--- drivers/crypto/stm32/stm32-hash.c | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-)
drivers/crypto/stm32/stm32-cryp.c +1 -3
@@ -2590,10 +2590,8 @@ static int stm32_cryp_probe(struct platform_device *pdev) ret =devm_request_threaded_irq (dev, irq, stm32_cryp_irq, stm32_cryp_irq_thread,IRQF_ONESHOT ,dev_name (dev), cryp); -if (ret) { -dev_err (dev,"Cannot grab IRQ\n" ); +if (ret)return ret; - } cryp->clk =devm_clk_get (dev,NULL );if (IS_ERR (cryp->clk)) {
drivers/crypto/stm32/stm32-hash.c +1 -3
@@ -2356,10 +2356,8 @@ static int stm32_hash_probe(struct platform_device *pdev) stm32_hash_irq_thread,IRQF_ONESHOT , dev_name(dev), hdev); -if (ret) { -dev_err (dev,"Cannot grab IRQ\n" ); +if (ret)return ret; - } }else {dev_info (dev,"No IRQ, use polling mode\n" ); hdev->polled = true;
Signature · 1 line
2.34.1
Re: [PATCH 12/12] crypto: stm32 - Remove redundant dev_err()
Quoted 43 lines · Pan Chuang
Since commit
55b48e23f5c4("genirq/devres: Add error handling in devm_request_*_irq()"),devm_request_threaded_irq()automatically logs detailed error messages on failure. Remove the now-redundant driver-specificdev_err()calls.Signed-off-by Pan Chuang <[email protected]>--- drivers/crypto/stm32/stm32-cryp.c | 4 +--- drivers/crypto/stm32/stm32-hash.c | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-)drivers/crypto/stm32/stm32-cryp.c +1
-3@@ -2590,10 +2590,8 @@ static int stm32_cryp_probe(struct platform_device *pdev) ret =devm_request_threaded_irq (dev, irq, stm32_cryp_irq, stm32_cryp_irq_thread,IRQF_ONESHOT ,dev_name (dev), cryp); -if (ret) { -dev_err (dev,"Cannot grab IRQ\n" ); +if (ret)return ret; - } cryp->clk =devm_clk_get (dev,NULL );if (IS_ERR (cryp->clk)) {drivers/crypto/stm32/stm32-hash.c +1
-3@@ -2356,10 +2356,8 @@ static int stm32_hash_probe(struct platform_device *pdev) stm32_hash_irq_thread,IRQF_ONESHOT , dev_name(dev), hdev); -if (ret) { -dev_err (dev,"Cannot grab IRQ\n" ); +if (ret)return ret; - } }else {dev_info (dev,"No IRQ, use polling mode\n" ); hdev->polled = true;
Herbert Xu
maintainerQuoted 47 lines · Pan Chuang
Commit
55b48e23f5c4("genirq/devres: Add error handling in devm_request_*_irq()") added automatic error logging todevm_request_threaded_irq()anddevm_request_any_context_irq()via the newdevm_request_result()helper, which prints device name, IRQ number, handler functions, and error code on failure.Since
devm_request_irq()is a static inline wrapper arounddevm_request_threaded_irq(), it also benefits from this automatic logging.Remove the now-redundant
dev_err()anddev_err_probe()calls in crypto drivers that follow these devm_request_*_irq() functions, as the core now provides more detailed diagnostic information on failure.Pan Chuang (12):
crypto: allwinner - Remove redundantdev_err()
crypto: amlogic - Remove redundantdev_err()
crypto: aspeed - Remove redundantdev_err()
crypto: Remove redundant dev_err()/dev_err_probe()
crypto: caam - Remove redundantdev_err()
crypto: ccree - Remove redundantdev_err()
crypto: sl3516 - Remove redundantdev_err()
crypto: safexcel - Remove redundantdev_err()
crypto: keembay - Remove redundantdev_err()
crypto: octeontx2 - Remove redundantdev_err()
crypto: rockchip - Remove redundantdev_err()
crypto: stm32 - Remove redundantdev_err()drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c | 4 +--- drivers/crypto/allwinner/sun8i-ss/sun8i-ss-core.c | 4 +--- drivers/crypto/amlogic/amlogic-gxl-core.c | 4 +--- drivers/crypto/aspeed/aspeed-acry.c | 4 +--- drivers/crypto/aspeed/aspeed-hace.c | 4 +--- drivers/crypto/atmel-aes.c | 4 +--- drivers/crypto/atmel-sha.c | 4 +--- drivers/crypto/atmel-tdes.c | 4 +--- drivers/crypto/caam/jr.c | 2 -- drivers/crypto/ccree/cc_driver.c | 4 +--- drivers/crypto/gemini/sl3516-ce-core.c | 4 +--- drivers/crypto/img-hash.c | 4 +--- drivers/crypto/inside-secure/safexcel.c | 4 +--- drivers/crypto/intel/keembay/keembay-ocs-aes-core.c | 4 +--- drivers/crypto/intel/keembay/keembay-ocs-ecc.c | 4 +--- drivers/crypto/intel/keembay/keembay-ocs-hcu-core.c | 4 +--- drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c | 5 +---- drivers/crypto/mxs-dcp.c | 8 ++------
Quoted 11 lines
drivers/crypto/omap-aes.c | 4 +--- drivers/crypto/omap-des.c | 4 +--- drivers/crypto/omap-sham.c | 5 +---- drivers/crypto/rockchip/rk3288_crypto.c | 4 +--- drivers/crypto/sahara.c | 3 +-- drivers/crypto/stm32/stm32-cryp.c | 4 +--- drivers/crypto/stm32/stm32-hash.c | 4 +--- 25 files changed, 25 insertions(+), 78 deletions(-)Signature · 1 line
2.34.1
All applied. Thanks.
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt