* [PATCH v2] cryptodev: avoid algorithm strings null pointers @ 2023-06-08 8:03 xixin.liu 2023-06-09 6:46 ` [EXT] " Akhil Goyal 0 siblings, 1 reply; 5+ messages in thread From: xixin.liu @ 2023-06-08 8:03 UTC (permalink / raw) To: dev; +Cc: gakhil, liuxixin2020 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [EXT] [PATCH v2] cryptodev: avoid algorithm strings null pointers 2023-06-08 8:03 [PATCH v2] cryptodev: avoid algorithm strings null pointers xixin.liu @ 2023-06-09 6:46 ` Akhil Goyal 2023-06-09 7:00 ` 答复: " liu xixin 0 siblings, 1 reply; 5+ messages in thread From: Akhil Goyal @ 2023-06-09 6:46 UTC (permalink / raw) To: xixin.liu, dev > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* 答复: [EXT] [PATCH v2] cryptodev: avoid algorithm strings null pointers 2023-06-09 6:46 ` [EXT] " Akhil Goyal @ 2023-06-09 7:00 ` liu xixin 2023-06-09 8:06 ` Akhil Goyal 0 siblings, 1 reply; 5+ messages in thread From: liu xixin @ 2023-06-09 7:00 UTC (permalink / raw) To: Akhil Goyal, dev > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [EXT] [PATCH v2] cryptodev: avoid algorithm strings null pointers 2023-06-09 7:00 ` 答复: " liu xixin @ 2023-06-09 8:06 ` Akhil Goyal [not found] ` <TY3PR01MB1157507D3557958548014534EC251A@TY3PR01MB11575.jpnprd01.prod.outlook.com> 0 siblings, 1 reply; 5+ messages in thread From: Akhil Goyal @ 2023-06-09 8:06 UTC (permalink / raw) To: liu xixin, dev > -----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. ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <TY3PR01MB1157507D3557958548014534EC251A@TY3PR01MB11575.jpnprd01.prod.outlook.com>]
* RE: [EXT] [PATCH v2] cryptodev: avoid algorithm strings null pointers [not found] ` <TY3PR01MB1157507D3557958548014534EC251A@TY3PR01MB11575.jpnprd01.prod.outlook.com> @ 2023-06-09 8:30 ` Akhil Goyal 0 siblings, 0 replies; 5+ messages in thread From: Akhil Goyal @ 2023-06-09 8:30 UTC (permalink / raw) To: liu xixin, dev [-- 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 --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-09 8:30 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-08 8:03 [PATCH v2] cryptodev: avoid algorithm strings null pointers xixin.liu
2023-06-09 6:46 ` [EXT] " Akhil Goyal
2023-06-09 7:00 ` 答复: " liu xixin
2023-06-09 8:06 ` Akhil Goyal
[not found] ` <TY3PR01MB1157507D3557958548014534EC251A@TY3PR01MB11575.jpnprd01.prod.outlook.com>
2023-06-09 8:30 ` Akhil Goyal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).