* Re: [dpdk-dev] [PATCH] app/testpmd: guarantee that array access is in range
2020-02-19 12:40 [dpdk-dev] [PATCH] app/testpmd: guarantee that array access is in range Ferruh Yigit
@ 2020-02-19 13:30 ` Lipiec, Herakliusz
2020-02-19 14:02 ` Iremonger, Bernard
1 sibling, 0 replies; 4+ messages in thread
From: Lipiec, Herakliusz @ 2020-02-19 13:30 UTC (permalink / raw)
To: Yigit, Ferruh, Lu, Wenzhuo, Wu, Jingjing, Iremonger, Bernard,
Govindharajan, Hariprasad, Burakov, Anatoly
Cc: dev
> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Wednesday, February 19, 2020 12:40 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>; Lipiec, Herakliusz
> <herakliusz.lipiec@intel.com>; Govindharajan, Hariprasad
> <hariprasad.govindharajan@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: [PATCH] app/testpmd: guarantee that array access is in range
>
> Coverity complains about out of bound access, which is a false positive.
>
> The return value of the 'parse_port_list()' can't be bigger than 'maxsize'
> because of the logic in the function. ('value >= (int)maxsize'
> check and 'marked[]' usage.)
>
> But this is not explicitly clear, causing coverity warning and same question can
> be rise by reviews later.
>
> Adding a redundant check to highlight the access is in range, this is done by
> replacing existing redundant check.
>
> This is also good to protect against out out bound access in case
> 'parse_port_list()' behaviour changes later unexpectedly.
>
> Coverity issue: 354229
> Fixes: 2df00d562d20 ("app/testpmd: add --portlist option")
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> app/test-pmd/config.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> 9d9520223..d93941f03 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -2703,7 +2703,7 @@ parse_fwd_portlist(const char *portlist)
> * and thereby calculate the total number of
> * valid ports
> */
> - for (i = 0; i < portcount && valid_port_count < portcount; i++) {
> + for (i = 0; i < portcount && i < RTE_DIM(portindex); i++) {
> if (rte_eth_dev_is_valid_port(portindex[i])) {
> portindex[valid_port_count] = portindex[i];
> valid_port_count++;
> --
> 2.24.1
Reviewed-by: Herakliusz Lipiec <herakliusz.lipiec@intel.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [PATCH] app/testpmd: guarantee that array access is in range
2020-02-19 12:40 [dpdk-dev] [PATCH] app/testpmd: guarantee that array access is in range Ferruh Yigit
2020-02-19 13:30 ` Lipiec, Herakliusz
@ 2020-02-19 14:02 ` Iremonger, Bernard
2020-02-19 14:41 ` Ferruh Yigit
1 sibling, 1 reply; 4+ messages in thread
From: Iremonger, Bernard @ 2020-02-19 14:02 UTC (permalink / raw)
To: Yigit, Ferruh, Lu, Wenzhuo, Wu, Jingjing, Lipiec, Herakliusz,
Govindharajan, Hariprasad, Burakov, Anatoly
Cc: dev
> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Wednesday, February 19, 2020 12:40 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>; Lipiec, Herakliusz
> <herakliusz.lipiec@intel.com>; Govindharajan, Hariprasad
> <hariprasad.govindharajan@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>
> Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: [PATCH] app/testpmd: guarantee that array access is in range
>
> Coverity complains about out of bound access, which is a false positive.
>
> The return value of the 'parse_port_list()' can't be bigger than 'maxsize'
> because of the logic in the function. ('value >= (int)maxsize'
> check and 'marked[]' usage.)
>
> But this is not explicitly clear, causing coverity warning and same question can
> be rise by reviews later.
>
> Adding a redundant check to highlight the access is in range, this is done by
> replacing existing redundant check.
>
> This is also good to protect against out out bound access in case
> 'parse_port_list()' behaviour changes later unexpectedly.
>
> Coverity issue: 354229
> Fixes: 2df00d562d20 ("app/testpmd: add --portlist option")
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
^ permalink raw reply [flat|nested] 4+ messages in thread