[PATCHv2,09/22] crypto: add timeout to crypto_wait_req
diff mbox series

Message ID 20191105140111.20285-10-t-kristo@ti.com
State New
Headers show
Series
  • crypto: omap-sham: fixes towards 5.5
Related show

Commit Message

Tero Kristo Nov. 5, 2019, 2 p.m. UTC
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.

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

Comments

Herbert Xu Nov. 15, 2019, 5:29 a.m. UTC | #1
On Tue, Nov 05, 2019 at 04:00:58PM +0200, Tero Kristo 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.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> ---
>  include/linux/crypto.h | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)

As we discussed before this patch is not acceptable because it
would cause a use-after-free.

Cheers,
Tero Kristo Nov. 15, 2019, 7:44 a.m. UTC | #2
On 15/11/2019 07:29, Herbert Xu wrote:
> On Tue, Nov 05, 2019 at 04:00:58PM +0200, Tero Kristo 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.
>>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> ---
>>   include/linux/crypto.h | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> As we discussed before this patch is not acceptable because it
> would cause a use-after-free.

Yep, its fine to ditch this one as it was provided as just a nice to 
have initially anyway. Any comments for the rest of the series?

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Herbert Xu Nov. 15, 2019, 9:10 a.m. UTC | #3
On Fri, Nov 15, 2019 at 09:44:51AM +0200, Tero Kristo wrote:
>
> Yep, its fine to ditch this one as it was provided as just a nice to have
> initially anyway. Any comments for the rest of the series?

The rest of them are fine.

Thanks,

Patch
diff mbox series

diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 19ea3a371d7b..41279eeecb21 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;
 	};