Hi Lukasz, Thanks for the detailed analysis. Series Acked-by: Akhil Goyal <akhil.go...@nxp.com>
Applied to dpdk-next-crypto Thanks. > > This patch fixes management of memory for authentication > and encryption keys. > There were two issues with former state of implementation: > > 1) Invalid access to dpaa_sec_session union members > The dpaa_sec_session structure includes an anonymous union: > union { > struct {...} aead_key; > struct { > struct {...} cipher_key; > struct {...} auth_key; > }; > }; > Depending on the used algorithm a rte_zmalloc() function > allocated memory that was kept in aead_key, cipher_key > or auth_key. However every time the memory was released, > rte_free() was called only on cipher and auth keys, even > if pointer to allocated memory was stored in aead_key. > > The C language specification defines such behavior as undefined. > As the cipher_key and aead_key are similar, have same sizes and > alignment, it has worked, but it's directly against C specification. > > This patch fixes this, providing a free_session_data() function > to free the keys data. It verifies which algorithm was used > (aead or auth+cipher) and frees proper part of the union. > > 2) Some keys might have been freed multiple times > In functions like: dpaa_sec_cipher_init(), dpaa_sec_auth_init(), > dpaa_sec_chain_init(), dpaa_sec_aead_init() keys data were freed > before returning due to some error conditions. However the pointers > were not zeroed causing another calls to ret_free from higher > layers of code. This causes an error log about invalid memory address > to be printed. > > This patch fixes it by making only one layer responsible for freeing > memory: > * dpaa_sec_set_session_parameters() for allocations made in: > - dpaa_sec_cipher_init() > - dpaa_sec_auth_init() > - dpaa_sec_chain_init() > - dpaa_sec_aead_init() > * dpaa_sec_set_ipsec_session() for allocations made in: > - dpaa_sec_ipsec_aead_init() > - dpaa_sec_ipsec_proto_init() > * dpaa_sec_set_pdcp_session() for allocations made in: > - dpaa_sec_set_pdcp_session() /*only one layer in case of pdcp*/ > > Signed-off-by: Lukasz Wojciechowski <l.wojciec...@partner.samsung.com> > ---