From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id 1C0F645B6A;
Fri, 18 Oct 2024 19:35:57 +0200 (CEST)
Received: from mails.dpdk.org (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id B431040267;
Fri, 18 Oct 2024 19:35:56 +0200 (CEST)
Received: from mail-pj1-f43.google.com (mail-pj1-f43.google.com
[209.85.216.43]) by mails.dpdk.org (Postfix) with ESMTP id 57EF04025C
for <dev@dpdk.org>; Fri, 18 Oct 2024 19:35:55 +0200 (CEST)
Received: by mail-pj1-f43.google.com with SMTP id
98e67ed59e1d1-2e2b9480617so1908594a91.1
for <dev@dpdk.org>; Fri, 18 Oct 2024 10:35:55 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1729272954;
x=1729877754; darn=dpdk.org;
h=content-transfer-encoding:mime-version:references:in-reply-to
:message-id:subject:cc:to:from:date:from:to:cc:subject:date
:message-id:reply-to;
bh=ve9ZT/ol2ErVERsPcpka0+I/DujmVcZoFaLjpIJc9rc=;
b=Ptj3VJ/BuofbgPIfPWtj1zr53gfNypOfgk2w9sCVxXvGLRsnn6Jrb2ai6MghorM+nh
NYXoqh3siIWXLSsjmdPg+pkfHuvGoGF6chFHVoVniU63RVz5hhbL8R8cYOWhM48TbenG
VPXbD/XZh33aye7N+kpDQRILgVmYX9d6lcr6q7DalcuS3tldIV+o+A12osflEIp2b7Js
zm9o5vuS0xK16dS2sgOsaLKlJxF67iAagnjvOQTo1hUG08lRPTPzQn61SZ+9inUmgBzN
BEx/i9mnCZC211/1vmJDwCGldeZhTms1gqh9svXtaQNkKVfJ5xSQZqhKbCBIaft7NKBp
TM2A==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20230601; t=1729272954; x=1729877754;
h=content-transfer-encoding:mime-version:references:in-reply-to
:message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc
:subject:date:message-id:reply-to;
bh=ve9ZT/ol2ErVERsPcpka0+I/DujmVcZoFaLjpIJc9rc=;
b=lRnBkGr/vuTyW/dzDTE6L3IDLIlyNMPJCjoE7rlz0o1f3Joj1WWH5IekdRrE6BJosc
PJJ835If0+f1svdXYo+3i29jsKm9eg6bhmT0BYM0zo1ijbNxDHdP3B/2vR0bb81PRyOO
R96WskFhzK4de9QqVEAyCLeh0gpgyi6WUMZH6EfspZkIK+M/8xgrfOL4Ywr43Lcj7bYT
FtCuTq0/vpxNFzCnaXDDGmSU7goAXuqdfSK/6auk/JywRFOKmESY+IjQQ5Y9KzQBZmr3
tWxoPHQe5IPOFJ8AMFgvaJBq4mgcBLCEY1LznyACBmj2M928HxqhwIfCUTCk7m8UFPYn
2UsQ==
X-Forwarded-Encrypted: i=1;
AJvYcCWQ/FQhK9duBr0UZZfrcnwldFXH7PpnXQQ23C+CglfKK1F1aQnnSCfICfUSBP8i6ytaVO8=@dpdk.org
X-Gm-Message-State: AOJu0YyANb66UWQP18r4rMer3kMb47p8DJWVGfauD+Zo1OFmoZrvt4hQ
7SbOhooUOzpS6xBsZ5tQNmBl5ujDwAryrN9ZHh1fs2TfNPCM6SzlfkJ5xYvRVyk=
X-Google-Smtp-Source: AGHT+IFsk5Ovo/A9OfO5sHUkAIzeGso9Vuudg43DnG54LLUM6VUtN+MwHgYaqqEMpDabPN2kRK0voA==
X-Received: by 2002:a17:90a:7408:b0:2e2:cfef:86 with SMTP id
98e67ed59e1d1-2e5616dea35mr3835250a91.4.1729272954296;
Fri, 18 Oct 2024 10:35:54 -0700 (PDT)
Received: from hermes.local (204-195-96-226.wavecable.com. [204.195.96.226])
by smtp.gmail.com with ESMTPSA id
98e67ed59e1d1-2e55d7bf525sm2400349a91.15.2024.10.18.10.35.53
(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
Fri, 18 Oct 2024 10:35:54 -0700 (PDT)
Date: Fri, 18 Oct 2024 10:35:51 -0700
From: Stephen Hemminger <stephen@networkplumber.org>
To: Hanumanth Pothula <hpothula@marvell.com>
Cc: Jerin Jacob <jerinj@marvell.com>, <dev@dpdk.org>, <hkalra@marvell.com>
Subject: Re: [PATCH] event/octeontx: resolve possible integer overflow
Message-ID: <20241018103551.2563fcfa@hermes.local>
In-Reply-To: <20241018075903.53757-1-hpothula@marvell.com>
References: <20241018075903.53757-1-hpothula@marvell.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
On Fri, 18 Oct 2024 13:29:03 +0530
Hanumanth Pothula <hpothula@marvell.com> wrote:
> The last argument passed to ssovf_parsekv() is an
> unsigned char*, but it is accessed as an integer.
> This can lead to an integer overflow.
>
> Hence, make ensure the argument is accessed as a char
> and for better error handling use strtol instead of atoi.
>
> Signed-off-by: Hanumanth Pothula <hpothula@marvell.com>
> ---
> drivers/event/octeontx/ssovf_evdev.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/event/octeontx/ssovf_evdev.c b/drivers/event/octeontx/ssovf_evdev.c
> index 3a933b1db7..ccb447d33a 100644
> --- a/drivers/event/octeontx/ssovf_evdev.c
> +++ b/drivers/event/octeontx/ssovf_evdev.c
> @@ -719,8 +719,16 @@ ssovf_close(struct rte_eventdev *dev)
> static int
> ssovf_parsekv(const char *key __rte_unused, const char *value, void *opaque)
> {
> - int *flag = opaque;
> - *flag = !!atoi(value);
> + uint8_t *flag = (uint8_t *)opaque;
> + char *end;
> +
> + errno = 0;
> + *flag = (uint8_t)strtol(value, &end, 2);
> + if ((errno != 0) || (value == end)) {
> + ssovf_log_err("fail to get key val ret:%d err:%d", *flag, errno);
> + return -EINVAL;
> + }
> +
> return 0;
> }
Cast of opaque is unnecessary in C.
Use strtoul to avoid allowing negative numbers.
Passing 2 as argument makes it assume binary so 101 is legal value and returns 5
and it is not helping.
Why not:
diff --git a/drivers/event/octeontx/ssovf_evdev.c b/drivers/event/octeontx/ssovf_evdev.c
index 3a933b1db7..9804f5bc59 100644
--- a/drivers/event/octeontx/ssovf_evdev.c
+++ b/drivers/event/octeontx/ssovf_evdev.c
@@ -717,10 +717,20 @@ ssovf_close(struct rte_eventdev *dev)
}
static int
-ssovf_parsekv(const char *key __rte_unused, const char *value, void *opaque)
+ssovf_parsekv(const char *key, const char *value, void *opaque)
{
- int *flag = opaque;
- *flag = !!atoi(value);
+ uint8_t *flag = opaque;
+ unsigned long v;
+ char *end;
+
+ errno = 0;
+ v = strtoul(value, &end, 0);
+ if (errno != 0 || end == value || *end != '\0') {
+ ssvf_log_err("invalid %s value %s", key, value);
+ return -EINVAL;
+ }
+
+ *flag = !!v;
return 0;
}