From: Ferruh Yigit <ferruh.yigit@intel.com> To: Kevin Laatz <kevin.laatz@intel.com>, dev@dpdk.org Cc: bruce.richardson@intel.com, stephen@networkplumber.org, stable@dpdk.org Subject: Re: [dpdk-stable] [PATCH v2] net/ring: fix unchecked return value Date: Mon, 12 Oct 2020 12:57:11 +0100 Message-ID: <307b8070-977d-187f-c9c0-4b97477eda98@intel.com> (raw) In-Reply-To: <20201001170902.487111-1-kevin.laatz@intel.com> On 10/1/2020 6:09 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> > > --- > v2: added consumed characters count check > --- > drivers/net/ring/rte_eth_ring.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c > index 40fe1ca4ba..66367465fd 100644 > --- a/drivers/net/ring/rte_eth_ring.c > +++ b/drivers/net/ring/rte_eth_ring.c > @@ -538,8 +538,13 @@ parse_internal_args(const char *key __rte_unused, const char *value, > { > struct ring_internal_args **internal_args = data; > void *args; > + int n; > > - sscanf(value, "%p", &args); > + if (sscanf(value, "%p%n", &args, &n) != 1 || (size_t)n != strlen(value)) { two small details, 1- I see following note in the sscanf manual: https://linux.die.net/man/3/sscanf " The C standard says: "Execution of a %n directive does not increment the assignment count returned at the completion of execution" but the Corrigendum seems to contradict this. Probably it is wise not to make any assumptions on the effect of %n conversions on the return value. " So what do you think checking return value as " == 0" ? 2) If the 'value' is more than a pointer can hold, like "0xbbbbbbbbbbbbbbbbbb", the arg will be '0xffffffffffffffff', the 'n' will be 20. The "(size_t)n != strlen(value)" check doesn't catch this. What do you think adding another "strnlen(value, 18)", since 18 can be the largest pointer, even before 'sscanf()' ? This also protects against strlen with non-null terminated 'value'. > + PMD_LOG(ERR, "Error parsing internal args"); > + > + return -1; > + } > > *internal_args = args; > >
next prev parent reply other threads:[~2020-10-12 11:57 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-22 17:20 [dpdk-stable] [PATCH] " Kevin Laatz 2020-09-23 8:06 ` [dpdk-stable] [dpdk-dev] " David Marchand 2020-09-23 9:39 ` 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 [this message] 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=307b8070-977d-187f-c9c0-4b97477eda98@intel.com \ --to=ferruh.yigit@intel.com \ --cc=bruce.richardson@intel.com \ --cc=dev@dpdk.org \ --cc=kevin.laatz@intel.com \ --cc=stable@dpdk.org \ --cc=stephen@networkplumber.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