Bill Fischofer(Bill-Fischofer-Linaro) replied on github web page:
platform/linux-generic/odp_crypto.c
@@ -112,8 +115,22 @@ struct odp_crypto_generic_session_t {
const EVP_MD *evp_md;
crypto_func_t func;
} auth;
+
+ /* These bitfields are cleared at odp_crypto_session_destroy()
+ * together with the rest of data */
+ crypto_valid cipher_valid[(ODP_THREAD_COUNT_MAX + CV_SIZE - 1) /
+ CV_SIZE];
+ crypto_valid hmac_valid[(ODP_THREAD_COUNT_MAX + CV_SIZE - 1) /
+ CV_SIZE];
+ unsigned idx;
};
+#define IS_VALID(session, type, id) \
+ (session->type ## _valid[id / CV_SIZE] & (1 << (id % CV_SIZE)))
+
+#define SET_VALID(session, type, id) \
+ session->type ## _valid[id / CV_SIZE] |= (1 << (id % CV_SIZE))
Comment:
Checkpatch wants this enclosed in parentheses. Since `IS_VALID` follows that
model, no harm in doing so with `SET_VALID` as well.
```
ERROR: Macros with complex values should be enclosed in parentheses
#63: FILE: platform/linux-generic/odp_crypto.c:131:
+#define SET_VALID(session, type, id) \
+ session->type ## _valid[id / CV_SIZE] |= (1 << (id % CV_SIZE))
```
> Dmitry Eremin-Solenikov(lumag) wrote:
> 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_r157580102
updated_at 2017-12-18 22:35:39
