Re: [PATCH 09/10] crypto: add timeout to crypto_wait_req

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/11/2019 08:39, Gilad Ben-Yossef wrote:
Hi,


On Thu, Oct 17, 2019 at 3:26 PM Tero Kristo <t-kristo@xxxxxx> wrote:

Currently crypto_wait_req waits indefinitely for an async crypto request
to complete. This is bad as it can cause for example the crypto test
manager to hang without any notification as to why it has happened.
Instead of waiting indefinitely, add a 1 second timeout to the call,
and provide a warning print if a timeout happens.

While the incentive is clear and positive, this suggested solution
creates problems of its own.
In many (most?) cases where we are waiting here, we are waiting for a
DMA operation to finish from hardware.
Exiting while this pending DMA operation is not finished, even with a
proper error return value, is dangerous because
unless the calling code takes great care to not release the memory the
DMA is being done from/to, this can have disastrous effects.

As Eric has already mentioned, one second might seem like a long time,
but we don't really know if it is enough.

How about adding a second API (ig. crypto_wait_req_timeout) which
supports a calee specified timeout where
the calle knows how to correctly deal with timeout and port the
relevant call sites to use this?

Yeah, that would work for me. I guess we could just swap the testmgr to use this timeout API, as it is quite clear it should timeout rather than wait indefinitely, and afaics, the data buffers it uses are limited size. It doesn't really matter for it whether the timeout is 1 second or 10 seconds, as long as it eventually times out.

-Tero


Thanks!
Gilad



Signed-off-by: Tero Kristo <t-kristo@xxxxxx>
---
  include/linux/crypto.h | 9 ++++++++-
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 19ea3a371d7b..b8f0e5c3cc0c 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -682,8 +682,15 @@ static inline int crypto_wait_req(int err, struct crypto_wait *wait)
         switch (err) {
         case -EINPROGRESS:
         case -EBUSY:
-               wait_for_completion(&wait->completion);
+               err = wait_for_completion_timeout(&wait->completion,
+                                                 msecs_to_jiffies(1000));
                 reinit_completion(&wait->completion);
+               if (!err) {
+                       pr_err("%s: timeout for %p\n", __func__, wait);
+                       err = -ETIMEDOUT;
+                       break;
+               }
+
                 err = wait->err;
                 break;
         };
--
2.17.1

--



--
Gilad Ben-Yossef
Chief Coffee Drinker

values of β will give rise to dom!


--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki



[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux