Re: [ovs-dev] [PATCH v2 1/2] lib/netdev-dpdk: make device name parsing more robust

2016-02-19 Thread Ben Pfaff
Daniele, are you the right person to review and apply this pair of
patches?

Thanks,

Ben.

On Mon, Jan 25, 2016 at 02:55:57PM -0500, Mauricio Vasquez B wrote:
> Current implementation of dpdk_dev_parse_name does not perform a robust
> error handling, port names as "dpdkr", "dpdkr1x", "dpdkr 5" are considered
> valid.
> 
> With this path only positive port numbers in decimal notation are considered
> valid.
> 
> Signed-off-by: Mauricio Vasquez B 
> ---
> v2:
> - replace strtol by strtoul
> - more strict parsing, ports as "dpdr 5" are now not valid.
> Thanks to Aaron Conole!
>  lib/netdev-dpdk.c | 30 --
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index de7e488..5d09230 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -18,7 +18,9 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -187,7 +189,7 @@ struct dpdk_ring {
>  /* For the client rings */
>  struct rte_ring *cring_tx;
>  struct rte_ring *cring_rx;
> -int user_port_id; /* User given port no, parsed from port name */
> +unsigned int user_port_id; /* User given port no, parsed from port name 
> */
>  int eth_port_id; /* ethernet device port id */
>  struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
>  };
> @@ -636,18 +638,42 @@ unlock:
>  return err;
>  }
>  
> +/* dev_name must be the prefix followed by a positive decimal number.
> + * (no leading + or - signs are allowed) */
>  static int
>  dpdk_dev_parse_name(const char dev_name[], const char prefix[],
>  unsigned int *port_no)
>  {
>  const char *cport;
> +unsigned long port;
> +char *endptr;
>  
>  if (strncmp(dev_name, prefix, strlen(prefix))) {
>  return ENODEV;
>  }
>  
> +errno = 0;
>  cport = dev_name + strlen(prefix);
> -*port_no = strtol(cport, NULL, 0); /* string must be null terminated */
> +
> +if(!isdigit(cport[0])) {
> +return ENODEV;
> +}
> +
> +port = strtoul(cport, &endptr, 10);
> +
> +if(errno != 0) {
> +return errno;
> +}
> +
> +if(endptr == NULL || *endptr != '\0' || endptr == cport) {
> +return ENODEV;
> +}
> +
> +if(port > UINT_MAX) {
> +return ENODEV;
> +}
> +
> +*port_no = port;
>  return 0;
>  }
>  
> -- 
> 1.9.1
> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev


[ovs-dev] [PATCH v2 1/2] lib/netdev-dpdk: make device name parsing more robust

2016-01-25 Thread Mauricio Vasquez B
Current implementation of dpdk_dev_parse_name does not perform a robust
error handling, port names as "dpdkr", "dpdkr1x", "dpdkr 5" are considered
valid.

With this path only positive port numbers in decimal notation are considered
valid.

Signed-off-by: Mauricio Vasquez B 
---
v2:
- replace strtol by strtoul
- more strict parsing, ports as "dpdr 5" are now not valid.
Thanks to Aaron Conole!
 lib/netdev-dpdk.c | 30 --
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index de7e488..5d09230 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -18,7 +18,9 @@
 
 #include 
 #include 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -187,7 +189,7 @@ struct dpdk_ring {
 /* For the client rings */
 struct rte_ring *cring_tx;
 struct rte_ring *cring_rx;
-int user_port_id; /* User given port no, parsed from port name */
+unsigned int user_port_id; /* User given port no, parsed from port name */
 int eth_port_id; /* ethernet device port id */
 struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex);
 };
@@ -636,18 +638,42 @@ unlock:
 return err;
 }
 
+/* dev_name must be the prefix followed by a positive decimal number.
+ * (no leading + or - signs are allowed) */
 static int
 dpdk_dev_parse_name(const char dev_name[], const char prefix[],
 unsigned int *port_no)
 {
 const char *cport;
+unsigned long port;
+char *endptr;
 
 if (strncmp(dev_name, prefix, strlen(prefix))) {
 return ENODEV;
 }
 
+errno = 0;
 cport = dev_name + strlen(prefix);
-*port_no = strtol(cport, NULL, 0); /* string must be null terminated */
+
+if(!isdigit(cport[0])) {
+return ENODEV;
+}
+
+port = strtoul(cport, &endptr, 10);
+
+if(errno != 0) {
+return errno;
+}
+
+if(endptr == NULL || *endptr != '\0' || endptr == cport) {
+return ENODEV;
+}
+
+if(port > UINT_MAX) {
+return ENODEV;
+}
+
+*port_no = port;
 return 0;
 }
 
-- 
1.9.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev