pci: fix parsing of address without function number

Message ID 20181111235856.31429-1-thomas@monjalon.net (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series pci: fix parsing of address without function number |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

Thomas Monjalon Nov. 11, 2018, 11:58 p.m. UTC
  If the last part of the PCI address (function number) is missing,
the parsing was successful, assuming function 0.
The call to strtoul is not returning an error in such a case,
so an explicit check is inserted before.

This bug has always been there in older parsing macros:
	- GET_PCIADDR_FIELD
	- GET_BLACKLIST_FIELD

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Reported-by: Wisam Jaddo <wisamm@mellanox.com>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/librte_pci/rte_pci.c | 4 ++++
 1 file changed, 4 insertions(+)
  

Comments

Gaëtan Rivet Nov. 12, 2018, 9:29 a.m. UTC | #1
On Mon, Nov 12, 2018 at 12:58:56AM +0100, Thomas Monjalon wrote:
> If the last part of the PCI address (function number) is missing,
> the parsing was successful, assuming function 0.
> The call to strtoul is not returning an error in such a case,
> so an explicit check is inserted before.
> 
> This bug has always been there in older parsing macros:
> 	- GET_PCIADDR_FIELD
> 	- GET_BLACKLIST_FIELD
> 

Returning 0, that's understandable, but errno not being set
is rather lame from strtoul.

> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Reported-by: Wisam Jaddo <wisamm@mellanox.com>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>

> ---
>  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 530738dbd..f400178bb 100644
> --- a/lib/librte_pci/rte_pci.c
> +++ b/lib/librte_pci/rte_pci.c
> @@ -30,6 +30,10 @@ get_u8_pciaddr_field(const char *in, void *_u8, char dlm)
>  	uint8_t *u8 = _u8;
>  	char *end;
>  
> +	/* empty string is an error though strtoul() returns 0 */
> +	if (*in == '\0')
> +		return NULL;
> +
>  	errno = 0;
>  	val = strtoul(in, &end, 16);
>  	if (errno != 0 || end[0] != dlm || val > UINT8_MAX) {
> -- 
> 2.19.0
>
  
Thomas Monjalon Nov. 13, 2018, 5 p.m. UTC | #2
12/11/2018 10:29, Gaëtan Rivet:
> On Mon, Nov 12, 2018 at 12:58:56AM +0100, Thomas Monjalon wrote:
> > If the last part of the PCI address (function number) is missing,
> > the parsing was successful, assuming function 0.
> > The call to strtoul is not returning an error in such a case,
> > so an explicit check is inserted before.
> > 
> > This bug has always been there in older parsing macros:
> > 	- GET_PCIADDR_FIELD
> > 	- GET_BLACKLIST_FIELD
> > 
> 
> Returning 0, that's understandable, but errno not being set
> is rather lame from strtoul.
> 
> > Fixes: af75078fece3 ("first public release")
> > Cc: stable@dpdk.org
> > 
> > Reported-by: Wisam Jaddo <wisamm@mellanox.com>
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>

Applied
  

Patch

diff --git a/lib/librte_pci/rte_pci.c b/lib/librte_pci/rte_pci.c
index 530738dbd..f400178bb 100644
--- a/lib/librte_pci/rte_pci.c
+++ b/lib/librte_pci/rte_pci.c
@@ -30,6 +30,10 @@  get_u8_pciaddr_field(const char *in, void *_u8, char dlm)
 	uint8_t *u8 = _u8;
 	char *end;
 
+	/* empty string is an error though strtoul() returns 0 */
+	if (*in == '\0')
+		return NULL;
+
 	errno = 0;
 	val = strtoul(in, &end, 16);
 	if (errno != 0 || end[0] != dlm || val > UINT8_MAX) {