All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] examples/qos_meter: fix unchecked return value
@ 2016-04-21 11:47 Michal Jastrzebski
  2016-04-21 11:47 ` [PATCH v2] examples/qos_sched: fix out-of-bounds read Michal Jastrzebski
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Michal Jastrzebski @ 2016-04-21 11:47 UTC (permalink / raw)
  To: cristian.dumitrescu, roy.fan.zhang, jasvinder.singh
  Cc: dev, Slawomir Mrozowicz

From: Slawomir Mrozowicz <slawomirx.mrozowicz@intel.com>

Fix issue reported by Coverity.

Coverity ID 30693: Unchecked return value
check_return: Calling rte_meter_srtcm_config without checking return value.

Fixes: e6541fdec8b2 ("meter: initial import")

Signed-off-by: Slawomir Mrozowicz <slawomirx.mrozowicz@intel.com>
---
 examples/qos_meter/main.c | 15 ++++++++++-----
 examples/qos_meter/main.h |  2 +-
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/examples/qos_meter/main.c b/examples/qos_meter/main.c
index b968b00..16b0b87 100644
--- a/examples/qos_meter/main.c
+++ b/examples/qos_meter/main.c
@@ -133,14 +133,17 @@ struct rte_meter_trtcm_params app_trtcm_params[] = {
 
 FLOW_METER app_flows[APP_FLOWS_MAX];
 
-static void
+static int
 app_configure_flow_table(void)
 {
 	uint32_t i, j;
+	int ret = 0;
 
-	for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) % RTE_DIM(PARAMS)){
-		FUNC_CONFIG(&app_flows[i], &PARAMS[j]);
-	}
+	for (i = 0, j = 0; i < APP_FLOWS_MAX;
+			i ++, j = (j + 1) % RTE_DIM(PARAMS))
+		ret |= FUNC_CONFIG(&app_flows[i], &PARAMS[j]);
+
+	return ret;
 }
 
 static inline void
@@ -381,7 +384,9 @@ main(int argc, char **argv)
 	rte_eth_promiscuous_enable(port_tx);
 
 	/* App configuration */
-	app_configure_flow_table();
+	ret = app_configure_flow_table();
+	if (ret < 0)
+		rte_exit(EXIT_FAILURE, "Invalid configure flow table\n");
 
 	/* Launch per-lcore init on every lcore */
 	rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER);
diff --git a/examples/qos_meter/main.h b/examples/qos_meter/main.h
index 530bf69..54867dc 100644
--- a/examples/qos_meter/main.h
+++ b/examples/qos_meter/main.h
@@ -51,7 +51,7 @@ enum policer_action policer_table[e_RTE_METER_COLORS][e_RTE_METER_COLORS] =
 #if APP_MODE == APP_MODE_FWD
 
 #define FUNC_METER(a,b,c,d) color, flow_id=flow_id, pkt_len=pkt_len, time=time
-#define FUNC_CONFIG(a,b)
+#define FUNC_CONFIG(a, b) 0
 #define PARAMS	app_srtcm_params
 #define FLOW_METER int
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2] examples/qos_sched: fix out-of-bounds read
  2016-04-21 11:47 [PATCH v2] examples/qos_meter: fix unchecked return value Michal Jastrzebski
@ 2016-04-21 11:47 ` Michal Jastrzebski
  2016-04-28 10:59   ` Dumitrescu, Cristian
       [not found] ` <1461239256-8104-3-git-send-email-michalx.k.jastrzebski@intel.com>
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Michal Jastrzebski @ 2016-04-21 11:47 UTC (permalink / raw)
  To: cristian.dumitrescu, roy.fan.zhang, jasvinder.singh
  Cc: dev, Slawomir Mrozowicz

From: Slawomir Mrozowicz <slawomirx.mrozowicz@intel.com>

Fix issue reported by Coverity.

Coverity ID 30708: Out-of-bounds read
overrun-local: Overrunning array tokens of 8 8-byte elements
at element index 4294967294 (byte offset 34359738352)
using index i (which evaluates to 4294967294).

Fixes: de3cfa2c9823 ("sched: initial import")

Signed-off-by: Slawomir Mrozowicz <slawomirx.mrozowicz@intel.com>
---
 examples/qos_sched/args.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
