Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

crypto: caam/jr - Convert to platform remove callback returning void

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.

The driver adapted here suffers from this wrong assumption. Returning
-EBUSY if there are still users results in resource leaks and probably a
crash. Also further down passing the error code of caam_jr_shutdown() to
the caller only results in another error message and has no further
consequences compared to returning zero.

Still convert the driver to return no value in the remove callback. This
also allows to drop caam_jr_platform_shutdown() as the only function
called by it now has the same prototype.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

authored by

Uwe Kleine-König and committed by
Herbert Xu
304a2efe 580399bb

+9 -13
+9 -13
drivers/crypto/caam/jr.c
··· 180 180 return ret; 181 181 } 182 182 183 - static int caam_jr_remove(struct platform_device *pdev) 183 + static void caam_jr_remove(struct platform_device *pdev) 184 184 { 185 185 int ret; 186 186 struct device *jrdev; ··· 193 193 caam_rng_exit(jrdev->parent); 194 194 195 195 /* 196 - * Return EBUSY if job ring already allocated. 196 + * If a job ring is still allocated there is trouble ahead. Once 197 + * caam_jr_remove() returned, jrpriv will be freed and the registers 198 + * will get unmapped. So any user of such a job ring will probably 199 + * crash. 197 200 */ 198 201 if (atomic_read(&jrpriv->tfm_count)) { 199 - dev_err(jrdev, "Device is busy\n"); 200 - return -EBUSY; 202 + dev_alert(jrdev, "Device is busy; consumers might start to crash\n"); 203 + return; 201 204 } 202 205 203 206 /* Unregister JR-based RNG & crypto algorithms */ ··· 215 212 ret = caam_jr_shutdown(jrdev); 216 213 if (ret) 217 214 dev_err(jrdev, "Failed to shut down job ring\n"); 218 - 219 - return ret; 220 - } 221 - 222 - static void caam_jr_platform_shutdown(struct platform_device *pdev) 223 - { 224 - caam_jr_remove(pdev); 225 215 } 226 216 227 217 /* Main per-ring interrupt handler */ ··· 819 823 .pm = pm_ptr(&caam_jr_pm_ops), 820 824 }, 821 825 .probe = caam_jr_probe, 822 - .remove = caam_jr_remove, 823 - .shutdown = caam_jr_platform_shutdown, 826 + .remove_new = caam_jr_remove, 827 + .shutdown = caam_jr_remove, 824 828 }; 825 829 826 830 static int __init jr_driver_init(void)