From: David Marchand <david.marchand@redhat.com>
To: Kevin Laatz <kevin.laatz@intel.com>
Cc: dev <dev@dpdk.org>, "Yigit, Ferruh" <ferruh.yigit@intel.com>,
Bruce Richardson <bruce.richardson@intel.com>,
dpdk stable <stable@dpdk.org>
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] net/ring: fix unchecked return value
Date: Wed, 23 Sep 2020 10:06:25 +0200
Message-ID: <CAJFAV8z-XF2dr0ZAUL6AFsGzGzz5kxpHvRBVxrxZsgZm1PHU7Q@mail.gmail.com> (raw)
In-Reply-To: <20200922172015.266698-1-kevin.laatz@intel.com>
On Tue, Sep 22, 2020 at 7:25 PM Kevin Laatz <kevin.laatz@intel.com> 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>
> ---
> drivers/net/ring/rte_eth_ring.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
> index 40fe1ca4ba..62060e46ce 100644
> --- a/drivers/net/ring/rte_eth_ring.c
> +++ b/drivers/net/ring/rte_eth_ring.c
> @@ -539,7 +539,8 @@ parse_internal_args(const char *key __rte_unused, const char *value,
> struct ring_internal_args **internal_args = data;
> void *args;
>
> - sscanf(value, "%p", &args);
> + if (sscanf(value, "%p", &args) != 1)
> + return -1;
Not sure this really needs fixing, as I understood the internal option
is something only the driver uses.
On the patch itself, sscanf stops at the first character it deems
incorrect, meaning that you would not detect trailing chars, like for
0x1234Z.
You can detect this by adding a canary.
$ cat sscanf.c
#include <stdio.h>
int main(int argc, char *argv[])
{
void *args;
char c;
if (sscanf(argv[1], "%p", &args) != 1)
printf("'%%p' KO for %s\n", argv[1]);
else
printf("'%%p' ok for %s\n", argv[1]);
if (sscanf(argv[1], "%p%c", &args, &c) != 1)
printf("'%%p%%c' KO for %s\n", argv[1]);
else
printf("'%%p%%c' ok for %s\n", argv[1]);
return 0;
}
$ gcc -o sscanf -Wall -Werror sscanf.c
$ ./sscanf 0x1234
'%p' ok for 0x1234
'%p%c' ok for 0x1234
$ ./sscanf 0x1234Z
'%p' ok for 0x1234Z
'%p%c' KO for 0x1234Z
--
David Marchand
next prev parent reply other threads:[~2020-09-23 8:06 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-22 17:20 [dpdk-stable] " Kevin Laatz
2020-09-23 8:06 ` David Marchand [this message]
2020-09-23 9:39 ` [dpdk-stable] [dpdk-dev] " Bruce Richardson
2020-09-23 9:43 ` David Marchand
2020-09-23 10:04 ` Kevin Laatz
2020-09-23 10:25 ` Bruce Richardson
2020-09-25 12:43 ` [dpdk-stable] " Ferruh Yigit
2020-10-01 14:14 ` Kevin Laatz
2020-10-01 14:51 ` Ferruh Yigit
2020-10-01 17:09 ` [dpdk-stable] [PATCH v2] " Kevin Laatz
2020-10-12 11:57 ` Ferruh Yigit
2020-10-12 12:45 ` Bruce Richardson
2020-10-12 13:04 ` Ferruh Yigit
2020-10-12 13:11 ` Bruce Richardson
2020-10-13 13:07 ` [dpdk-stable] [PATCH v3] " Kevin Laatz
2020-10-13 17:23 ` Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAJFAV8z-XF2dr0ZAUL6AFsGzGzz5kxpHvRBVxrxZsgZm1PHU7Q@mail.gmail.com \
--to=david.marchand@redhat.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=kevin.laatz@intel.com \
--cc=stable@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
patches for DPDK stable branches
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://inbox.dpdk.org/stable/0 stable/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 stable stable/ https://inbox.dpdk.org/stable \
stable@dpdk.org
public-inbox-index stable
Example config snippet for mirrors.
Newsgroup available over NNTP:
nntp://inbox.dpdk.org/inbox.dpdk.stable
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git