index 3e7fd08..d819269 100644
--- a/examples/qos_sched/args.c
+++ b/examples/qos_sched/args.c
@@ -175,9 +175,11 @@ app_parse_opt_vals(const char *conf_str, char separator, uint32_t n_vals, uint32
 
 	n_tokens = rte_strsplit(string, strnlen(string, 32), tokens, n_vals, separator);
 
-	for(i = 0; i < n_tokens; i++) {
+	if (n_tokens > MAX_OPT_VALUES)
+		return -1;
+
+	for (i = 0; i < n_tokens; i++)
 		opt_vals[i] = (uint32_t)atol(tokens[i]);
-	}
 
 	free(string);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] examples/qos_sched: fix copy-paste error
       [not found] ` <1461239256-8104-3-git-send-email-michalx.k.jastrzebski@intel.com>
@ 2016-04-28 10:58   ` Dumitrescu, Cristian
  0 siblings, 0 replies; 7+ messages in thread
From: Dumitrescu, Cristian @ 2016-04-28 10:58 UTC (permalink / raw)
  To: Jastrzebski, MichalX K, Zhang, Roy Fan, Singh, Jasvinder
  Cc: dev, Mrozowicz, SlawomirX



> -----Original Message-----
> From: Jastrzebski, MichalX K
> Sent: Thursday, April 21, 2016 12:48 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Mrozowicz, SlawomirX
> <slawomirx.mrozowicz@intel.com>
> Subject: [PATCH v2] examples/qos_sched: fix copy-paste error
> 
> From: Slawomir Mrozowicz <slawomirx.mrozowicz@intel.com>
> 
> Coverity ID 30699: Copy-paste error;
> rx_port in pconf->rx_port looks like a copy-paste error.
> 
> Fixes: de3cfa2c9823 ("sched: initial import")
> 
> Signed-off-by: Slawomir Mrozowicz <slawomirx.mrozowicz@intel.com>
> ---
>  examples/qos_sched/args.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
> index 3e7fd08..1916790 100644
> --- a/examples/qos_sched/args.c
> +++ b/examples/qos_sched/args.c
> @@ -270,7 +270,7 @@ app_parse_flow_conf(const char *conf_str)
>  	}
>  	if (pconf->tx_port >= RTE_MAX_ETHPORTS) {
>  		RTE_LOG(ERR, APP, "pfc %u: invalid tx port %"PRIu8"
> index\n",
> -				nb_pfc, pconf->rx_port);
> +				nb_pfc, pconf->tx_port);
>  		return -1;
>  	}
> 
> --
> 1.9.1

Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] examples/qos_sched: fix negative loop bound
       [not found] ` <1461239256-8104-4-git-send-email-michalx.k.jastrzebski@intel.com>
@ 2016-04-28 10:58   ` Dumitrescu, Cristian
  0 siblings, 0 replies; 7+ messages in thread
From: Dumitrescu, Cristian @ 2016-04-28 10:58 UTC (permalink / raw)
  To: Jastrzebski, MichalX K, Zhang, Roy Fan, Singh, Jasvinder
  Cc: dev, Mrozowicz, SlawomirX



