linux-kernel - [PATCH 12/12] crypto: caam - change JR device ownership scheme

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: <20190904023515.7107-13-andrew.smirnov@gmail.com>
Date:   Tue,  3 Sep 2019 19:35:15 -0700
From:   Andrey Smirnov <andrew.smirnov@...il.com>
To:     linux-crypto@...r.kernel.org
Cc:     Andrey Smirnov <andrew.smirnov@...il.com>,
        Chris Healy <cphealy@...il.com>,
        Lucas Stach <l.stach@...gutronix.de>,
        Horia Geantă <horia.geanta@....com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Iuliana Prodan <iuliana.prodan@....com>,
        linux-kernel@...r.kernel.org
Subject: [PATCH 12/12] crypto: caam - change JR device ownership scheme

Returning -EBUSY from platform device's .remove() callback won't stop
the removal process, so the code in caam_jr_remove() is not going to
have the desired effect of preventing JR from being removed.

In order to be able to deal with removal of the JR device, change the
code as follows:

  1. To make sure that underlying struct device remains valid for as
     long as we have a reference to it, add appropriate device
     refcount management to caam_jr_alloc() and caam_jr_free()

  2. To make sure that device removal doesn't happen in parallel to
      use using the device in caam_jr_enqueue() augment the latter to
      acquire/release device lock for the duration of the subroutine

  3. In order to handle the case when caam_jr_enqueue() is executed
     right after corresponding caam_jr_remove(), add code to check
     that driver data has not been set to NULL.

Signed-off-by: Andrey Smirnov <andrew.smirnov@...il.com>
Cc: Chris Healy <cphealy@...il.com>
Cc: Lucas Stach <l.stach@...gutronix.de>
Cc: Horia Geantă <horia.geanta@....com>
Cc: Herbert Xu <herbert@...dor.apana.org.au>
Cc: Iuliana Prodan <iuliana.prodan@....com>
Cc: linux-crypto@...r.kernel.org
Cc: linux-kernel@...r.kernel.org
---
 drivers/crypto/caam/jr.c | 36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index 47b389cb1c62..8a30bbd7f2aa 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -124,14 +124,6 @@ static int caam_jr_remove(struct platform_device *pdev)
 	jrdev = &pdev->dev;
 	jrpriv = dev_get_drvdata(jrdev);
 
-	/*
-	 * Return EBUSY if job ring already allocated.
-	 */
-	if (atomic_read(&jrpriv->tfm_count)) {
-		dev_err(jrdev, "Device is busy\n");
-		return -EBUSY;
-	}
-
 	/* Unregister JR-based RNG & crypto algorithms */
 	unregister_algs();
 
@@ -300,7 +292,7 @@ struct device *caam_jr_alloc(void)
 
 	if (min_jrpriv) {
 		atomic_inc(&min_jrpriv->tfm_count);
-		dev = min_jrpriv->dev;
+		dev = get_device(min_jrpriv->dev);
 	}
 	spin_unlock(&driver_data.jr_alloc_lock);
 
@@ -318,13 +310,16 @@ void caam_jr_free(struct device *rdev)
 	struct caam_drv_private_jr *jrpriv = dev_get_drvdata(rdev);
 
 	atomic_dec(&jrpriv->tfm_count);
+	put_device(rdev);
 }
 EXPORT_SYMBOL(caam_jr_free);
 
 /**
  * caam_jr_enqueue() - Enqueue a job descriptor head. Returns 0 if OK,
  * -EBUSY if the queue is full, -EIO if it cannot map the caller's
- * descriptor.
+ * descriptor, -ENODEV if given device was removed and is no longer
+ * valid
+ *
  * @dev:  device of the job ring to be used. This device should have
  *        been assigned prior by caam_jr_register().
  * @desc: points to a job descriptor that execute our request. All
@@ -354,15 +349,32 @@ int caam_jr_enqueue(struct device *dev, u32 *desc,
 				u32 status, void *areq),
 		    void *areq)
 {
-	struct caam_drv_private_jr *jrp = dev_get_drvdata(dev);
+	struct caam_drv_private_jr *jrp;
 	struct caam_jrentry_info *head_entry;
 	int head, tail, desc_size;
 	dma_addr_t desc_dma;
 
+	/*
+	 * Lock the device to prevent it from being removed while we
+	 * are using it
+	 */
+	device_lock(dev);
+
+	/*
+	 * If driver data is NULL, it is very likely that this device
+	 * was removed already. Nothing we can do here but bail out.
+	 */
+	jrp = dev_get_drvdata(dev);
+	if (!jrp) {
+		device_unlock(dev);
+		return -ENODEV;
+	}
+
 	desc_size = (caam32_to_cpu(*desc) & HDR_JD_LENGTH_MASK) * sizeof(u32);
 	desc_dma = dma_map_single(dev, desc, desc_size, DMA_TO_DEVICE);
 	if (dma_mapping_error(dev, desc_dma)) {
 		dev_err(dev, "caam_jr_enqueue(): can't map jobdesc\n");
+		device_unlock(dev);
 		return -EIO;
 	}
 
@@ -375,6 +387,7 @@ int caam_jr_enqueue(struct device *dev, u32 *desc,
 	    CIRC_SPACE(head, tail, JOBR_DEPTH) <= 0) {
 		spin_unlock_bh(&jrp->inplock);
 		dma_unmap_single(dev, desc_dma, desc_size, DMA_TO_DEVICE);
+		device_unlock(dev);
 		return -EBUSY;
 	}
 
@@ -411,6 +424,7 @@ int caam_jr_enqueue(struct device *dev, u32 *desc,
 		jrp->inpring_avail = rd_reg32(&jrp->rregs->inpring_avail);
 
 	spin_unlock_bh(&jrp->inplock);
+	device_unlock(dev);
 
 	return 0;
 }
-- 
2.21.0