2014-04-11 07:34:05

by Fan Du

[permalink] [raw]
Subject: Why care about signal when instantiate an crypt template

Hi,

I recently bump into a issue, ike daemon got interrupted(EINTR),
after looking at the code, it seems there are places in crypto code
where returning EINTR when current tasks has signal pending.

For example:
crypto_alloc_base and crypto_alloc_tfm

435 err:
436 if (err != -EAGAIN)
437 break;
438 if (signal_pending(current)) {
439 err = -EINTR;
440 break;
441 }
442 }

I can't understand why the codes here needs to care about signals?

Thanks a lot for any explanations.

--
浮沉随浪只记今朝笑

--fan


2014-04-17 07:44:52

by Herbert Xu

[permalink] [raw]
Subject: Re: Why care about signal when instantiate an crypt template

Fan Du <[email protected]> wrote:
> Hi,
>
> I recently bump into a issue, ike daemon got interrupted(EINTR),
> after looking at the code, it seems there are places in crypto code
> where returning EINTR when current tasks has signal pending.
>
> For example:
> crypto_alloc_base and crypto_alloc_tfm
>
> 435 err:
> 436 if (err != -EAGAIN)
> 437 break;
> 438 if (signal_pending(current)) {
> 439 err = -EINTR;
> 440 break;
> 441 }
> 442 }
>
> I can't understand why the codes here needs to care about signals?

Because otherwise you may end up with something that you can't
kill from user-space. You should fix your app.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

2014-04-17 09:15:33

by Fan Du

[permalink] [raw]
Subject: Re: Why care about signal when instantiate an crypt template

Hi Herbert

On 2014年04月17日 15:44, Herbert Xu wrote:
> Fan Du<[email protected]> wrote:
>> Hi,
>>
>> I recently bump into a issue, ike daemon got interrupted(EINTR),
>> after looking at the code, it seems there are places in crypto code
>> where returning EINTR when current tasks has signal pending.
>>
>> For example:
>> crypto_alloc_base and crypto_alloc_tfm
>>
>> 435 err:
>> 436 if (err != -EAGAIN)
>> 437 break;
>> 438 if (signal_pending(current)) {
>> 439 err = -EINTR;
>> 440 break;
>> 441 }
>> 442 }
>>
>> I can't understand why the codes here needs to care about signals?
>
> Because otherwise you may end up with something that you can't
> kill from user-space. You should fix your app.
Ok, I'm starting to follow your meaning.

The signal checking is only to bail out from the infinite loop, it shouldn't
override original err code -EAGAIN with -EINTR.

With -EAGAIN, user space app can try again, what do you think?

@@ -550,12 +550,8 @@ void *crypto_alloc_tfm(const char *alg_name,
err = PTR_ERR(tfm);

err:
- if (err != -EAGAIN)
+ if ((err != -EAGAIN) || signal_pending(current))
break;
- if (signal_pending(current)) {
- err = -EINTR;
- break;
- }
}




--
浮沉随浪只记今朝笑

--fan

2014-04-17 09:52:48

by Herbert Xu

[permalink] [raw]
Subject: Re: Why care about signal when instantiate an crypt template

On Thu, Apr 17, 2014 at 05:20:34PM +0800, Fan Du wrote:
>
> The signal checking is only to bail out from the infinite loop, it shouldn't
> override original err code -EAGAIN with -EINTR.
>
> With -EAGAIN, user space app can try again, what do you think?

EAGAIN makes no sense when we bail out due to a signal.

Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt