diff mbox series

app/testpmd: guarantee that array access is in range

Message ID 20200219124003.1025267-1-ferruh.yigit@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers show
Series app/testpmd: guarantee that array access is in range | expand

Checks

Context Check Description
ci/travis-robot success Travis build: passed
ci/Intel-compilation fail apply issues
ci/iol-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/checkpatch success coding style OK

Commit Message

Ferruh Yigit Feb. 19, 2020, 12:40 p.m. UTC
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(-)

Comments

Lipiec, Herakliusz Feb. 19, 2020, 1:30 p.m. UTC | #1
> -----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>
Iremonger, Bernard Feb. 19, 2020, 2:02 p.m. UTC | #2
> -----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>
Ferruh Yigit Feb. 19, 2020, 2:41 p.m. UTC | #3
On 2/19/2020 2:02 PM, Iremonger, Bernard wrote:
> 
> 
>> -----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>
> 

Applied to dpdk-next-net/master, thanks.
diff mbox series

Patch

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++;