Dmitry Eremin-Solenikov(lumag) replied on github web page:
platform/linux-generic/odp_crypto.c
line 103
@@ -146,6 +171,8 @@ odp_crypto_generic_session_t *alloc_session(void)
}
odp_spinlock_unlock(&global->lock);
+ session->idx = (session - global->sessions) / sizeof(*session);
+
Comment:
No, keys should be deleted ASAP - during `odp_crypto_session_destroy()`.
> JannePeltonen wrote
> Yes, the hmac reset of course (what was I thinking?). But for that this
> should be enough (I have used it in my test patch with OpenSSL 1.1.0 and
> 1.0.2): HMAC_Init_ex(ctx, NULL, 0, NULL, NULL) and be more clear in that key
> len is not being changed.
>
> Looking at the man page I do not think HMAC_Init_Ex() is going to even look
> at key_len when key is NULL.
>> JannePeltonen wrote
>> ok, I can see it now. I think it would be less fragile to clear the session
>> at alloc() instead of relying it to be cleared at init_global() and
>> destroy().
>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>> OK, I will add a comment. This is necessary to restart (reset) HMAC
>>> calculation without resetting the key and auth length.
>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>> Whole `odp_crypto_generic_session_t` data is cleared during
>>>> `odp_crypto_session_destroy()`
>>>>> JannePeltonen wrote
>>>>> Why is this needed here? The key length was already set. If this is
>>>>> because of some OpenSSL weirdness, an explanatory comment would be nice.
>>>>>> JannePeltonen wrote
>>>>>> The cipher_valid and hmac_valid bits are not cleared at session
>>>>>> allocation, so they can be left to what they were in an old and freed
>>>>>> session that occupied the same memory slot.
>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>> Then I'd follow "Maxim's rule" and do the renames as a separate
>>>>>>> follow-on PR.
>>>>>>>> muvarov wrote
>>>>>>>> I prefer the rule that new code goes in in clean way. Even if rest of
>>>>>>>> the code used old rules.
>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>> That makes sense to me.
>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>> Then I would prefer to use names w/o underscore for now and to
>>>>>>>>>> change all of them later.
>>>>>>>>>>> Bill Fischofer(Bill-Fischofer-Linaro) wrote:
>>>>>>>>>>> I don't see the need to do that at this point as these are internal
>>>>>>>>>>> routines anyway. That type of restructure might be something to
>>>>>>>>>>> consider for Caterpillar.
>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>> Hmm. The rest of init/term functions do not have such prefix.
>>>>>>>>>>>> Should we change them?
>>>>>>>>>>>>> Dmitry Eremin-Solenikov(lumag) wrote:
>>>>>>>>>>>>> No, `sizeof uint32_t`
>>>>>>>>>>>>>> muvarov wrote
>>>>>>>>>>>>>> `_odp_crypto_term_local`
>>>>>>>>>>>>>>> muvarov wrote
>>>>>>>>>>>>>>> `_odp_crypto_init_local`
>>>>>>>>>>>>>>>> muvarov wrote
>>>>>>>>>>>>>>>> is 32 everywhere MAX_SESSIONS?
https://github.com/Linaro/odp/pull/342#discussion_r157506476
updated_at 2017-12-18 22:35:39