> -----Original Message-----
> From: Jastrzebski, MichalX K
> Sent: Thursday, April 21, 2016 12:48 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Mrozowicz, SlawomirX
> <slawomirx.mrozowicz@intel.com>
> Subject: [PATCH v2] examples/qos_sched: fix negative loop bound
> 
> From: Slawomir Mrozowicz <slawomirx.mrozowicz@intel.com>
> 
> Coverity ID 30704: Negative loop bound
> negative_returns: Using unsigned variable n_tokens in a loop exit condition.
> 
> Fixes: de3cfa2c9823 ("sched: initial import")
> 
> Signed-off-by: Slawomir Mrozowicz <slawomirx.mrozowicz@intel.com>
> ---
>  examples/qos_sched/args.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
> index 3e7fd08..7a98e5c 100644
> --- a/examples/qos_sched/args.c
> +++ b/examples/qos_sched/args.c
> @@ -162,7 +162,7 @@ static int
>  app_parse_opt_vals(const char *conf_str, char separator, uint32_t n_vals,
> uint32_t *opt_vals)
>  {
>  	char *string;
> -	uint32_t i, n_tokens;
> +	int i, n_tokens;
>  	char *tokens[MAX_OPT_VALUES];
> 
>  	if (conf_str == NULL || opt_vals == NULL || n_vals == 0 || n_vals >
> MAX_OPT_VALUES)
> --
> 1.9.1

Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] examples/qos_sched: fix out-of-bounds read
  2016-04-21 11:47 ` [PATCH v2] examples/qos_sched: fix out-of-bounds read Michal Jastrzebski
@ 2016-04-28 10:59   ` Dumitrescu, Cristian
  2016-05-16 15:35     ` Thomas Monjalon
  0 siblings, 1 reply; 7+ messages in thread
From: Dumitrescu, Cristian @ 2016-04-28 10:59 UTC (permalink / raw)
  To: Jastrzebski, MichalX K, Zhang, Roy Fan, Singh, Jasvinder
  Cc: dev, Mrozowicz, SlawomirX



> -----Original Message-----
> From: Jastrzebski, MichalX K
> Sent: Thursday, April 21, 2016 12:48 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Mrozowicz, SlawomirX
> <slawomirx.mrozowicz@intel.com>
> Subject: [PATCH v2] examples/qos_sched: fix out-of-bounds read
> 
> From: Slawomir Mrozowicz <slawomirx.mrozowicz@intel.com>
> 
> Fix issue reported by Coverity.
> 
> Coverity ID 30708: Out-of-bounds read
> overrun-local: Overrunning array tokens of 8 8-byte elements
> at element index 4294967294 (byte offset 34359738352)
> using index i (which evaluates to 4294967294).
> 
> Fixes: de3cfa2c9823 ("sched: initial import")
> 
> Signed-off-by: Slawomir Mrozowicz <slawomirx.mrozowicz@intel.com>
> ---
>  examples/qos_sched/args.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/examples/qos_sched/args.c b/examples/qos_sched/args.c
> index 3e7fd08..d819269 100644
> --- a/examples/qos_sched/args.c
> +++ b/examples/qos_sched/args.c
> @@ -175,9 +175,11 @@ app_parse_opt_vals(const char *conf_str, char
> separator, uint32_t n_vals, uint32
> 
>  	n_tokens = rte_strsplit(string, strnlen(string, 32), tokens, n_vals,
> separator);
> 
> -	for(i = 0; i < n_tokens; i++) {
> +	if (n_tokens > MAX_OPT_VALUES)
> +		return -1;
> +
> +	for (i = 0; i < n_tokens; i++)
>  		opt_vals[i] = (uint32_t)atol(tokens[i]);
> -	}
> 
>  	free(string);
> 
> --
> 1.9.1

Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] examples/qos_meter: fix unchecked return value
  2016-04-21 11:47 [PATCH v2] examples/qos_meter: fix unchecked return value Michal Jastrzebski
                   ` (2 preceding siblings ...)
       [not found] ` <1461239256-8104-4-git-send-email-michalx.k.jastrzebski@intel.com>
@ 2016-04-28 11:22 ` Dumitrescu, Cristian
  3 siblings, 0 replies; 7+ messages in thread
From: Dumitrescu, Cristian @ 2016-04-28 11:22 UTC (permalink / raw)
  To: Jastrzebski, MichalX K, Zhang, Roy Fan, Singh, Jasvinder
  Cc: dev, Mrozowicz, SlawomirX



> -----Original Message-----
> From: Jastrzebski, MichalX K
> Sent: Thursday, April 21, 2016 12:48 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; Singh, Jasvinder <jasvinder.singh@intel.com>
> Cc: dev@dpdk.org; Mrozowicz, SlawomirX
> <slawomirx.mrozowicz@intel.com>
> Subject: [PATCH v2] examples/qos_meter: fix unchecked return value
> 
> From: Slawomir Mrozowicz <slawomirx.mrozowicz@intel.com>
> 
> Fix issue reported by Coverity.
> 
> Coverity ID 30693: Unchecked return value
> check_return: Calling rte_meter_srtcm_config without checking return
> value.
> 
> Fixes: e6541fdec8b2 ("meter: initial import")
> 
> Signed-off-by: Slawomir Mrozowicz <slawomirx.mrozowicz@intel.com>
> ---
>  examples/qos_meter/main.c | 15 ++++++++++-----
>  examples/qos_meter/main.h |  2 +-
>  2 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/examples/qos_meter/main.c b/examples/qos_meter/main.c
> index b968b00..16b0b87 100644
> --- a/examples/qos_meter/main.c
> +++ b/examples/qos_meter/main.c
> @@ -133,14 +133,17 @@ struct rte_meter_trtcm_params
> app_trtcm_params[] = {
> 
>  FLOW_METER app_flows[APP_FLOWS_MAX];
> 
> -static void
> +static int
>  app_configure_flow_table(void)
>  {
>  	uint32_t i, j;
> +	int ret = 0;
> 
> -	for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) %
> RTE_DIM(PARAMS)){
> -		FUNC_CONFIG(&app_flows[i], &PARAMS[j]);
> -	}
> +	for (i = 0, j = 0; i < APP_FLOWS_MAX;
> +			i ++, j = (j + 1) % RTE_DIM(PARAMS))
> +		ret |= FUNC_CONFIG(&app_flows[i], &PARAMS[j]);
> +
> +	return ret;
>  }


