[PATCH v3 01/28] crypto: change backlog return code to -EIOCBQUEUED

[Gilad Ben-Yossef]

On Mon, Jul 3, 2017 at 3:35 PM, Herbert Xu <herbert at gondor.apana.org.au> wrote:
> On Sun, Jul 02, 2017 at 05:41:43PM +0300, Gilad Ben-Yossef wrote:
>> The crypto API was using the -EBUSY return value to indicate
>> both a hard failure to submit a crypto operation into a
>> transformation provider when the latter was busy and the backlog
>> mechanism was not enabled as well as a notification that the operation
>> was queued into the backlog when the backlog mechanism was enabled.
>>
>> Having the same return code indicate two very different conditions
>> depending on a flag is both error prone and requires extra runtime
>> check like the following to discern between the cases:
>>
>>       if (err == -EINPROGRESS ||
>>           (err == -EBUSY && (ahash_request_flags(req) &
>>                              CRYPTO_TFM_REQ_MAY_BACKLOG)))
>>
>> This patch changes the return code used to indicate a crypto op
>> was queued in the backlog to -EIOCBQUEUED, thus resolving both
>> issues.
>>
>> Signed-off-by: Gilad Ben-Yossef <gilad at benyossef.com>
>
> So you're changing the success case from EBUSY to EIOCBQUEUED.
> This results in a lot of churn as you have to change every single
> driver and caller.
>
> How about changing the error case to use something other than
> EBUSY instead?

That was indeed my first thought - I wanted to change EBUSY to EAGAIN.
It might even be a more descriptive error message since the failure is
transient.

What stopped me was that I saw the algif interface passes this EBUSY value
to user space. I don't know of any software that depends on this value but
I'm really averse to changing user space API.

Of course, we can just plant a (ret != AGAIN : ? EBUSY) in the algif interface
return to user space code path but that felt like a kludge to me.

And it doesn't really save any churn, I think - I'd still have to
visit all the places that
use the flags value to distinguish between the two EBUSY use cases and  I need
to visit most places that check for EBUSY as backlog indication because I'm
replacing the check with the generic replacement, so it doesn't look
like in the end
it will be a smaller change.

Having said that, if you prefer me to replace the error case I'd
happily do that.

Thanks,
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html