* [dpdk-dev] [PATCH v1 0/2] pci: a comment and a minor fix @ 2020-05-13 10:47 Gaetan Rivet 2020-05-13 10:47 ` [dpdk-dev] [PATCH v1 1/2] pci: fix allowing underflow when parsing PCI id Gaetan Rivet 2020-05-13 10:47 ` [dpdk-dev] [PATCH v1 2/2] pci: explain how empty strings are rejected in DBDF Gaetan Rivet 0 siblings, 2 replies; 7+ messages in thread From: Gaetan Rivet @ 2020-05-13 10:47 UTC (permalink / raw) To: dev Add a clarifying comment and fix the PCI addr parser behavior for some edge-case. Gaetan Rivet (2): pci: fix allowing underflow when parsing PCI id pci: explain how empty strings are rejected in DBDF lib/librte_pci/rte_pci.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) -- 2.26.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [dpdk-dev] [PATCH v1 1/2] pci: fix allowing underflow when parsing PCI id 2020-05-13 10:47 [dpdk-dev] [PATCH v1 0/2] pci: a comment and a minor fix Gaetan Rivet @ 2020-05-13 10:47 ` Gaetan Rivet 2020-05-14 8:52 ` Stojaczyk, Dariusz 2020-05-19 9:17 ` David Marchand 2020-05-13 10:47 ` [dpdk-dev] [PATCH v1 2/2] pci: explain how empty strings are rejected in DBDF Gaetan Rivet 1 sibling, 2 replies; 7+ messages in thread From: Gaetan Rivet @ 2020-05-13 10:47 UTC (permalink / raw) To: dev; +Cc: stable The function strtoul will not return ERANGE if the input is negative, as one might expect. 0000:-FFFFFFFFFFFFFFFB:00.0 is not a better way to write 0000:05:00.0. To simplify checking for '-', forbid using spaces before the field value. 0000: 00: 2c.0 Should not be accepted. Fixes: af75078fece3 ("first public release") Cc: stable@dpdk.org Signed-off-by: Gaetan Rivet <grive@u256.net> --- lib/librte_pci/rte_pci.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/librte_pci/rte_pci.c b/lib/librte_pci/rte_pci.c index d1ab6b414..e4ecdc32f 100644 --- a/lib/librte_pci/rte_pci.c +++ b/lib/librte_pci/rte_pci.c @@ -35,6 +35,12 @@ get_u8_pciaddr_field(const char *in, void *_u8, char dlm) if (*in == '\0') return NULL; + /* PCI field starting with spaces is forbidden. + * Negative wrap-around is not reported as an error by strtoul. + */ + if (*in == ' ' || *in == '-') + return NULL; + errno = 0; val = strtoul(in, &end, 16); if (errno != 0 || end[0] != dlm || val > UINT8_MAX) { @@ -70,6 +76,12 @@ pci_dbdf_parse(const char *input, struct rte_pci_addr *dev_addr) unsigned long val; char *end; + /* PCI id starting with spaces is forbidden. + * Negative wrap-around is not reported as an error by strtoul. + */ + if (*in == ' ' || *in == '-') + return EINVAL; + errno = 0; val = strtoul(in, &end, 16); if (errno != 0 || end[0] != ':' || val > UINT16_MAX) -- 2.26.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v1 1/2] pci: fix allowing underflow when parsing PCI id 2020-05-13 10:47 ` [dpdk-dev] [PATCH v1 1/2] pci: fix allowing underflow when parsing PCI id Gaetan Rivet @ 2020-05-14 8:52 ` Stojaczyk, Dariusz 2020-05-19 9:17 ` David Marchand 1 sibling, 0 replies; 7+ messages in thread From: Stojaczyk, Dariusz @ 2020-05-14 8:52 UTC (permalink / raw) To: Gaetan Rivet, dev; +Cc: stable > -----Original Message----- > From: dev <dev-bounces@dpdk.org> On Behalf Of Gaetan Rivet > Sent: Wednesday, May 13, 2020 12:48 PM > To: dev@dpdk.org > Cc: stable@dpdk.org > Subject: [dpdk-dev] [PATCH v1 1/2] pci: fix allowing underflow when parsing PCI > id > > The function strtoul will not return ERANGE if the input is negative, as > one might expect. > > 0000:-FFFFFFFFFFFFFFFB:00.0 > > is not a better way to write 0000:05:00.0. > To simplify checking for '-', forbid using spaces before the field value. > > 0000: 00: 2c.0 > > Should not be accepted. > > Fixes: af75078fece3 ("first public release") > Cc: stable@dpdk.org > Signed-off-by: Gaetan Rivet <grive@u256.net> > --- Acked-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v1 1/2] pci: fix allowing underflow when parsing PCI id 2020-05-13 10:47 ` [dpdk-dev] [PATCH v1 1/2] pci: fix allowing underflow when parsing PCI id Gaetan Rivet 2020-05-14 8:52 ` Stojaczyk, Dariusz @ 2020-05-19 9:17 ` David Marchand 1 sibling, 0 replies; 7+ messages in thread From: David Marchand @ 2020-05-19 9:17 UTC (permalink / raw) To: Gaetan Rivet; +Cc: dev, dpdk stable, Luca Boccassi, Kevin Traynor On Wed, May 13, 2020 at 12:48 PM Gaetan Rivet <grive@u256.net> wrote: > > The function strtoul will not return ERANGE if the input is negative, as > one might expect. > > 0000:-FFFFFFFFFFFFFFFB:00.0 > > is not a better way to write 0000:05:00.0. > To simplify checking for '-', forbid using spaces before the field value. > > 0000: 00: 2c.0 > > Should not be accepted. > > Fixes: af75078fece3 ("first public release") > Cc: stable@dpdk.org Not sure about backporting this one, will let stable maintainers reconsider this. > Signed-off-by: Gaetan Rivet <grive@u256.net> Acked-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com> > --- > lib/librte_pci/rte_pci.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/lib/librte_pci/rte_pci.c b/lib/librte_pci/rte_pci.c > index d1ab6b414..e4ecdc32f 100644 > --- a/lib/librte_pci/rte_pci.c > +++ b/lib/librte_pci/rte_pci.c > @@ -35,6 +35,12 @@ get_u8_pciaddr_field(const char *in, void *_u8, char dlm) > if (*in == '\0') > return NULL; > > + /* PCI field starting with spaces is forbidden. > + * Negative wrap-around is not reported as an error by strtoul. > + */ > + if (*in == ' ' || *in == '-') > + return NULL; > + > errno = 0; > val = strtoul(in, &end, 16); > if (errno != 0 || end[0] != dlm || val > UINT8_MAX) { > @@ -70,6 +76,12 @@ pci_dbdf_parse(const char *input, struct rte_pci_addr *dev_addr) > unsigned long val; > char *end; > > + /* PCI id starting with spaces is forbidden. > + * Negative wrap-around is not reported as an error by strtoul. > + */ > + if (*in == ' ' || *in == '-') > + return EINVAL; Should be -EINVAL, fixed. > + > errno = 0; > val = strtoul(in, &end, 16); > if (errno != 0 || end[0] != ':' || val > UINT16_MAX) > -- > 2.26.2 > Applied, thanks. -- David Marchand ^ permalink raw reply [flat|nested] 7+ messages in thread
* [dpdk-dev] [PATCH v1 2/2] pci: explain how empty strings are rejected in DBDF 2020-05-13 10:47 [dpdk-dev] [PATCH v1 0/2] pci: a comment and a minor fix Gaetan Rivet 2020-05-13 10:47 ` [dpdk-dev] [PATCH v1 1/2] pci: fix allowing underflow when parsing PCI id Gaetan Rivet @ 2020-05-13 10:47 ` Gaetan Rivet 2020-05-14 8:53 ` Stojaczyk, Dariusz 2020-05-19 9:18 ` David Marchand 1 sibling, 2 replies; 7+ messages in thread From: Gaetan Rivet @ 2020-05-13 10:47 UTC (permalink / raw) To: dev Empty strings are forbidden as input to rte_pci_addr_parse(). It is explicitly enforced in BDF parsing as parsing the bus field will immediately fail. The related check is commented. It is implicitly enforced in DBDF parsing, as the domain would be parsed to 0 without error, but the check `end[0] != ':'` afterward will return -EINVAL. Enforcing consistency between parsers by reading the code is not helped by this property being implicit. Add a comment to explain. Signed-off-by: Gaetan Rivet <grive@u256.net> --- lib/librte_pci/rte_pci.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/librte_pci/rte_pci.c b/lib/librte_pci/rte_pci.c index e4ecdc32f..60e6fbae7 100644 --- a/lib/librte_pci/rte_pci.c +++ b/lib/librte_pci/rte_pci.c @@ -84,6 +84,10 @@ pci_dbdf_parse(const char *input, struct rte_pci_addr *dev_addr) errno = 0; val = strtoul(in, &end, 16); + /* Empty string is not an error for strtoul, but the check + * end[0] != ':' + * will detect the issue. + */ if (errno != 0 || end[0] != ':' || val > UINT16_MAX) return -EINVAL; dev_addr->domain = (uint16_t)val; -- 2.26.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v1 2/2] pci: explain how empty strings are rejected in DBDF 2020-05-13 10:47 ` [dpdk-dev] [PATCH v1 2/2] pci: explain how empty strings are rejected in DBDF Gaetan Rivet @ 2020-05-14 8:53 ` Stojaczyk, Dariusz 2020-05-19 9:18 ` David Marchand 1 sibling, 0 replies; 7+ messages in thread From: Stojaczyk, Dariusz @ 2020-05-14 8:53 UTC (permalink / raw) To: Gaetan Rivet, dev > -----Original Message----- > From: dev <dev-bounces@dpdk.org> On Behalf Of Gaetan Rivet > Sent: Wednesday, May 13, 2020 12:48 PM > To: dev@dpdk.org > Subject: [dpdk-dev] [PATCH v1 2/2] pci: explain how empty strings are rejected > in DBDF > > Empty strings are forbidden as input to rte_pci_addr_parse(). > It is explicitly enforced in BDF parsing as parsing the bus > field will immediately fail. The related check is commented. > > It is implicitly enforced in DBDF parsing, as the domain would be > parsed to 0 without error, but the check `end[0] != ':'` afterward > will return -EINVAL. > > Enforcing consistency between parsers by reading the code is not helped > by this property being implicit. Add a comment to explain. > > Signed-off-by: Gaetan Rivet <grive@u256.net> > --- Acked-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v1 2/2] pci: explain how empty strings are rejected in DBDF 2020-05-13 10:47 ` [dpdk-dev] [PATCH v1 2/2] pci: explain how empty strings are rejected in DBDF Gaetan Rivet 2020-05-14 8:53 ` Stojaczyk, Dariusz @ 2020-05-19 9:18 ` David Marchand 1 sibling, 0 replies; 7+ messages in thread From: David Marchand @ 2020-05-19 9:18 UTC (permalink / raw) To: Gaetan Rivet; +Cc: dev On Wed, May 13, 2020 at 12:48 PM Gaetan Rivet <grive@u256.net> wrote: > > Empty strings are forbidden as input to rte_pci_addr_parse(). > It is explicitly enforced in BDF parsing as parsing the bus > field will immediately fail. The related check is commented. > > It is implicitly enforced in DBDF parsing, as the domain would be > parsed to 0 without error, but the check `end[0] != ':'` afterward > will return -EINVAL. > > Enforcing consistency between parsers by reading the code is not helped > by this property being implicit. Add a comment to explain. > > Signed-off-by: Gaetan Rivet <grive@u256.net> Acked-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com> Applied, thanks. -- David Marchand ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-05-19 9:18 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-13 10:47 [dpdk-dev] [PATCH v1 0/2] pci: a comment and a minor fix Gaetan Rivet 2020-05-13 10:47 ` [dpdk-dev] [PATCH v1 1/2] pci: fix allowing underflow when parsing PCI id Gaetan Rivet 2020-05-14 8:52 ` Stojaczyk, Dariusz 2020-05-19 9:17 ` David Marchand 2020-05-13 10:47 ` [dpdk-dev] [PATCH v1 2/2] pci: explain how empty strings are rejected in DBDF Gaetan Rivet 2020-05-14 8:53 ` Stojaczyk, Dariusz 2020-05-19 9:18 ` David Marchand
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).