diff mbox series

[v3] net/ring: fix unchecked return value

Message ID 20201013130704.1186595-1-kevin.laatz@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series [v3] net/ring: fix unchecked return value |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/travis-robot success Travis build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Kevin Laatz Oct. 13, 2020, 1:07 p.m. UTC 
  Add a check for the return value of the sscanf call in
parse_internal_args(), returning an error if we don't get the expected
result.

Coverity issue: 362049
Fixes: 96cb19521147 ("net/ring: use EAL APIs in PMD specific API")
Cc: stable@dpdk.org

Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>

---
v2: added consumed characters count check

v3: add more improved checks
---
 drivers/net/ring/rte_eth_ring.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Ferruh Yigit Oct. 13, 2020, 5:23 p.m. UTC | #1 
On 10/13/2020 2:07 PM, Kevin Laatz wrote:
> Add a check for the return value of the sscanf call in
> parse_internal_args(), returning an error if we don't get the expected
> result.
> 
> Coverity issue: 362049
> Fixes: 96cb19521147 ("net/ring: use EAL APIs in PMD specific API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>

<...>

> +#define ETH_RING_INTERNAL_ARG_MAX_LEN	19

Added following comment while merging: /* "0x..16chars..\0" */

<...>

> -	sscanf(value, "%p", &args);
> +	/* make sure 'value' is valid pointer length */
> +	if (strnlen(value, ETH_RING_INTERNAL_ARG_MAX_LEN) >=
> +			ETH_RING_INTERNAL_ARG_MAX_LEN) {
> +		PMD_LOG(ERR, "Error parsing internal args, 'value' too long");

'value' is variable name and may not fit to the debug log.
Replaced with "..., argument is too long" while merging.

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
Applied to dpdk-next-net/main, thanks.
diff mbox series

Patch

diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 40fe1ca4ba..41692305e7 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -16,6 +16,7 @@ 
 #define ETH_RING_ACTION_CREATE		"CREATE"
 #define ETH_RING_ACTION_ATTACH		"ATTACH"
 #define ETH_RING_INTERNAL_ARG		"internal"
+#define ETH_RING_INTERNAL_ARG_MAX_LEN	19
 
 static const char *valid_arguments[] = {
 	ETH_RING_NUMA_NODE_ACTION_ARG,
@@ -538,8 +539,21 @@  parse_internal_args(const char *key __rte_unused, const char *value,
 {
 	struct ring_internal_args **internal_args = data;
 	void *args;
+	int ret, n;
 
-	sscanf(value, "%p", &args);
+	/* make sure 'value' is valid pointer length */
+	if (strnlen(value, ETH_RING_INTERNAL_ARG_MAX_LEN) >=
+			ETH_RING_INTERNAL_ARG_MAX_LEN) {
+		PMD_LOG(ERR, "Error parsing internal args, 'value' too long");
+		return -1;
+	}
+
+	ret = sscanf(value, "%p%n", &args, &n);
+	if (ret == 0 || (size_t)n != strlen(value)) {
+		PMD_LOG(ERR, "Error parsing internal args");
+
+		return -1;
+	}
 
 	*internal_args = args;