diff mbox series

[v2,1/2] crypto/ccp: sha3 support enabling in ccp

Message ID 20200122105222.3758-1-ssardar@amd.com (mailing list archive)
State Changes Requested, archived
Delegated to: akhil goyal
Headers
Series [v2,1/2] crypto/ccp: sha3 support enabling in ccp |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

ssardar@amd.com Jan. 22, 2020, 10:52 a.m. UTC 
  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>
---
 lib/librte_cryptodev/rte_cryptodev.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Kumar, Ravi1 Jan. 22, 2020, 11:11 a.m. UTC | #1 
[AMD Official Use Only - Internal Distribution Only]

For series,
Acked-by: Ravi Kumar <ravi1.kumar@amd.com>

Regards,
Ravi

>-----Original Message-----
>From: Sardar, Shamsher singh <Shamshersingh.Sardar@amd.com> 
>Sent: Wednesday, January 22, 2020 4:22 PM
>To: dev@dpdk.org
>Cc: Kumar, Ravi1 <Ravi1.Kumar@amd.com>
>Subject: [PATCH v2 1/2] crypto/ccp: sha3 support enabling in ccp
>
>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>
>---
> lib/librte_cryptodev/rte_cryptodev.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
>diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
>index 5c6359b5c..6ee4f1086 100644
>--- a/lib/librte_cryptodev/rte_cryptodev.c
>+++ b/lib/librte_cryptodev/rte_cryptodev.c
>@@ -140,7 +140,16 @@ rte_crypto_auth_algorithm_strings[] = {
> 
> 	[RTE_CRYPTO_AUTH_KASUMI_F9]	= "kasumi-f9",
> 	[RTE_CRYPTO_AUTH_SNOW3G_UIA2]	= "snow3g-uia2",
>-	[RTE_CRYPTO_AUTH_ZUC_EIA3]	= "zuc-eia3"
>+	[RTE_CRYPTO_AUTH_ZUC_EIA3]	= "zuc-eia3",
>+
>+	[RTE_CRYPTO_AUTH_SHA3_224]	= "sha3-224",
>+	[RTE_CRYPTO_AUTH_SHA3_224_HMAC]	= "sha3-224-hmac",
>+	[RTE_CRYPTO_AUTH_SHA3_256]	= "sha3-256",
>+	[RTE_CRYPTO_AUTH_SHA3_256_HMAC]	= "sha3-256-hmac",
>+	[RTE_CRYPTO_AUTH_SHA3_384]	= "sha3-384",
>+	[RTE_CRYPTO_AUTH_SHA3_384_HMAC]	= "sha3-384-hmac",
>+	[RTE_CRYPTO_AUTH_SHA3_512]	= "sha3-512",
>+	[RTE_CRYPTO_AUTH_SHA3_512_HMAC]	= "sha3-512-hmac"
> };
> 
> /**
>-- 
>2.17.1
>
Akhil Goyal Feb. 5, 2020, 12:24 p.m. UTC | #2 
> 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

Thanks.
Thomas Monjalon Feb. 5, 2020, 10:22 p.m. UTC | #3 
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

I don't understand how the size of this array can impact the application,
but I prefer to stay on the safe side for now.

Please let's discuss this patch in 20.02-rc3 timeframe.
David Marchand Feb. 6, 2020, 9:34 a.m. UTC | #4 
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/
diff mbox series

Patch

diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
index 5c6359b5c..6ee4f1086 100644
--- a/lib/librte_cryptodev/rte_cryptodev.c
+++ b/lib/librte_cryptodev/rte_cryptodev.c
@@ -140,7 +140,16 @@  rte_crypto_auth_algorithm_strings[] = {
 
 	[RTE_CRYPTO_AUTH_KASUMI_F9]	= "kasumi-f9",
 	[RTE_CRYPTO_AUTH_SNOW3G_UIA2]	= "snow3g-uia2",
-	[RTE_CRYPTO_AUTH_ZUC_EIA3]	= "zuc-eia3"
+	[RTE_CRYPTO_AUTH_ZUC_EIA3]	= "zuc-eia3",
+
+	[RTE_CRYPTO_AUTH_SHA3_224]	= "sha3-224",
+	[RTE_CRYPTO_AUTH_SHA3_224_HMAC]	= "sha3-224-hmac",
+	[RTE_CRYPTO_AUTH_SHA3_256]	= "sha3-256",
+	[RTE_CRYPTO_AUTH_SHA3_256_HMAC]	= "sha3-256-hmac",
+	[RTE_CRYPTO_AUTH_SHA3_384]	= "sha3-384",
+	[RTE_CRYPTO_AUTH_SHA3_384_HMAC]	= "sha3-384-hmac",
+	[RTE_CRYPTO_AUTH_SHA3_512]	= "sha3-512",
+	[RTE_CRYPTO_AUTH_SHA3_512_HMAC]	= "sha3-512-hmac"
 };
 
 /**