Re: [dpdk-dev] [PATCH v2 1/2] crypto/ccp: sha3 support enabling in ccp

[David Marchand <david.marchand@redhat.com>]

On Wed, Feb 5, 2020 at 11:22 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> 05/02/2020 13:24, Akhil Goyal:
> > > For series,
> > > Acked-by: Ravi Kumar <ravi1.kumar@amd.com>
> > >
> > > >
> > > >From: Sardar Shamsher Singh <Shamshersingh.Sardar@amd.com>
> > > >
> > > >sha3 support enabled in AMD-CCP crypto controller
> > > >
> > > >Signed-off-by: Sardar Shamsher Singh <Shamshersingh.Sardar@amd.com>
> > > >---
> > Change patch title and description as below
> >     cryptodev: fix missing SHA3 algo strings
> >
> >     SHA3 support was added earlier but algo strings were
> >     missing. This patch add the missing strings.
> >
> >     Fixes: 1df800f89518 ("crypto/ccp: support SHA3 family")
> >     Cc: stable@dpdk.org
> >
> >     Signed-off-by: Sardar Shamsher Singh <Shamshersingh.Sardar@amd.com>
> >     Acked-by: Ravi Kumar <ravi1.kumar@amd.com>
> >     Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
> >
> > Applied to dpdk-next-crypto
>
> Sorry I must drop this patch because it triggers an ABI warning:
>   [C]'const char* rte_crypto_auth_algorithm_strings[]' was changed at rte_crypto_sym.h:320:1:
>     size of symbol changed from 168 to 232

This is still not clear to me, but here is how I understand the issue.


An exposed array (and its size) ends up in both the shared library and
the final binary data section.

[dmarchan@wsfd-netdev66 dpdk]$ readelf -sW
~/builds/build-gcc-shared/app/dpdk-test-crypto-perf |grep
rte_crypto_auth_algorithm_strings
    86: 00000000004141a0   168 OBJECT  GLOBAL DEFAULT   24
rte_crypto_auth_algorithm_strings@DPDK_20.0 (4)
   308: 00000000004141a0   168 OBJECT  GLOBAL DEFAULT   24
rte_crypto_auth_algorithm_strings@@DPDK_20.0
[dmarchan@wsfd-netdev66 dpdk]$ readelf -sW
~/builds/build-gcc-shared/lib/librte_cryptodev.so |grep
rte_crypto_auth_algorithm_strings
    57: 000000000000b220   168 OBJECT  GLOBAL DEFAULT   23
rte_crypto_auth_algorithm_strings@@DPDK_20.0
   158: 000000000000b220   168 OBJECT  GLOBAL DEFAULT   23
rte_crypto_auth_algorithm_strings

At runtime, the linker chooses to rewire all access to the final
binary data section, not the shared library local representation.

Now, if we update the array size, the shared library code is built
with the assumption of the increased size.
But at runtime with an "old" binary, the shared library code runs with
a shorter array, with potential out of bound access.

Interesting article:
https://developers.redhat.com/blog/2019/05/06/how-c-array-sizes-become-part-of-the-binary-interface-of-a-library/


-- 
David Marchand