* [PATCH] net/ice: fix possible memory leak
@ 2024-07-11 16:59 Vladimir Medvedkin
2024-07-11 17:05 ` Bruce Richardson
2024-07-15 18:04 ` [PATCH v2 1/3] " Vladimir Medvedkin
0 siblings, 2 replies; 26+ messages in thread
From: Vladimir Medvedkin @ 2024-07-11 16:59 UTC (permalink / raw)
To: dev; +Cc: bruce.richardson, ting.xu, Michael Theodore Stolarchuk
This patch fixes possible memleak inside the
ice_hash_parse_raw_pattern().
Additionally replaces using strlen() with more secure strnlen().
Also replaces the returned inconsistent rte_errno
with explicit error statuses.
Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow offloading in RSS")
Cc: ting.xu@intel.com
Cc:stable@dpdk.org
Reported-by: Michael Theodore Stolarchuk <mike.stolarchuk@arista.com>
Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com>
---
drivers/net/ice/ice_hash.c | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c
index f923641533..a7dbb54d91 100644
--- a/drivers/net/ice/ice_hash.c
+++ b/drivers/net/ice/ice_hash.c
@@ -650,18 +650,18 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad,
uint8_t *pkt_buf, *msk_buf;
uint8_t tmp_val = 0;
uint8_t tmp_c = 0;
- int i, j;
+ int i, j, ret = 0;
if (ad->psr == NULL)
- return -rte_errno;
+ return -ENOTSUP;
raw_spec = item->spec;
raw_mask = item->mask;
- spec_len = strlen((char *)(uintptr_t)raw_spec->pattern);
- if (strlen((char *)(uintptr_t)raw_mask->pattern) !=
- spec_len)
- return -rte_errno;
+ spec_len = strnlen((char *)(uintptr_t)raw_spec->pattern, raw_spec->length);
+ if (strnlen((char *)(uintptr_t)raw_mask->pattern, raw_mask->length) !=
+ spec_len)
+ return -EINVAL;
pkt_len = spec_len / 2;
@@ -670,8 +670,10 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad,
return -ENOMEM;
msk_buf = rte_zmalloc(NULL, pkt_len, 0);
- if (!msk_buf)
+ if (!msk_buf) {
+ rte_free(pkt_buf);
return -ENOMEM;
+ }
/* convert string to int array */
for (i = 0, j = 0; i < spec_len; i += 2, j++) {
@@ -708,18 +710,22 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad,
msk_buf[j] = tmp_val * 16 + tmp_c - '0';
}
- if (ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt))
- return -rte_errno;
+ ret = ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt);
+ if (ret)
+ goto free_mem;
- if (ice_parser_profile_init(&rslt, pkt_buf, msk_buf,
- pkt_len, ICE_BLK_RSS, true, &prof))
- return -rte_errno;
+ ret = ice_parser_profile_init(&rslt, pkt_buf, msk_buf,
+ pkt_len, ICE_BLK_RSS, true, &prof);
+ if (ret)
+ goto free_mem;
rte_memcpy(&meta->raw.prof, &prof, sizeof(prof));
+free_mem:
rte_free(pkt_buf);
rte_free(msk_buf);
- return 0;
+
+ return ret;
}
static void
--
2.34.1
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH] net/ice: fix possible memory leak 2024-07-11 16:59 [PATCH] net/ice: fix possible memory leak Vladimir Medvedkin @ 2024-07-11 17:05 ` Bruce Richardson 2024-07-11 17:07 ` Medvedkin, Vladimir 2024-07-15 18:04 ` [PATCH v2 1/3] " Vladimir Medvedkin 1 sibling, 1 reply; 26+ messages in thread From: Bruce Richardson @ 2024-07-11 17:05 UTC (permalink / raw) To: Vladimir Medvedkin; +Cc: dev, ting.xu, Michael Theodore Stolarchuk On Thu, Jul 11, 2024 at 04:59:07PM +0000, Vladimir Medvedkin wrote: > This patch fixes possible memleak inside the > ice_hash_parse_raw_pattern(). > Additionally replaces using strlen() with more secure strnlen(). > Also replaces the returned inconsistent rte_errno > with explicit error statuses. > Hi Vladimir, I think you could do with explaining more about how the memory leak comes about. It's also not helped by having multiple additional changes in this patch, can they be split off into a separate patch to fix the error codes (and change the strlen in passing), or even two additional patches for the error code and strlen issues individually. /Bruce. > Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow offloading in RSS") > Cc: ting.xu@intel.com > Cc:stable@dpdk.org > > Reported-by: Michael Theodore Stolarchuk <mike.stolarchuk@arista.com> > Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> > --- > drivers/net/ice/ice_hash.c | 32 +++++++++++++++++++------------- > 1 file changed, 19 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c > index f923641533..a7dbb54d91 100644 > --- a/drivers/net/ice/ice_hash.c > +++ b/drivers/net/ice/ice_hash.c > @@ -650,18 +650,18 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, > uint8_t *pkt_buf, *msk_buf; > uint8_t tmp_val = 0; > uint8_t tmp_c = 0; > - int i, j; > + int i, j, ret = 0; > > if (ad->psr == NULL) > - return -rte_errno; > + return -ENOTSUP; > > raw_spec = item->spec; > raw_mask = item->mask; > > - spec_len = strlen((char *)(uintptr_t)raw_spec->pattern); > - if (strlen((char *)(uintptr_t)raw_mask->pattern) != > - spec_len) > - return -rte_errno; > + spec_len = strnlen((char *)(uintptr_t)raw_spec->pattern, raw_spec->length); > + if (strnlen((char *)(uintptr_t)raw_mask->pattern, raw_mask->length) != > + spec_len) > + return -EINVAL; > > pkt_len = spec_len / 2; > > @@ -670,8 +670,10 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, > return -ENOMEM; > > msk_buf = rte_zmalloc(NULL, pkt_len, 0); > - if (!msk_buf) > + if (!msk_buf) { > + rte_free(pkt_buf); > return -ENOMEM; > + } > > /* convert string to int array */ > for (i = 0, j = 0; i < spec_len; i += 2, j++) { > @@ -708,18 +710,22 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, > msk_buf[j] = tmp_val * 16 + tmp_c - '0'; > } > > - if (ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt)) > - return -rte_errno; > + ret = ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt); > + if (ret) > + goto free_mem; > > - if (ice_parser_profile_init(&rslt, pkt_buf, msk_buf, > - pkt_len, ICE_BLK_RSS, true, &prof)) > - return -rte_errno; > + ret = ice_parser_profile_init(&rslt, pkt_buf, msk_buf, > + pkt_len, ICE_BLK_RSS, true, &prof); > + if (ret) > + goto free_mem; > > rte_memcpy(&meta->raw.prof, &prof, sizeof(prof)); > > +free_mem: > rte_free(pkt_buf); > rte_free(msk_buf); > - return 0; > + > + return ret; > } > > static void > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH] net/ice: fix possible memory leak 2024-07-11 17:05 ` Bruce Richardson @ 2024-07-11 17:07 ` Medvedkin, Vladimir 0 siblings, 0 replies; 26+ messages in thread From: Medvedkin, Vladimir @ 2024-07-11 17:07 UTC (permalink / raw) To: Richardson, Bruce; +Cc: dev, Stolarchuk, Michael Hi Bruce, Sure, will submit v2. Thanks! -----Original Message----- From: Richardson, Bruce <bruce.richardson@intel.com> Sent: Thursday, July 11, 2024 6:05 PM To: Medvedkin, Vladimir <vladimir.medvedkin@intel.com> Cc: dev@dpdk.org; ting.xu@intel.com; Stolarchuk, Michael <mike.stolarchuk@arista.com> Subject: Re: [PATCH] net/ice: fix possible memory leak On Thu, Jul 11, 2024 at 04:59:07PM +0000, Vladimir Medvedkin wrote: > This patch fixes possible memleak inside the > ice_hash_parse_raw_pattern(). > Additionally replaces using strlen() with more secure strnlen(). > Also replaces the returned inconsistent rte_errno with explicit error > statuses. > Hi Vladimir, I think you could do with explaining more about how the memory leak comes about. It's also not helped by having multiple additional changes in this patch, can they be split off into a separate patch to fix the error codes (and change the strlen in passing), or even two additional patches for the error code and strlen issues individually. /Bruce. > Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow > offloading in RSS") > Cc: ting.xu@intel.com > Cc:stable@dpdk.org > > Reported-by: Michael Theodore Stolarchuk <mike.stolarchuk@arista.com> > Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> > --- > drivers/net/ice/ice_hash.c | 32 +++++++++++++++++++------------- > 1 file changed, 19 insertions(+), 13 deletions(-) > > diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c > index f923641533..a7dbb54d91 100644 > --- a/drivers/net/ice/ice_hash.c > +++ b/drivers/net/ice/ice_hash.c > @@ -650,18 +650,18 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, > uint8_t *pkt_buf, *msk_buf; > uint8_t tmp_val = 0; > uint8_t tmp_c = 0; > - int i, j; > + int i, j, ret = 0; > > if (ad->psr == NULL) > - return -rte_errno; > + return -ENOTSUP; > > raw_spec = item->spec; > raw_mask = item->mask; > > - spec_len = strlen((char *)(uintptr_t)raw_spec->pattern); > - if (strlen((char *)(uintptr_t)raw_mask->pattern) != > - spec_len) > - return -rte_errno; > + spec_len = strnlen((char *)(uintptr_t)raw_spec->pattern, raw_spec->length); > + if (strnlen((char *)(uintptr_t)raw_mask->pattern, raw_mask->length) != > + spec_len) > + return -EINVAL; > > pkt_len = spec_len / 2; > > @@ -670,8 +670,10 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, > return -ENOMEM; > > msk_buf = rte_zmalloc(NULL, pkt_len, 0); > - if (!msk_buf) > + if (!msk_buf) { > + rte_free(pkt_buf); > return -ENOMEM; > + } > > /* convert string to int array */ > for (i = 0, j = 0; i < spec_len; i += 2, j++) { @@ -708,18 +710,22 > @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, > msk_buf[j] = tmp_val * 16 + tmp_c - '0'; > } > > - if (ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt)) > - return -rte_errno; > + ret = ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt); > + if (ret) > + goto free_mem; > > - if (ice_parser_profile_init(&rslt, pkt_buf, msk_buf, > - pkt_len, ICE_BLK_RSS, true, &prof)) > - return -rte_errno; > + ret = ice_parser_profile_init(&rslt, pkt_buf, msk_buf, > + pkt_len, ICE_BLK_RSS, true, &prof); > + if (ret) > + goto free_mem; > > rte_memcpy(&meta->raw.prof, &prof, sizeof(prof)); > > +free_mem: > rte_free(pkt_buf); > rte_free(msk_buf); > - return 0; > + > + return ret; > } > > static void > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 1/3] net/ice: fix possible memory leak 2024-07-11 16:59 [PATCH] net/ice: fix possible memory leak Vladimir Medvedkin 2024-07-11 17:05 ` Bruce Richardson @ 2024-07-15 18:04 ` Vladimir Medvedkin 2024-07-15 18:04 ` [PATCH v2 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin ` (3 more replies) 1 sibling, 4 replies; 26+ messages in thread From: Vladimir Medvedkin @ 2024-07-15 18:04 UTC (permalink / raw) To: dev; +Cc: bruce.richardson, stable, Michael Theodore Stolarchuk This patch fixes possible memory leak inside the ice_hash_parse_raw_pattern() due to the lack of a call to rte_free() for previously allocated pkt_buf and msk_buf. Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow offloading in RSS") Cc: stable@dpdk.org Reported-by: Michael Theodore Stolarchuk <mike.stolarchuk@arista.com> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> --- drivers/net/ice/ice_hash.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c index f923641533..913f54fca4 100644 --- a/drivers/net/ice/ice_hash.c +++ b/drivers/net/ice/ice_hash.c @@ -650,7 +650,7 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, uint8_t *pkt_buf, *msk_buf; uint8_t tmp_val = 0; uint8_t tmp_c = 0; - int i, j; + int i, j, ret = 0; if (ad->psr == NULL) return -rte_errno; @@ -670,8 +670,10 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, return -ENOMEM; msk_buf = rte_zmalloc(NULL, pkt_len, 0); - if (!msk_buf) + if (!msk_buf) { + rte_free(pkt_buf); return -ENOMEM; + } /* convert string to int array */ for (i = 0, j = 0; i < spec_len; i += 2, j++) { @@ -708,17 +710,20 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, msk_buf[j] = tmp_val * 16 + tmp_c - '0'; } - if (ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt)) - return -rte_errno; + ret = ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt); + if (ret) + goto free_mem; - if (ice_parser_profile_init(&rslt, pkt_buf, msk_buf, - pkt_len, ICE_BLK_RSS, true, &prof)) - return -rte_errno; + ret = ice_parser_profile_init(&rslt, pkt_buf, msk_buf, + pkt_len, ICE_BLK_RSS, true, &prof); + goto free_mem; rte_memcpy(&meta->raw.prof, &prof, sizeof(prof)); +free_mem: rte_free(pkt_buf); rte_free(msk_buf); + return 0; } -- 2.34.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 2/3] net/ice: refactor raw pattern parsing function 2024-07-15 18:04 ` [PATCH v2 1/3] " Vladimir Medvedkin @ 2024-07-15 18:04 ` Vladimir Medvedkin 2024-07-17 11:05 ` Bruce Richardson 2024-07-15 18:04 ` [PATCH v2 3/3] net/ice: fix return value for " Vladimir Medvedkin ` (2 subsequent siblings) 3 siblings, 1 reply; 26+ messages in thread From: Vladimir Medvedkin @ 2024-07-15 18:04 UTC (permalink / raw) To: dev; +Cc: bruce.richardson Replace strlen with more secure strnlen in ice_hash_parse_raw_pattern. Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> --- drivers/net/ice/ice_hash.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c index 913f54fca4..00503d0d28 100644 --- a/drivers/net/ice/ice_hash.c +++ b/drivers/net/ice/ice_hash.c @@ -658,9 +658,9 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, raw_spec = item->spec; raw_mask = item->mask; - spec_len = strlen((char *)(uintptr_t)raw_spec->pattern); - if (strlen((char *)(uintptr_t)raw_mask->pattern) != - spec_len) + spec_len = strnlen((char *)(uintptr_t)raw_spec->pattern, raw_spec->length); + if (strnlen((char *)(uintptr_t)raw_mask->pattern, raw_spec->length) != + spec_len) return -rte_errno; pkt_len = spec_len / 2; -- 2.34.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/3] net/ice: refactor raw pattern parsing function 2024-07-15 18:04 ` [PATCH v2 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin @ 2024-07-17 11:05 ` Bruce Richardson 0 siblings, 0 replies; 26+ messages in thread From: Bruce Richardson @ 2024-07-17 11:05 UTC (permalink / raw) To: Vladimir Medvedkin; +Cc: dev On Mon, Jul 15, 2024 at 06:04:40PM +0000, Vladimir Medvedkin wrote: > Replace strlen with more secure strnlen in ice_hash_parse_raw_pattern. > > Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> > --- Acked-by: Bruce Richardson <bruce.richardson@intel.com> > drivers/net/ice/ice_hash.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c > index 913f54fca4..00503d0d28 100644 > --- a/drivers/net/ice/ice_hash.c > +++ b/drivers/net/ice/ice_hash.c > @@ -658,9 +658,9 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, > raw_spec = item->spec; > raw_mask = item->mask; > > - spec_len = strlen((char *)(uintptr_t)raw_spec->pattern); > - if (strlen((char *)(uintptr_t)raw_mask->pattern) != > - spec_len) > + spec_len = strnlen((char *)(uintptr_t)raw_spec->pattern, raw_spec->length); > + if (strnlen((char *)(uintptr_t)raw_mask->pattern, raw_spec->length) != > + spec_len) > return -rte_errno; > > pkt_len = spec_len / 2; > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 3/3] net/ice: fix return value for raw pattern parsing function 2024-07-15 18:04 ` [PATCH v2 1/3] " Vladimir Medvedkin 2024-07-15 18:04 ` [PATCH v2 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin @ 2024-07-15 18:04 ` Vladimir Medvedkin 2024-07-17 11:06 ` Bruce Richardson 2024-07-17 11:04 ` [PATCH v2 1/3] net/ice: fix possible memory leak Bruce Richardson 2024-07-22 8:28 ` [PATCH v3 " Vladimir Medvedkin 3 siblings, 1 reply; 26+ messages in thread From: Vladimir Medvedkin @ 2024-07-15 18:04 UTC (permalink / raw) To: dev; +Cc: bruce.richardson, stable If the parser was not initialized when calling ice_hash_parse_raw_pattern() -rte_errno was returned. Replace returning rte_errno with ENOTSUP since rte_errno is meaningless in the context of ice_hash_parse_raw_pattern(). Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow offloading in RSS") Cc: stable@dpdk.org Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> --- drivers/net/ice/ice_hash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c index 00503d0d28..13a68b8f02 100644 --- a/drivers/net/ice/ice_hash.c +++ b/drivers/net/ice/ice_hash.c @@ -653,7 +653,7 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, int i, j, ret = 0; if (ad->psr == NULL) - return -rte_errno; + return -ENOTSUP; raw_spec = item->spec; raw_mask = item->mask; -- 2.34.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 3/3] net/ice: fix return value for raw pattern parsing function 2024-07-15 18:04 ` [PATCH v2 3/3] net/ice: fix return value for " Vladimir Medvedkin @ 2024-07-17 11:06 ` Bruce Richardson 0 siblings, 0 replies; 26+ messages in thread From: Bruce Richardson @ 2024-07-17 11:06 UTC (permalink / raw) To: Vladimir Medvedkin; +Cc: dev, stable On Mon, Jul 15, 2024 at 06:04:41PM +0000, Vladimir Medvedkin wrote: > If the parser was not initialized when calling ice_hash_parse_raw_pattern() > -rte_errno was returned. Replace returning rte_errno with ENOTSUP since > rte_errno is meaningless in the context of ice_hash_parse_raw_pattern(). > > Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow offloading in RSS") > Cc: stable@dpdk.org > > Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com> ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/3] net/ice: fix possible memory leak 2024-07-15 18:04 ` [PATCH v2 1/3] " Vladimir Medvedkin 2024-07-15 18:04 ` [PATCH v2 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin 2024-07-15 18:04 ` [PATCH v2 3/3] net/ice: fix return value for " Vladimir Medvedkin @ 2024-07-17 11:04 ` Bruce Richardson 2024-07-22 8:28 ` [PATCH v3 " Vladimir Medvedkin 3 siblings, 0 replies; 26+ messages in thread From: Bruce Richardson @ 2024-07-17 11:04 UTC (permalink / raw) To: Vladimir Medvedkin; +Cc: dev, stable, Michael Theodore Stolarchuk On Mon, Jul 15, 2024 at 06:04:39PM +0000, Vladimir Medvedkin wrote: > This patch fixes possible memory leak inside the > ice_hash_parse_raw_pattern() due to the lack of a call to rte_free() > for previously allocated pkt_buf and msk_buf. > > Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow offloading in RSS") > Cc: stable@dpdk.org > > Reported-by: Michael Theodore Stolarchuk <mike.stolarchuk@arista.com> > Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> Hi, comments line below. /Bruce > --- > drivers/net/ice/ice_hash.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c > index f923641533..913f54fca4 100644 > --- a/drivers/net/ice/ice_hash.c > +++ b/drivers/net/ice/ice_hash.c > @@ -650,7 +650,7 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, > uint8_t *pkt_buf, *msk_buf; > uint8_t tmp_val = 0; > uint8_t tmp_c = 0; > - int i, j; > + int i, j, ret = 0; > > if (ad->psr == NULL) > return -rte_errno; > @@ -670,8 +670,10 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, > return -ENOMEM; > > msk_buf = rte_zmalloc(NULL, pkt_len, 0); > - if (!msk_buf) > + if (!msk_buf) { > + rte_free(pkt_buf); > return -ENOMEM; > + } > > /* convert string to int array */ > for (i = 0, j = 0; i < spec_len; i += 2, j++) { > @@ -708,17 +710,20 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, > msk_buf[j] = tmp_val * 16 + tmp_c - '0'; > } > > - if (ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt)) > - return -rte_errno; > + ret = ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt); > + if (ret) > + goto free_mem; We are losing the error code retrn value here. I think the final return in this function should be "return ret" rather than "return 0" so we can propagate up the error. > > - if (ice_parser_profile_init(&rslt, pkt_buf, msk_buf, > - pkt_len, ICE_BLK_RSS, true, &prof)) > - return -rte_errno; > + ret = ice_parser_profile_init(&rslt, pkt_buf, msk_buf, > + pkt_len, ICE_BLK_RSS, true, &prof); > + goto free_mem; Same here. Also the "if" statement before the "goto" is missing. > > rte_memcpy(&meta->raw.prof, &prof, sizeof(prof)); > > +free_mem: > rte_free(pkt_buf); > rte_free(msk_buf); > + > return 0; > } > > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3 1/3] net/ice: fix possible memory leak 2024-07-15 18:04 ` [PATCH v2 1/3] " Vladimir Medvedkin ` (2 preceding siblings ...) 2024-07-17 11:04 ` [PATCH v2 1/3] net/ice: fix possible memory leak Bruce Richardson @ 2024-07-22 8:28 ` Vladimir Medvedkin 2024-07-22 8:28 ` [PATCH v3 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin ` (3 more replies) 3 siblings, 4 replies; 26+ messages in thread From: Vladimir Medvedkin @ 2024-07-22 8:28 UTC (permalink / raw) To: dev; +Cc: bruce.richardson, stable, Michael Theodore Stolarchuk This patch fixes possible memory leak inside the ice_hash_parse_raw_pattern() due to the lack of a call to rte_free() for previously allocated pkt_buf and msk_buf. Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow offloading in RSS") Cc: stable@dpdk.org Reported-by: Michael Theodore Stolarchuk <mike.stolarchuk@arista.com> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> --- drivers/net/ice/ice_hash.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c index f923641533..cdce1d0ea2 100644 --- a/drivers/net/ice/ice_hash.c +++ b/drivers/net/ice/ice_hash.c @@ -650,7 +650,7 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, uint8_t *pkt_buf, *msk_buf; uint8_t tmp_val = 0; uint8_t tmp_c = 0; - int i, j; + int i, j, ret = 0; if (ad->psr == NULL) return -rte_errno; @@ -670,8 +670,10 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, return -ENOMEM; msk_buf = rte_zmalloc(NULL, pkt_len, 0); - if (!msk_buf) + if (!msk_buf) { + rte_free(pkt_buf); return -ENOMEM; + } /* convert string to int array */ for (i = 0, j = 0; i < spec_len; i += 2, j++) { @@ -708,18 +710,21 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, msk_buf[j] = tmp_val * 16 + tmp_c - '0'; } - if (ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt)) - return -rte_errno; + ret = ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt); + if (ret) + goto free_mem; - if (ice_parser_profile_init(&rslt, pkt_buf, msk_buf, - pkt_len, ICE_BLK_RSS, true, &prof)) - return -rte_errno; + ret = ice_parser_profile_init(&rslt, pkt_buf, msk_buf, + pkt_len, ICE_BLK_RSS, true, &prof); + goto free_mem; rte_memcpy(&meta->raw.prof, &prof, sizeof(prof)); +free_mem: rte_free(pkt_buf); rte_free(msk_buf); - return 0; + + return ret; } static void -- 2.34.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3 2/3] net/ice: refactor raw pattern parsing function 2024-07-22 8:28 ` [PATCH v3 " Vladimir Medvedkin @ 2024-07-22 8:28 ` Vladimir Medvedkin 2024-07-22 8:28 ` [PATCH v3 3/3] net/ice: fix return value for " Vladimir Medvedkin ` (2 subsequent siblings) 3 siblings, 0 replies; 26+ messages in thread From: Vladimir Medvedkin @ 2024-07-22 8:28 UTC (permalink / raw) To: dev; +Cc: bruce.richardson Replace strlen with more secure strnlen in ice_hash_parse_raw_pattern. Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> --- drivers/net/ice/ice_hash.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c index cdce1d0ea2..d63e673b25 100644 --- a/drivers/net/ice/ice_hash.c +++ b/drivers/net/ice/ice_hash.c @@ -658,9 +658,9 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, raw_spec = item->spec; raw_mask = item->mask; - spec_len = strlen((char *)(uintptr_t)raw_spec->pattern); - if (strlen((char *)(uintptr_t)raw_mask->pattern) != - spec_len) + spec_len = strnlen((char *)(uintptr_t)raw_spec->pattern, raw_spec->length); + if (strnlen((char *)(uintptr_t)raw_mask->pattern, raw_spec->length) != + spec_len) return -rte_errno; pkt_len = spec_len / 2; -- 2.34.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v3 3/3] net/ice: fix return value for raw pattern parsing function 2024-07-22 8:28 ` [PATCH v3 " Vladimir Medvedkin 2024-07-22 8:28 ` [PATCH v3 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin @ 2024-07-22 8:28 ` Vladimir Medvedkin 2024-07-22 10:41 ` [PATCH v3 1/3] net/ice: fix possible memory leak Bruce Richardson 2024-07-22 10:59 ` [PATCH v4 " Vladimir Medvedkin 3 siblings, 0 replies; 26+ messages in thread From: Vladimir Medvedkin @ 2024-07-22 8:28 UTC (permalink / raw) To: dev; +Cc: bruce.richardson, stable If the parser was not initialized when calling ice_hash_parse_raw_pattern() -rte_errno was returned. Replace returning rte_errno with ENOTSUP since rte_errno is meaningless in the context of ice_hash_parse_raw_pattern(). Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow offloading in RSS") Cc: stable@dpdk.org Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> --- drivers/net/ice/ice_hash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c index d63e673b25..b040a198bb 100644 --- a/drivers/net/ice/ice_hash.c +++ b/drivers/net/ice/ice_hash.c @@ -653,7 +653,7 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, int i, j, ret = 0; if (ad->psr == NULL) - return -rte_errno; + return -ENOTSUP; raw_spec = item->spec; raw_mask = item->mask; -- 2.34.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v3 1/3] net/ice: fix possible memory leak 2024-07-22 8:28 ` [PATCH v3 " Vladimir Medvedkin 2024-07-22 8:28 ` [PATCH v3 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin 2024-07-22 8:28 ` [PATCH v3 3/3] net/ice: fix return value for " Vladimir Medvedkin @ 2024-07-22 10:41 ` Bruce Richardson 2024-07-22 11:00 ` Medvedkin, Vladimir 2024-07-22 10:59 ` [PATCH v4 " Vladimir Medvedkin 3 siblings, 1 reply; 26+ messages in thread From: Bruce Richardson @ 2024-07-22 10:41 UTC (permalink / raw) To: Vladimir Medvedkin; +Cc: dev, stable, Michael Theodore Stolarchuk On Mon, Jul 22, 2024 at 08:28:34AM +0000, Vladimir Medvedkin wrote: > This patch fixes possible memory leak inside the > ice_hash_parse_raw_pattern() due to the lack of a call to rte_free() > for previously allocated pkt_buf and msk_buf. > > Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow offloading in RSS") > Cc: stable@dpdk.org > > Reported-by: Michael Theodore Stolarchuk <mike.stolarchuk@arista.com> > Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> > --- > drivers/net/ice/ice_hash.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > <snip> > - if (ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt)) > - return -rte_errno; > + ret = ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt); > + if (ret) > + goto free_mem; > > - if (ice_parser_profile_init(&rslt, pkt_buf, msk_buf, > - pkt_len, ICE_BLK_RSS, true, &prof)) > - return -rte_errno; > + ret = ice_parser_profile_init(&rslt, pkt_buf, msk_buf, > + pkt_len, ICE_BLK_RSS, true, &prof); > + goto free_mem; Are we not still missing an "if (ret != 0)" here? If so, I can just add on apply. /Bruce ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH v3 1/3] net/ice: fix possible memory leak 2024-07-22 10:41 ` [PATCH v3 1/3] net/ice: fix possible memory leak Bruce Richardson @ 2024-07-22 11:00 ` Medvedkin, Vladimir 0 siblings, 0 replies; 26+ messages in thread From: Medvedkin, Vladimir @ 2024-07-22 11:00 UTC (permalink / raw) To: Richardson, Bruce; +Cc: dev, stable, Stolarchuk, Michael -----Original Message----- From: Richardson, Bruce <bruce.richardson@intel.com> Sent: Monday, July 22, 2024 11:42 AM To: Medvedkin, Vladimir <vladimir.medvedkin@intel.com> Cc: dev@dpdk.org; stable@dpdk.org; Stolarchuk, Michael <mike.stolarchuk@arista.com> Subject: Re: [PATCH v3 1/3] net/ice: fix possible memory leak On Mon, Jul 22, 2024 at 08:28:34AM +0000, Vladimir Medvedkin wrote: > This patch fixes possible memory leak inside the > ice_hash_parse_raw_pattern() due to the lack of a call to rte_free() > for previously allocated pkt_buf and msk_buf. > > Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow > offloading in RSS") > Cc: stable@dpdk.org > > Reported-by: Michael Theodore Stolarchuk <mike.stolarchuk@arista.com> > Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> > --- > drivers/net/ice/ice_hash.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > <snip> > - if (ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt)) > - return -rte_errno; > + ret = ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt); > + if (ret) > + goto free_mem; > > - if (ice_parser_profile_init(&rslt, pkt_buf, msk_buf, > - pkt_len, ICE_BLK_RSS, true, &prof)) > - return -rte_errno; > + ret = ice_parser_profile_init(&rslt, pkt_buf, msk_buf, > + pkt_len, ICE_BLK_RSS, true, &prof); > + goto free_mem; Are we not still missing an "if (ret != 0)" here? If so, I can just add on apply. That's correct, will send v4 /Bruce ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v4 1/3] net/ice: fix possible memory leak 2024-07-22 8:28 ` [PATCH v3 " Vladimir Medvedkin ` (2 preceding siblings ...) 2024-07-22 10:41 ` [PATCH v3 1/3] net/ice: fix possible memory leak Bruce Richardson @ 2024-07-22 10:59 ` Vladimir Medvedkin 2024-07-22 10:59 ` [PATCH v4 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin ` (2 more replies) 3 siblings, 3 replies; 26+ messages in thread From: Vladimir Medvedkin @ 2024-07-22 10:59 UTC (permalink / raw) To: dev; +Cc: bruce.richardson, stable, Michael Theodore Stolarchuk This patch fixes possible memory leak inside the ice_hash_parse_raw_pattern() due to the lack of a call to rte_free() for previously allocated pkt_buf and msk_buf. Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow offloading in RSS") Cc: stable@dpdk.org Reported-by: Michael Theodore Stolarchuk <mike.stolarchuk@arista.com> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> --- drivers/net/ice/ice_hash.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c index f923641533..6b3095e2c5 100644 --- a/drivers/net/ice/ice_hash.c +++ b/drivers/net/ice/ice_hash.c @@ -650,7 +650,7 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, uint8_t *pkt_buf, *msk_buf; uint8_t tmp_val = 0; uint8_t tmp_c = 0; - int i, j; + int i, j, ret = 0; if (ad->psr == NULL) return -rte_errno; @@ -670,8 +670,10 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, return -ENOMEM; msk_buf = rte_zmalloc(NULL, pkt_len, 0); - if (!msk_buf) + if (!msk_buf) { + rte_free(pkt_buf); return -ENOMEM; + } /* convert string to int array */ for (i = 0, j = 0; i < spec_len; i += 2, j++) { @@ -708,18 +710,22 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, msk_buf[j] = tmp_val * 16 + tmp_c - '0'; } - if (ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt)) - return -rte_errno; + ret = ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt); + if (ret) + goto free_mem; - if (ice_parser_profile_init(&rslt, pkt_buf, msk_buf, - pkt_len, ICE_BLK_RSS, true, &prof)) - return -rte_errno; + ret = ice_parser_profile_init(&rslt, pkt_buf, msk_buf, + pkt_len, ICE_BLK_RSS, true, &prof); + if (ret) + goto free_mem; rte_memcpy(&meta->raw.prof, &prof, sizeof(prof)); +free_mem: rte_free(pkt_buf); rte_free(msk_buf); - return 0; + + return ret; } static void -- 2.34.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v4 2/3] net/ice: refactor raw pattern parsing function 2024-07-22 10:59 ` [PATCH v4 " Vladimir Medvedkin @ 2024-07-22 10:59 ` Vladimir Medvedkin 2024-07-22 11:25 ` Bruce Richardson 2024-07-22 10:59 ` [PATCH v4 3/3] net/ice: fix return value for " Vladimir Medvedkin 2024-07-22 13:50 ` [PATCH v5 1/3] net/ice: fix possible memory leak Vladimir Medvedkin 2 siblings, 1 reply; 26+ messages in thread From: Vladimir Medvedkin @ 2024-07-22 10:59 UTC (permalink / raw) To: dev; +Cc: bruce.richardson Replace strlen with more secure strnlen in ice_hash_parse_raw_pattern. Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> --- drivers/net/ice/ice_hash.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c index 6b3095e2c5..506ea261e8 100644 --- a/drivers/net/ice/ice_hash.c +++ b/drivers/net/ice/ice_hash.c @@ -658,9 +658,9 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, raw_spec = item->spec; raw_mask = item->mask; - spec_len = strlen((char *)(uintptr_t)raw_spec->pattern); - if (strlen((char *)(uintptr_t)raw_mask->pattern) != - spec_len) + spec_len = strnlen((char *)(uintptr_t)raw_spec->pattern, raw_spec->length); + if (strnlen((char *)(uintptr_t)raw_mask->pattern, raw_spec->length) != + spec_len) return -rte_errno; pkt_len = spec_len / 2; -- 2.34.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v4 2/3] net/ice: refactor raw pattern parsing function 2024-07-22 10:59 ` [PATCH v4 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin @ 2024-07-22 11:25 ` Bruce Richardson 2024-07-22 13:51 ` Medvedkin, Vladimir 0 siblings, 1 reply; 26+ messages in thread From: Bruce Richardson @ 2024-07-22 11:25 UTC (permalink / raw) To: Vladimir Medvedkin; +Cc: dev On Mon, Jul 22, 2024 at 10:59:49AM +0000, Vladimir Medvedkin wrote: > Replace strlen with more secure strnlen in ice_hash_parse_raw_pattern. > > Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> > --- > drivers/net/ice/ice_hash.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c > index 6b3095e2c5..506ea261e8 100644 > --- a/drivers/net/ice/ice_hash.c > +++ b/drivers/net/ice/ice_hash.c > @@ -658,9 +658,9 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, > raw_spec = item->spec; > raw_mask = item->mask; > > - spec_len = strlen((char *)(uintptr_t)raw_spec->pattern); > - if (strlen((char *)(uintptr_t)raw_mask->pattern) != > - spec_len) > + spec_len = strnlen((char *)(uintptr_t)raw_spec->pattern, raw_spec->length); > + if (strnlen((char *)(uintptr_t)raw_mask->pattern, raw_spec->length) != > + spec_len) Are we missing something by not checking the return values from the length calls for overflow? If spec_len == raw_spec->length, then we have an overflow, and if raw_mask similarly overflows the comparison would still pass and not flag an error. /Bruce > return -rte_errno; > > pkt_len = spec_len / 2; > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: [PATCH v4 2/3] net/ice: refactor raw pattern parsing function 2024-07-22 11:25 ` Bruce Richardson @ 2024-07-22 13:51 ` Medvedkin, Vladimir 0 siblings, 0 replies; 26+ messages in thread From: Medvedkin, Vladimir @ 2024-07-22 13:51 UTC (permalink / raw) To: Richardson, Bruce; +Cc: dev -----Original Message----- From: Richardson, Bruce <bruce.richardson@intel.com> Sent: Monday, July 22, 2024 12:25 PM To: Medvedkin, Vladimir <vladimir.medvedkin@intel.com> Cc: dev@dpdk.org Subject: Re: [PATCH v4 2/3] net/ice: refactor raw pattern parsing function On Mon, Jul 22, 2024 at 10:59:49AM +0000, Vladimir Medvedkin wrote: > Replace strlen with more secure strnlen in ice_hash_parse_raw_pattern. > > Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> > --- > drivers/net/ice/ice_hash.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c > index 6b3095e2c5..506ea261e8 100644 > --- a/drivers/net/ice/ice_hash.c > +++ b/drivers/net/ice/ice_hash.c > @@ -658,9 +658,9 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, > raw_spec = item->spec; > raw_mask = item->mask; > > - spec_len = strlen((char *)(uintptr_t)raw_spec->pattern); > - if (strlen((char *)(uintptr_t)raw_mask->pattern) != > - spec_len) > + spec_len = strnlen((char *)(uintptr_t)raw_spec->pattern, raw_spec->length); > + if (strnlen((char *)(uintptr_t)raw_mask->pattern, raw_spec->length) != > + spec_len) Are we missing something by not checking the return values from the length calls for overflow? If spec_len == raw_spec->length, then we have an overflow, and if raw_mask similarly overflows the comparison would still pass and not flag an error. Fixed in v5 /Bruce > return -rte_errno; > > pkt_len = spec_len / 2; > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v4 3/3] net/ice: fix return value for raw pattern parsing function 2024-07-22 10:59 ` [PATCH v4 " Vladimir Medvedkin 2024-07-22 10:59 ` [PATCH v4 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin @ 2024-07-22 10:59 ` Vladimir Medvedkin 2024-07-22 13:50 ` [PATCH v5 1/3] net/ice: fix possible memory leak Vladimir Medvedkin 2 siblings, 0 replies; 26+ messages in thread From: Vladimir Medvedkin @ 2024-07-22 10:59 UTC (permalink / raw) To: dev; +Cc: bruce.richardson, stable If the parser was not initialized when calling ice_hash_parse_raw_pattern() -rte_errno was returned. Replace returning rte_errno with ENOTSUP since rte_errno is meaningless in the context of ice_hash_parse_raw_pattern(). Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow offloading in RSS") Cc: stable@dpdk.org Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> --- drivers/net/ice/ice_hash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c index 506ea261e8..1188962752 100644 --- a/drivers/net/ice/ice_hash.c +++ b/drivers/net/ice/ice_hash.c @@ -653,7 +653,7 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, int i, j, ret = 0; if (ad->psr == NULL) - return -rte_errno; + return -ENOTSUP; raw_spec = item->spec; raw_mask = item->mask; -- 2.34.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 1/3] net/ice: fix possible memory leak 2024-07-22 10:59 ` [PATCH v4 " Vladimir Medvedkin 2024-07-22 10:59 ` [PATCH v4 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin 2024-07-22 10:59 ` [PATCH v4 3/3] net/ice: fix return value for " Vladimir Medvedkin @ 2024-07-22 13:50 ` Vladimir Medvedkin 2024-07-22 13:50 ` [PATCH v5 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin ` (2 more replies) 2 siblings, 3 replies; 26+ messages in thread From: Vladimir Medvedkin @ 2024-07-22 13:50 UTC (permalink / raw) To: dev; +Cc: bruce.richardson, stable, Michael Theodore Stolarchuk This patch fixes possible memory leak inside the ice_hash_parse_raw_pattern() due to the lack of a call to rte_free() for previously allocated pkt_buf and msk_buf. Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow offloading in RSS") Cc: stable@dpdk.org Reported-by: Michael Theodore Stolarchuk <mike.stolarchuk@arista.com> Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> --- drivers/net/ice/ice_hash.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c index f923641533..6b3095e2c5 100644 --- a/drivers/net/ice/ice_hash.c +++ b/drivers/net/ice/ice_hash.c @@ -650,7 +650,7 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, uint8_t *pkt_buf, *msk_buf; uint8_t tmp_val = 0; uint8_t tmp_c = 0; - int i, j; + int i, j, ret = 0; if (ad->psr == NULL) return -rte_errno; @@ -670,8 +670,10 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, return -ENOMEM; msk_buf = rte_zmalloc(NULL, pkt_len, 0); - if (!msk_buf) + if (!msk_buf) { + rte_free(pkt_buf); return -ENOMEM; + } /* convert string to int array */ for (i = 0, j = 0; i < spec_len; i += 2, j++) { @@ -708,18 +710,22 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, msk_buf[j] = tmp_val * 16 + tmp_c - '0'; } - if (ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt)) - return -rte_errno; + ret = ice_parser_run(ad->psr, pkt_buf, pkt_len, &rslt); + if (ret) + goto free_mem; - if (ice_parser_profile_init(&rslt, pkt_buf, msk_buf, - pkt_len, ICE_BLK_RSS, true, &prof)) - return -rte_errno; + ret = ice_parser_profile_init(&rslt, pkt_buf, msk_buf, + pkt_len, ICE_BLK_RSS, true, &prof); + if (ret) + goto free_mem; rte_memcpy(&meta->raw.prof, &prof, sizeof(prof)); +free_mem: rte_free(pkt_buf); rte_free(msk_buf); - return 0; + + return ret; } static void -- 2.34.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 2/3] net/ice: refactor raw pattern parsing function 2024-07-22 13:50 ` [PATCH v5 1/3] net/ice: fix possible memory leak Vladimir Medvedkin @ 2024-07-22 13:50 ` Vladimir Medvedkin 2024-07-22 15:10 ` Bruce Richardson 2024-07-22 13:50 ` [PATCH v5 3/3] net/ice: fix return value for " Vladimir Medvedkin 2024-07-22 15:09 ` [PATCH v5 1/3] net/ice: fix possible memory leak Bruce Richardson 2 siblings, 1 reply; 26+ messages in thread From: Vladimir Medvedkin @ 2024-07-22 13:50 UTC (permalink / raw) To: dev; +Cc: bruce.richardson Replace strlen with more secure strnlen in ice_hash_parse_raw_pattern. Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> tmp --- drivers/net/ice/ice_hash.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c index 6b3095e2c5..aa76718313 100644 --- a/drivers/net/ice/ice_hash.c +++ b/drivers/net/ice/ice_hash.c @@ -658,10 +658,13 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, raw_spec = item->spec; raw_mask = item->mask; - spec_len = strlen((char *)(uintptr_t)raw_spec->pattern); - if (strlen((char *)(uintptr_t)raw_mask->pattern) != - spec_len) - return -rte_errno; + spec_len = strnlen((char *)(uintptr_t)raw_spec->pattern, + raw_spec->length + 1); + if (spec_len != raw_spec->length) + return -EINVAL; + if (strnlen((char *)(uintptr_t)raw_mask->pattern, raw_spec->length + 1) != + spec_len) + return -EINVAL; pkt_len = spec_len / 2; -- 2.34.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v5 2/3] net/ice: refactor raw pattern parsing function 2024-07-22 13:50 ` [PATCH v5 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin @ 2024-07-22 15:10 ` Bruce Richardson 0 siblings, 0 replies; 26+ messages in thread From: Bruce Richardson @ 2024-07-22 15:10 UTC (permalink / raw) To: Vladimir Medvedkin; +Cc: dev On Mon, Jul 22, 2024 at 01:50:45PM +0000, Vladimir Medvedkin wrote: > Replace strlen with more secure strnlen in ice_hash_parse_raw_pattern. > > Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> > I believe there are quite a number of other small things in this function that could do with improvement e.g. the processing of the hex strings has no error checks for reporting invalid (i.e. non-hex) characters. However, this patch does improve things a bit by enhancing the length checks, so Acked-by: Bruce Richardson <bruce.richardson@intel.com> > --- > drivers/net/ice/ice_hash.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c > index 6b3095e2c5..aa76718313 100644 > --- a/drivers/net/ice/ice_hash.c > +++ b/drivers/net/ice/ice_hash.c > @@ -658,10 +658,13 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, > raw_spec = item->spec; > raw_mask = item->mask; > > - spec_len = strlen((char *)(uintptr_t)raw_spec->pattern); > - if (strlen((char *)(uintptr_t)raw_mask->pattern) != > - spec_len) > - return -rte_errno; > + spec_len = strnlen((char *)(uintptr_t)raw_spec->pattern, > + raw_spec->length + 1); > + if (spec_len != raw_spec->length) > + return -EINVAL; > + if (strnlen((char *)(uintptr_t)raw_mask->pattern, raw_spec->length + 1) != > + spec_len) > + return -EINVAL; > > pkt_len = spec_len / 2; > > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v5 3/3] net/ice: fix return value for raw pattern parsing function 2024-07-22 13:50 ` [PATCH v5 1/3] net/ice: fix possible memory leak Vladimir Medvedkin 2024-07-22 13:50 ` [PATCH v5 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin @ 2024-07-22 13:50 ` Vladimir Medvedkin 2024-07-22 15:11 ` Bruce Richardson 2024-07-22 15:09 ` [PATCH v5 1/3] net/ice: fix possible memory leak Bruce Richardson 2 siblings, 1 reply; 26+ messages in thread From: Vladimir Medvedkin @ 2024-07-22 13:50 UTC (permalink / raw) To: dev; +Cc: bruce.richardson, stable If the parser was not initialized when calling ice_hash_parse_raw_pattern() -rte_errno was returned. Replace returning rte_errno with ENOTSUP since rte_errno is meaningless in the context of ice_hash_parse_raw_pattern(). Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow offloading in RSS") Cc: stable@dpdk.org Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> --- drivers/net/ice/ice_hash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c index aa76718313..b720e0f755 100644 --- a/drivers/net/ice/ice_hash.c +++ b/drivers/net/ice/ice_hash.c @@ -653,7 +653,7 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, int i, j, ret = 0; if (ad->psr == NULL) - return -rte_errno; + return -ENOTSUP; raw_spec = item->spec; raw_mask = item->mask; -- 2.34.1 ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v5 3/3] net/ice: fix return value for raw pattern parsing function 2024-07-22 13:50 ` [PATCH v5 3/3] net/ice: fix return value for " Vladimir Medvedkin @ 2024-07-22 15:11 ` Bruce Richardson 0 siblings, 0 replies; 26+ messages in thread From: Bruce Richardson @ 2024-07-22 15:11 UTC (permalink / raw) To: Vladimir Medvedkin; +Cc: dev, stable On Mon, Jul 22, 2024 at 01:50:46PM +0000, Vladimir Medvedkin wrote: > If the parser was not initialized when calling ice_hash_parse_raw_pattern() > -rte_errno was returned. Replace returning rte_errno with ENOTSUP since > rte_errno is meaningless in the context of ice_hash_parse_raw_pattern(). > > Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow offloading in RSS") > Cc: stable@dpdk.org > > Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com> > --- > drivers/net/ice/ice_hash.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ice/ice_hash.c b/drivers/net/ice/ice_hash.c > index aa76718313..b720e0f755 100644 > --- a/drivers/net/ice/ice_hash.c > +++ b/drivers/net/ice/ice_hash.c > @@ -653,7 +653,7 @@ ice_hash_parse_raw_pattern(struct ice_adapter *ad, > int i, j, ret = 0; > > if (ad->psr == NULL) > - return -rte_errno; > + return -ENOTSUP; > > raw_spec = item->spec; > raw_mask = item->mask; > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v5 1/3] net/ice: fix possible memory leak 2024-07-22 13:50 ` [PATCH v5 1/3] net/ice: fix possible memory leak Vladimir Medvedkin 2024-07-22 13:50 ` [PATCH v5 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin 2024-07-22 13:50 ` [PATCH v5 3/3] net/ice: fix return value for " Vladimir Medvedkin @ 2024-07-22 15:09 ` Bruce Richardson 2024-07-22 15:18 ` Bruce Richardson 2 siblings, 1 reply; 26+ messages in thread From: Bruce Richardson @ 2024-07-22 15:09 UTC (permalink / raw) To: Vladimir Medvedkin; +Cc: dev, stable, Michael Theodore Stolarchuk On Mon, Jul 22, 2024 at 01:50:44PM +0000, Vladimir Medvedkin wrote: > This patch fixes possible memory leak inside the > ice_hash_parse_raw_pattern() due to the lack of a call to rte_free() > for previously allocated pkt_buf and msk_buf. > > Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow offloading in RSS") > Cc: stable@dpdk.org > > Reported-by: Michael Theodore Stolarchuk <mike.stolarchuk@arista.com> > Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> Acked-by: Bruce Richardson <bruce.richardson@intel.com> ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v5 1/3] net/ice: fix possible memory leak 2024-07-22 15:09 ` [PATCH v5 1/3] net/ice: fix possible memory leak Bruce Richardson @ 2024-07-22 15:18 ` Bruce Richardson 0 siblings, 0 replies; 26+ messages in thread From: Bruce Richardson @ 2024-07-22 15:18 UTC (permalink / raw) To: Vladimir Medvedkin; +Cc: dev, stable, Michael Theodore Stolarchuk On Mon, Jul 22, 2024 at 04:09:15PM +0100, Bruce Richardson wrote: > On Mon, Jul 22, 2024 at 01:50:44PM +0000, Vladimir Medvedkin wrote: > > This patch fixes possible memory leak inside the > > ice_hash_parse_raw_pattern() due to the lack of a call to rte_free() > > for previously allocated pkt_buf and msk_buf. > > > > Fixes: 1b9c68120a1c ("net/ice: enable protocol agnostic flow offloading in RSS") > > Cc: stable@dpdk.org > > > > Reported-by: Michael Theodore Stolarchuk <mike.stolarchuk@arista.com> > > Signed-off-by: Vladimir Medvedkin <vladimir.medvedkin@intel.com> > Acked-by: Bruce Richardson <bruce.richardson@intel.com> > Series applied to dpdk-next-net-intel. Thanks, /Bruce ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2024-07-22 15:18 UTC | newest] Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-07-11 16:59 [PATCH] net/ice: fix possible memory leak Vladimir Medvedkin 2024-07-11 17:05 ` Bruce Richardson 2024-07-11 17:07 ` Medvedkin, Vladimir 2024-07-15 18:04 ` [PATCH v2 1/3] " Vladimir Medvedkin 2024-07-15 18:04 ` [PATCH v2 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin 2024-07-17 11:05 ` Bruce Richardson 2024-07-15 18:04 ` [PATCH v2 3/3] net/ice: fix return value for " Vladimir Medvedkin 2024-07-17 11:06 ` Bruce Richardson 2024-07-17 11:04 ` [PATCH v2 1/3] net/ice: fix possible memory leak Bruce Richardson 2024-07-22 8:28 ` [PATCH v3 " Vladimir Medvedkin 2024-07-22 8:28 ` [PATCH v3 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin 2024-07-22 8:28 ` [PATCH v3 3/3] net/ice: fix return value for " Vladimir Medvedkin 2024-07-22 10:41 ` [PATCH v3 1/3] net/ice: fix possible memory leak Bruce Richardson 2024-07-22 11:00 ` Medvedkin, Vladimir 2024-07-22 10:59 ` [PATCH v4 " Vladimir Medvedkin 2024-07-22 10:59 ` [PATCH v4 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin 2024-07-22 11:25 ` Bruce Richardson 2024-07-22 13:51 ` Medvedkin, Vladimir 2024-07-22 10:59 ` [PATCH v4 3/3] net/ice: fix return value for " Vladimir Medvedkin 2024-07-22 13:50 ` [PATCH v5 1/3] net/ice: fix possible memory leak Vladimir Medvedkin 2024-07-22 13:50 ` [PATCH v5 2/3] net/ice: refactor raw pattern parsing function Vladimir Medvedkin 2024-07-22 15:10 ` Bruce Richardson 2024-07-22 13:50 ` [PATCH v5 3/3] net/ice: fix return value for " Vladimir Medvedkin 2024-07-22 15:11 ` Bruce Richardson 2024-07-22 15:09 ` [PATCH v5 1/3] net/ice: fix possible memory leak Bruce Richardson 2024-07-22 15:18 ` Bruce Richardson
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).