Received-SPF: softfail (google.com: domain of transitioning linux-crypto-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
From:   Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
To:     gilad@benyossef.com
Cc:     herbert@gondor.apana.org.au, davem@davemloft.net,
        linux-crypto@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
        Dung Nguyen <dung.nguyen.zy@renesas.com>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Subject: [RFC/PATCH] crypto: ccree - fix a race of enqueue_seq() in send_request_init()
Date:   Fri, 11 Mar 2022 17:19:09 +0900
Message-Id: <20220311081909.1661934-1-yoshihiro.shimoda.uh@renesas.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

From: Dung Nguyen <dung.nguyen.zy@renesas.com>

When loading ccree.ko, after registering cipher algorithm at
cc_cipher_alloc() and cc_hash_alloc() -> send_request_init() ->
enqueue_seq() has called to pushing descriptor into HW.

On other hand, if another thread have called to encrypt/decrypt
cipher (cc_cipher_encrypt/decrypt) -> cc_send_request() ->
cc_do_send_request() -> enqueue_seq() while send_request_init()
is running, the thread also has called to pushing descriptor into HW.
And then, it's possible to overwrite data.

The cc_do_send_request() locks mgr->hw_lock, but send_request_init()
doesn't lock mgr->hw_lock before enqueue_seq() is called. So,
send_request_init() should lock mgr->hw_lock before enqueue_seq() is
called.

This issue is possible to cause the following way in rare cases:
- CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n
- insmod ccree.ko & insmod tcrypt.ko

Signed-off-by: Dung Nguyen <dung.nguyen.zy@renesas.com>
[shimoda: revise the subject/description]
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 I believe we should fix this race. But, as I wrote the desciption
 above, there is in rare cases. So, I marked this patch as RFC.

 drivers/crypto/ccree/cc_request_mgr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/crypto/ccree/cc_request_mgr.c b/drivers/crypto/ccree/cc_request_mgr.c
index 887162df50f9..eef40022258b 100644
--- a/drivers/crypto/ccree/cc_request_mgr.c
+++ b/drivers/crypto/ccree/cc_request_mgr.c
@@ -513,6 +513,7 @@ int send_request_init(struct cc_drvdata *drvdata, struct cc_hw_desc *desc,
 
 	set_queue_last_ind(drvdata, &desc[(len - 1)]);
 
+	spin_lock_bh(&req_mgr_h->hw_lock);
 	/*
 	 * We are about to push command to the HW via the command registers
 	 * that may reference host memory. We need to issue a memory barrier
@@ -520,6 +521,7 @@ int send_request_init(struct cc_drvdata *drvdata, struct cc_hw_desc *desc,
 	 */
 	wmb();
 	enqueue_seq(drvdata, desc, len);
+	spin_unlock_bh(&req_mgr_h->hw_lock);
 
 	/* Update the free slots in HW queue */
 	req_mgr_h->q_free_slots =
-- 
2.25.1