On 3/14/2023 12:48 PM, Chengwen Feng wrote:
> The rte_kvargs_process() was used to parse KV pairs, it also supports
> to parse 'only keys' (e.g. socket_id) type. And the callback function
> parameter 'value' is NULL when parsed 'only keys'.
>
> This patch fixes segment fault when parse input args with 'only keys'.
>
> Fixes: 09c7e63a71f9 ("net/memif: introduce memory interface PMD")
> Fixes: 2f865ed07bb6 ("net/memif: use abstract socket address")
> Cc: stable@dpdk.org
>
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
<...>
> @@ -1889,6 +1913,9 @@ memif_set_mac(const char *key __rte_unused, const char *value, void *extra_args)
> {
> struct rte_ether_addr *ether_addr = (struct rte_ether_addr *)extra_args;
>
> + if (value == NULL || extra_args == NULL)
> + return -EINVAL;
> +
> if (rte_ether_unformat_addr(value, ether_addr) < 0)
> MIF_LOG(WARNING, "Failed to parse mac '%s'.", value);
Not related to the patch, but to record it,
it seems public 'rte_ether_unformat_addr()' API is missing parameter
verification, we need to fix it.
@@ -1720,6 +1720,9 @@ memif_set_role(const char *key __rte_unused, const char *value,
{
enum memif_role_t *role = (enum memif_role_t *)extra_args;
+ if (value == NULL || extra_args == NULL)
+ return -EINVAL;
+
if (strstr(value, "server") != NULL) {
*role = MEMIF_ROLE_SERVER;
} else if (strstr(value, "client") != NULL) {
@@ -1742,6 +1745,9 @@ memif_set_zc(const char *key __rte_unused, const char *value, void *extra_args)
{
uint32_t *flags = (uint32_t *)extra_args;
+ if (value == NULL || extra_args == NULL)
+ return -EINVAL;
+
if (strstr(value, "yes") != NULL) {
if (!rte_mcfg_get_single_file_segments()) {
MIF_LOG(ERR, "Zero-copy doesn't support multi-file segments.");
@@ -1762,6 +1768,9 @@ memif_set_id(const char *key __rte_unused, const char *value, void *extra_args)
{
memif_interface_id_t *id = (memif_interface_id_t *)extra_args;
+ if (value == NULL || extra_args == NULL)
+ return -EINVAL;
+
/* even if parsing fails, 0 is a valid id */
*id = strtoul(value, NULL, 10);
return 0;
@@ -1773,6 +1782,9 @@ memif_set_bs(const char *key __rte_unused, const char *value, void *extra_args)
unsigned long tmp;
uint16_t *pkt_buffer_size = (uint16_t *)extra_args;
+ if (value == NULL || extra_args == NULL)
+ return -EINVAL;
+
tmp = strtoul(value, NULL, 10);
if (tmp == 0 || tmp > 0xFFFF) {
MIF_LOG(ERR, "Invalid buffer size: %s.", value);
@@ -1789,6 +1801,9 @@ memif_set_rs(const char *key __rte_unused, const char *value, void *extra_args)
memif_log2_ring_size_t *log2_ring_size =
(memif_log2_ring_size_t *)extra_args;
+ if (value == NULL || extra_args == NULL)
+ return -EINVAL;
+
tmp = strtoul(value, NULL, 10);
if (tmp == 0 || tmp > ETH_MEMIF_MAX_LOG2_RING_SIZE) {
MIF_LOG(ERR, "Invalid ring size: %s (max %u).",
@@ -1840,6 +1855,9 @@ memif_set_socket_filename(const char *key __rte_unused, const char *value,
{
const char **socket_filename = (const char **)extra_args;
+ if (value == NULL || extra_args == NULL)
+ return -EINVAL;
+
*socket_filename = value;
return 0;
}
@@ -1849,6 +1867,9 @@ memif_set_is_socket_abstract(const char *key __rte_unused, const char *value, vo
{
uint32_t *flags = (uint32_t *)extra_args;
+ if (value == NULL || extra_args == NULL)
+ return -EINVAL;
+
if (strstr(value, "yes") != NULL) {
*flags |= ETH_MEMIF_FLAG_SOCKET_ABSTRACT;
} else if (strstr(value, "no") != NULL) {
@@ -1870,6 +1891,9 @@ memif_set_owner(const char *key, const char *value, void *extra_args)
char *end = NULL;
uint32_t *id = (uint32_t *)extra_args;
+ if (value == NULL || extra_args == NULL)
+ return -EINVAL;
+
val = strtoul(value, &end, 10);
if (*value == '\0' || *end != '\0') {
MIF_LOG(ERR, "Failed to parse %s: %s.", key, value);
@@ -1889,6 +1913,9 @@ memif_set_mac(const char *key __rte_unused, const char *value, void *extra_args)
{
struct rte_ether_addr *ether_addr = (struct rte_ether_addr *)extra_args;
+ if (value == NULL || extra_args == NULL)
+ return -EINVAL;
+
if (rte_ether_unformat_addr(value, ether_addr) < 0)
MIF_LOG(WARNING, "Failed to parse mac '%s'.", value);
return 0;
@@ -1899,6 +1926,9 @@ memif_set_secret(const char *key __rte_unused, const char *value, void *extra_ar
{
const char **secret = (const char **)extra_args;
+ if (value == NULL || extra_args == NULL)
+ return -EINVAL;
+
*secret = value;
return 0;
}