You should probably return of the first error rather than carry on. How would you know which is the flow that produced the configuration error?

for (i = 0, j = 0; i < APP_FLOWS_MAX; i ++, j = (j + 1) % RTE_DIM(PARAMS)){
	ret = FUNC_CONFIG(&app_flows[i], &PARAMS[j]);
	if (ret)
		return;
}

> 
>  static inline void
> @@ -381,7 +384,9 @@ main(int argc, char **argv)
>  	rte_eth_promiscuous_enable(port_tx);
> 
>  	/* App configuration */
> -	app_configure_flow_table();
> +	ret = app_configure_flow_table();
> +	if (ret < 0)
> +		rte_exit(EXIT_FAILURE, "Invalid configure flow table\n");
> 
>  	/* Launch per-lcore init on every lcore */
>  	rte_eal_mp_remote_launch(main_loop, NULL, CALL_MASTER);
> diff --git a/examples/qos_meter/main.h b/examples/qos_meter/main.h
> index 530bf69..54867dc 100644
> --- a/examples/qos_meter/main.h
> +++ b/examples/qos_meter/main.h
> @@ -51,7 +51,7 @@ enum policer_action
> policer_table[e_RTE_METER_COLORS][e_RTE_METER_COLORS] =
>  #if APP_MODE == APP_MODE_FWD
> 
>  #define FUNC_METER(a,b,c,d) color, flow_id=flow_id, pkt_len=pkt_len,
> time=time
> -#define FUNC_CONFIG(a,b)
> +#define FUNC_CONFIG(a, b) 0
>  #define PARAMS	app_srtcm_params
>  #define FLOW_METER int
> 
> --
> 1.9.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] examples/qos_sched: fix out-of-bounds read
  2016-04-28 10:59   ` Dumitrescu, Cristian
@ 2016-05-16 15:35     ` Thomas Monjalon
  0 siblings, 0 replies; 7+ messages in thread
From: Thomas Monjalon @ 2016-05-16 15:35 UTC (permalink / raw)
  To: Mrozowicz, SlawomirX
  Cc: dev, Dumitrescu, Cristian, Jastrzebski, MichalX K, Zhang,
	Roy Fan, Singh, Jasvinder

> > Fix issue reported by Coverity.
> > 
> > Coverity ID 30708: Out-of-bounds read
> > overrun-local: Overrunning array tokens of 8 8-byte elements
> > at element index 4294967294 (byte offset 34359738352)
> > using index i (which evaluates to 4294967294).
> > 
> > Fixes: de3cfa2c9823 ("sched: initial import")
> > 
> > Signed-off-by: Slawomir Mrozowicz <slawomirx.mrozowicz@intel.com>
> 
> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>

Applied, thanks

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-05-16 15:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-21 11:47 [PATCH v2] examples/qos_meter: fix unchecked return value Michal Jastrzebski
2016-04-21 11:47 ` [PATCH v2] examples/qos_sched: fix out-of-bounds read Michal Jastrzebski
2016-04-28 10:59   ` Dumitrescu, Cristian
2016-05-16 15:35     ` Thomas Monjalon
     [not found] ` <1461239256-8104-3-git-send-email-michalx.k.jastrzebski@intel.com>
2016-04-28 10:58   ` [PATCH v2] examples/qos_sched: fix copy-paste error Dumitrescu, Cristian
     [not found] ` <1461239256-8104-4-git-send-email-michalx.k.jastrzebski@intel.com>
2016-04-28 10:58   ` [PATCH v2] examples/qos_sched: fix negative loop bound Dumitrescu, Cristian
2016-04-28 11:22 ` [PATCH v2] examples/qos_meter: fix unchecked return value Dumitrescu, Cristian

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.