[PATCH v2] cryptodev: avoid algorithm strings null pointers

[PATCH v2] cryptodev: avoid algorithm strings null pointers

[xixin.liu]

The crypto algorithm strings identifiers that are Continuous may be null,
so there is needed to add null judgment.
When testing with dpdk-test-crypto-perf and passing in the parameter
--auth-algo sm3-hmac, The program caused a segfault due to a null pointer
passed in by strcmp.
Adding this patch can solve the segfault problem.

Signed-off-by: xixin.liu <liuxixin2020@outlook.com>
---
 lib/cryptodev/rte_cryptodev.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
index a96114b2da..41c23fc596 100644
--- a/lib/cryptodev/rte_cryptodev.c
+++ b/lib/cryptodev/rte_cryptodev.c
@@ -346,6 +346,8 @@ rte_cryptodev_get_cipher_algo_enum(enum rte_crypto_cipher_algorithm *algo_enum,
 	int ret = -1;	/* Invalid string */
 
 	for (i = 1; i < RTE_DIM(crypto_cipher_algorithm_strings); i++) {
+		if (crypto_cipher_algorithm_strings[i] == NULL)
+			continue;
 		if (strcmp(algo_string, crypto_cipher_algorithm_strings[i]) == 0) {
 			*algo_enum = (enum rte_crypto_cipher_algorithm) i;
 			ret = 0;
@@ -366,6 +368,8 @@ rte_cryptodev_get_auth_algo_enum(enum rte_crypto_auth_algorithm *algo_enum,
 	int ret = -1;	/* Invalid string */
 
 	for (i = 1; i < RTE_DIM(crypto_auth_algorithm_strings); i++) {
+		if (crypto_auth_algorithm_strings[i] == NULL)
+			continue;
 		if (strcmp(algo_string, crypto_auth_algorithm_strings[i]) == 0) {
 			*algo_enum = (enum rte_crypto_auth_algorithm) i;
 			ret = 0;
@@ -386,6 +390,8 @@ rte_cryptodev_get_aead_algo_enum(enum rte_crypto_aead_algorithm *algo_enum,
 	int ret = -1;	/* Invalid string */
 
 	for (i = 1; i < RTE_DIM(crypto_aead_algorithm_strings); i++) {
+		if (crypto_aead_algorithm_strings[i] == NULL)
+			continue;
 		if (strcmp(algo_string, crypto_aead_algorithm_strings[i]) == 0) {
 			*algo_enum = (enum rte_crypto_aead_algorithm) i;
 			ret = 0;
@@ -406,6 +412,8 @@ rte_cryptodev_asym_get_xform_enum(enum rte_crypto_asym_xform_type *xform_enum,
 	int ret = -1;	/* Invalid string */
 
 	for (i = 1; i < RTE_DIM(crypto_asym_xform_strings); i++) {
+		if (crypto_asym_xform_strings[i] == NULL)
+			continue;
 		if (strcmp(xform_string,
 			crypto_asym_xform_strings[i]) == 0) {
 			*xform_enum = (enum rte_crypto_asym_xform_type) i;
-- 
2.34.1

RE: [EXT] [PATCH v2] cryptodev: avoid algorithm strings null pointers

[Akhil Goyal]

> Subject: [EXT] [PATCH v2] cryptodev: avoid algorithm strings null pointers
> 
> The crypto algorithm strings identifiers that are Continuous may be null,
> so there is needed to add null judgment.
> When testing with dpdk-test-crypto-perf and passing in the parameter
> --auth-algo sm3-hmac, The program caused a segfault due to a null pointer
> passed in by strcmp.
> Adding this patch can solve the segfault problem.

I believe this is a fix and you should add fixes tag for this and need to be backported.

> 
> Signed-off-by: xixin.liu <liuxixin2020@outlook.com>

Signoff format is not correct.
Please follow https://doc.dpdk.org/guides/contributing/patches.html


> ---
>  lib/cryptodev/rte_cryptodev.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.c
> index a96114b2da..41c23fc596 100644
> --- a/lib/cryptodev/rte_cryptodev.c
> +++ b/lib/cryptodev/rte_cryptodev.c
> @@ -346,6 +346,8 @@ rte_cryptodev_get_cipher_algo_enum(enum
> rte_crypto_cipher_algorithm *algo_enum,
>  	int ret = -1;	/* Invalid string */
> 
>  	for (i = 1; i < RTE_DIM(crypto_cipher_algorithm_strings); i++) {
> +		if (crypto_cipher_algorithm_strings[i] == NULL)
> +			continue;

crypto_cipher_algorithm_strings is a fixed size array with all non-NULL known values
and the for loop is iterating over it. So, this check does not make sense to me.


>  		if (strcmp(algo_string, crypto_cipher_algorithm_strings[i]) == 0)
> {
>  			*algo_enum = (enum rte_crypto_cipher_algorithm) i;
>  			ret = 0;
> @@ -366,6 +368,8 @@ rte_cryptodev_get_auth_algo_enum(enum
> rte_crypto_auth_algorithm *algo_enum,
>  	int ret = -1;	/* Invalid string */
> 
>  	for (i = 1; i < RTE_DIM(crypto_auth_algorithm_strings); i++) {
> +		if (crypto_auth_algorithm_strings[i] == NULL)
> +			continue;
>  		if (strcmp(algo_string, crypto_auth_algorithm_strings[i]) == 0) {
>  			*algo_enum = (enum rte_crypto_auth_algorithm) i;
>  			ret = 0;
> @@ -386,6 +390,8 @@ rte_cryptodev_get_aead_algo_enum(enum
> rte_crypto_aead_algorithm *algo_enum,
>  	int ret = -1;	/* Invalid string */
> 
>  	for (i = 1; i < RTE_DIM(crypto_aead_algorithm_strings); i++) {
> +		if (crypto_aead_algorithm_strings[i] == NULL)
> +			continue;
>  		if (strcmp(algo_string, crypto_aead_algorithm_strings[i]) == 0) {
>  			*algo_enum = (enum rte_crypto_aead_algorithm) i;
>  			ret = 0;
> @@ -406,6 +412,8 @@ rte_cryptodev_asym_get_xform_enum(enum
> rte_crypto_asym_xform_type *xform_enum,
>  	int ret = -1;	/* Invalid string */
> 
>  	for (i = 1; i < RTE_DIM(crypto_asym_xform_strings); i++) {
> +		if (crypto_asym_xform_strings[i] == NULL)
> +			continue;
>  		if (strcmp(xform_string,
>  			crypto_asym_xform_strings[i]) == 0) {
>  			*xform_enum = (enum rte_crypto_asym_xform_type) i;
> --
> 2.34.1

答复: [EXT] [PATCH v2] cryptodev: avoid algorithm strings null pointers

[liu xixin]

> Subject: [EXT] [PATCH v2] cryptodev: avoid algorithm strings null 
> pointers
> 
> The crypto algorithm strings identifiers that are Continuous may be 
> null, so there is needed to add null judgment.
> When testing with dpdk-test-crypto-perf and passing in the parameter 
> --auth-algo sm3-hmac, The program caused a segfault due to a null 
> pointer passed in by strcmp.
> Adding this patch can solve the segfault problem.

I believe this is a fix and you should add fixes tag for this and need to be backported.

> 
> Signed-off-by: xixin.liu <liuxixin2020@outlook.com>

Signoff format is not correct.
Please follow https://doc.dpdk.org/guides/contributing/patches.html


> ---
>  lib/cryptodev/rte_cryptodev.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/lib/cryptodev/rte_cryptodev.c 
> b/lib/cryptodev/rte_cryptodev.c index a96114b2da..41c23fc596 100644
> --- a/lib/cryptodev/rte_cryptodev.c
> +++ b/lib/cryptodev/rte_cryptodev.c
> @@ -346,6 +346,8 @@ rte_cryptodev_get_cipher_algo_enum(enum
> rte_crypto_cipher_algorithm *algo_enum,
>  	int ret = -1;	/* Invalid string */
> 
>  	for (i = 1; i < RTE_DIM(crypto_cipher_algorithm_strings); i++) {
> +		if (crypto_cipher_algorithm_strings[i] == NULL)
> +			continue;

crypto_cipher_algorithm_strings is a fixed size array with all non-NULL known values and the for loop is iterating over it. So, this check does not make sense to me.
----> Not every element of the array is defined, eg. it is NULL that the first element [1],  if not check "strcmp(algo_string, crypto_cipher_algorithm_strings[i]" will fail

>  		if (strcmp(algo_string, crypto_cipher_algorithm_strings[i]) == 0) {
>  			*algo_enum = (enum rte_crypto_cipher_algorithm) i;
>  			ret = 0;
> @@ -366,6 +368,8 @@ rte_cryptodev_get_auth_algo_enum(enum
> rte_crypto_auth_algorithm *algo_enum,
>  	int ret = -1;	/* Invalid string */
> 
>  	for (i = 1; i < RTE_DIM(crypto_auth_algorithm_strings); i++) {
> +		if (crypto_auth_algorithm_strings[i] == NULL)
> +			continue;
>  		if (strcmp(algo_string, crypto_auth_algorithm_strings[i]) == 0) {
>  			*algo_enum = (enum rte_crypto_auth_algorithm) i;
>  			ret = 0;
> @@ -386,6 +390,8 @@ rte_cryptodev_get_aead_algo_enum(enum
> rte_crypto_aead_algorithm *algo_enum,
>  	int ret = -1;	/* Invalid string */
> 
>  	for (i = 1; i < RTE_DIM(crypto_aead_algorithm_strings); i++) {
> +		if (crypto_aead_algorithm_strings[i] == NULL)
> +			continue;
>  		if (strcmp(algo_string, crypto_aead_algorithm_strings[i]) == 0) {
>  			*algo_enum = (enum rte_crypto_aead_algorithm) i;
>  			ret = 0;
> @@ -406,6 +412,8 @@ rte_cryptodev_asym_get_xform_enum(enum
> rte_crypto_asym_xform_type *xform_enum,
>  	int ret = -1;	/* Invalid string */
> 
>  	for (i = 1; i < RTE_DIM(crypto_asym_xform_strings); i++) {
> +		if (crypto_asym_xform_strings[i] == NULL)
> +			continue;
>  		if (strcmp(xform_string,
>  			crypto_asym_xform_strings[i]) == 0) {
>  			*xform_enum = (enum rte_crypto_asym_xform_type) i;
> --
> 2.34.1

RE: [EXT] [PATCH v2] cryptodev: avoid algorithm strings null pointers

[Akhil Goyal]

> -----Original Message-----
> From: liu xixin <liuxixin2020@outlook.com>
> Sent: Friday, June 9, 2023 12:31 PM
> To: Akhil Goyal <gakhil@marvell.com>; dev@dpdk.org
> Subject: 答复: [EXT] [PATCH v2] cryptodev: avoid algorithm strings null pointers
> 
> > Subject: [EXT] [PATCH v2] cryptodev: avoid algorithm strings null
> > pointers
> >
> > The crypto algorithm strings identifiers that are Continuous may be
> > null, so there is needed to add null judgment.
> > When testing with dpdk-test-crypto-perf and passing in the parameter
> > --auth-algo sm3-hmac, The program caused a segfault due to a null
> > pointer passed in by strcmp.
> > Adding this patch can solve the segfault problem.
> 
> I believe this is a fix and you should add fixes tag for this and need to be
> backported.
> 
> >
> > Signed-off-by: xixin.liu <liuxixin2020@outlook.com>
> 
> Signoff format is not correct.
> Please follow https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__doc.dpdk.org_guides_contributing_patches.html&d=DwIGoQ&c=nKjWec2b
> 6R0mOyPaz7xtfQ&r=DnL7Si2wl_PRwpZ9TWey3eu68gBzn7DkPwuqhd6WNyo&m
> =6mzvm07Go5-
> S_2jVk9KRF1mOl0Juay2Sa2WI2XiNgTbg6ZhMcm75GNceSVhe0Doj&s=tvqhxvElc
> m_8h3e7YIBym6IAHk6BxUAFx2RKmjJ6Ibw&e=
> 
> 
> > ---
> >  lib/cryptodev/rte_cryptodev.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/lib/cryptodev/rte_cryptodev.c
> > b/lib/cryptodev/rte_cryptodev.c index a96114b2da..41c23fc596 100644
> > --- a/lib/cryptodev/rte_cryptodev.c
> > +++ b/lib/cryptodev/rte_cryptodev.c
> > @@ -346,6 +346,8 @@ rte_cryptodev_get_cipher_algo_enum(enum
> > rte_crypto_cipher_algorithm *algo_enum,
> >  	int ret = -1;	/* Invalid string */
> >
> >  	for (i = 1; i < RTE_DIM(crypto_cipher_algorithm_strings); i++) {
> > +		if (crypto_cipher_algorithm_strings[i] == NULL)
> > +			continue;
> 
> crypto_cipher_algorithm_strings is a fixed size array with all non-NULL known
> values and the for loop is iterating over it. So, this check does not make sense to
> me.
> ----> Not every element of the array is defined, eg. it is NULL that the first
> element [1],  if not check "strcmp(algo_string,
> crypto_cipher_algorithm_strings[i]" will fail

The loop starts from 1 so 0th element = NULL will not matter.
And for all other values it is not null. If something is missing,
then it can be added.
But this check is not needed.

RE: [EXT] [PATCH v2] cryptodev: avoid algorithm strings null pointers

[Akhil Goyal]

[-- Attachment #1: Type: text/plain, Size: 4861 bytes --]

Please reply only in plain text format.

All the values are added in the array but are jumbled up.
But still it will point to correct position in the array as we are using enum value.
Eg. RTE_CRYPTO_CIPHER_NULL = 1,
And in the array it is there as highlighted below.
Hence there is no hole present in the array.


static const char *
crypto_cipher_algorithm_strings[] = {
        [RTE_CRYPTO_CIPHER_3DES_CBC]    = "3des-cbc",
        [RTE_CRYPTO_CIPHER_3DES_ECB]    = "3des-ecb",
        [RTE_CRYPTO_CIPHER_3DES_CTR]    = "3des-ctr",

        [RTE_CRYPTO_CIPHER_AES_CBC]     = "aes-cbc",
        [RTE_CRYPTO_CIPHER_AES_CTR]     = "aes-ctr",
        [RTE_CRYPTO_CIPHER_AES_DOCSISBPI]       = "aes-docsisbpi",
        [RTE_CRYPTO_CIPHER_AES_ECB]     = "aes-ecb",
        [RTE_CRYPTO_CIPHER_AES_F8]      = "aes-f8",
        [RTE_CRYPTO_CIPHER_AES_XTS]     = "aes-xts",

        [RTE_CRYPTO_CIPHER_ARC4]        = "arc4",

        [RTE_CRYPTO_CIPHER_DES_CBC]     = "des-cbc",
        [RTE_CRYPTO_CIPHER_DES_DOCSISBPI]       = "des-docsisbpi",

        [RTE_CRYPTO_CIPHER_NULL]        = "null",             ----------------------------------------->

        [RTE_CRYPTO_CIPHER_KASUMI_F8]   = "kasumi-f8",
        [RTE_CRYPTO_CIPHER_SNOW3G_UEA2] = "snow3g-uea2",
        [RTE_CRYPTO_CIPHER_ZUC_EEA3]    = "zuc-eea3",
        [RTE_CRYPTO_CIPHER_SM4_ECB]     = "sm4-ecb",
        [RTE_CRYPTO_CIPHER_SM4_CBC]     = "sm4-cbc",
        [RTE_CRYPTO_CIPHER_SM4_CTR]     = "sm4-ctr",
        [RTE_CRYPTO_CIPHER_SM4_CFB]     = "sm4-cfb",
        [RTE_CRYPTO_CIPHER_SM4_OFB]     = "sm4-ofb"
};


From: liu xixin <liuxixin2020@outlook.com>
Sent: Friday, June 9, 2023 1:45 PM
To: Akhil Goyal <gakhil@marvell.com>; dev@dpdk.org
Subject: 回复: [EXT] [PATCH v2] cryptodev: avoid algorithm strings null pointers

> -----Original Message-----
> From: liu xixin <liuxixin2020@outlook.com<mailto:liuxixin2020@outlook.com>>
> Sent: Friday, June 9, 2023 12:31 PM
> To: Akhil Goyal <gakhil@marvell.com<mailto:gakhil@marvell.com>>; dev@dpdk.org<mailto:dev@dpdk.org>
> Subject: 答复: [EXT] [PATCH v2] cryptodev: avoid algorithm strings null pointers
>
> > Subject: [EXT] [PATCH v2] cryptodev: avoid algorithm strings null
> > pointers
> >
> > The crypto algorithm strings identifiers that are Continuous may be
> > null, so there is needed to add null judgment.
> > When testing with dpdk-test-crypto-perf and passing in the parameter
> > --auth-algo sm3-hmac, The program caused a segfault due to a null
> > pointer passed in by strcmp.
> > Adding this patch can solve the segfault problem.
>
> I believe this is a fix and you should add fixes tag for this and need to be
> backported.
>
> >
> > Signed-off-by: xixin.liu <liuxixin2020@outlook.com<mailto:liuxixin2020@outlook.com>>
>
> Signoff format is not correct.
> Please follow https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__doc.dpdk.org_guides_contributing_patches.html&d=DwIGoQ&c=nKjWec2b
> 6R0mOyPaz7xtfQ&r=DnL7Si2wl_PRwpZ9TWey3eu68gBzn7DkPwuqhd6WNyo&m
> =6mzvm07Go5-
> S_2jVk9KRF1mOl0Juay2Sa2WI2XiNgTbg6ZhMcm75GNceSVhe0Doj&s=tvqhxvElc
> m_8h3e7YIBym6IAHk6BxUAFx2RKmjJ6Ibw&e=
>
>
> > ---
> >  lib/cryptodev/rte_cryptodev.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/lib/cryptodev/rte_cryptodev.c
> > b/lib/cryptodev/rte_cryptodev.c index a96114b2da..41c23fc596 100644
> > --- a/lib/cryptodev/rte_cryptodev.c
> > +++ b/lib/cryptodev/rte_cryptodev.c
> > @@ -346,6 +346,8 @@ rte_cryptodev_get_cipher_algo_enum(enum
> > rte_crypto_cipher_algorithm *algo_enum,
> >      int ret = -1;   /* Invalid string */
> >
> >      for (i = 1; i < RTE_DIM(crypto_cipher_algorithm_strings); i++) {
> > +           if (crypto_cipher_algorithm_strings[i] == NULL)
> > +                   continue;
>
> crypto_cipher_algorithm_strings is a fixed size array with all non-NULL known
> values and the for loop is iterating over it. So, this check does not make sense to
> me.
> ----> Not every element of the array is defined, eg. it is NULL that the first
> element [1],  if not check "strcmp(algo_string,
> crypto_cipher_algorithm_strings[i]" will fail

The loop starts from 1 so 0th element = NULL will not matter.

And for all other values it is not null. If something is missing,
then it can be added.
But this check is not needed.
---> [0] is no need to check 。However, the values of all subsequent elements are checked to be null
static const char *
crypto_cipher_algorithm_strings[] = {
    [RTE_CRYPTO_CIPHER_3DES_CBC]    = "3des-cbc",  //this RTE_CRYPTO_CIPHER_3DES_CBC == 2，so 【1】already is NULL
    [RTE_CRYPTO_CIPHER_3DES_ECB]    = "3des-ecb", // same as RTE_CRYPTO_CIPHER_3DES_ECB == 4 ，and now 【1】【3】are all NULL
    [RTE_CRYPTO_CIPHER_3DES_CTR]    = "3des-ctr",  // and so on
...
};

[-- Attachment #2: Type: text/html, Size: 24332 bytes --]