[ovs-dev] [PATCH 4/4] datapath: Fix strncpy() errors

Russell Bryant rbryant at redhat.com
Fri Feb 20 18:22:17 UTC 2015


strncpy() has pretty annoying semantics.  One bit that is odd is that
it does not guarantee that the destination string will be null
terminated.  If no terminator is found in the first N bytes of the
source, it just fills the destination buffer and doesn't terminate it.

The changes here are to address cases where the destination could be
left without a terminator.  All of these cases were using a constant
for the 3rd argument.  These changes prefer using sizeof on the
destination where possible, even though it results in the same thing.
In cases where we know the destination has been zeroed out, the fix is
just subtracting 1 from the destination size.  In one case, it wasn't
obvious to me whether explicitly writing a terminating byte was required,
so I included it.

Signed-off-by: Russell Bryant <rbryant at redhat.com>
---
 datapath/linux/compat/genetlink-openvswitch.c | 3 ++-
 datapath/vport-gre.c                          | 4 ++--
 datapath/vport-lisp.c                         | 2 +-
 datapath/vport-vxlan.c                        | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/datapath/linux/compat/genetlink-openvswitch.c b/datapath/linux/compat/genetlink-openvswitch.c
index 08f0fab..bea2acb 100644
--- a/datapath/linux/compat/genetlink-openvswitch.c
+++ b/datapath/linux/compat/genetlink-openvswitch.c
@@ -28,7 +28,8 @@ int rpl___genl_register_family(struct rpl_genl_family *f)
 
 	f->compat_family.id = f->id;
 	f->compat_family.hdrsize = f->hdrsize;
-	strncpy(f->compat_family.name, f->name, GENL_NAMSIZ);
+	strncpy(f->compat_family.name, f->name, sizeof(f->compat_family.name) - 1);
+	f->compat_family.name[sizeof(f->compat_family.name) - 1] = '\0';
 	f->compat_family.version = f->version;
 	f->compat_family.maxattr = f->maxattr;
 	f->compat_family.netnsok = f->netnsok;
diff --git a/datapath/vport-gre.c b/datapath/vport-gre.c
index c0ed009..6df6b08 100644
--- a/datapath/vport-gre.c
+++ b/datapath/vport-gre.c
@@ -264,7 +264,7 @@ static struct vport *gre_create(const struct vport_parms *parms)
 	if (IS_ERR(vport))
 		goto error;
 
-	strncpy(vport_priv(vport), parms->name, IFNAMSIZ);
+	strncpy(vport_priv(vport), parms->name, IFNAMSIZ - 1);
 	rcu_assign_pointer(ovs_net->vport_net.gre_vport, vport);
 	return vport;
 
@@ -339,7 +339,7 @@ static struct vport *gre64_create(const struct vport_parms *parms)
 	if (IS_ERR(vport))
 		goto error;
 
-	strncpy(vport_priv(vport), parms->name, IFNAMSIZ);
+	strncpy(vport_priv(vport), parms->name, IFNAMSIZ - 1);
 	rcu_assign_pointer(ovs_net->vport_net.gre64_vport, vport);
 	return vport;
 error:
diff --git a/datapath/vport-lisp.c b/datapath/vport-lisp.c
index db4d06f..3e44a9b 100644
--- a/datapath/vport-lisp.c
+++ b/datapath/vport-lisp.c
@@ -382,7 +382,7 @@ static struct vport *lisp_tnl_create(const struct vport_parms *parms)
 
 	lisp_port = lisp_vport(vport);
 	lisp_port->dst_port = htons(dst_port);
-	strncpy(lisp_port->name, parms->name, IFNAMSIZ);
+	strncpy(lisp_port->name, parms->name, sizeof(lisp_port->name) - 1);
 
 	err = lisp_socket_init(lisp_port, net);
 	if (err)
diff --git a/datapath/vport-vxlan.c b/datapath/vport-vxlan.c
index 7fcb88a..ec83bf0 100644
--- a/datapath/vport-vxlan.c
+++ b/datapath/vport-vxlan.c
@@ -176,7 +176,7 @@ static struct vport *vxlan_tnl_create(const struct vport_parms *parms)
 		return vport;
 
 	vxlan_port = vxlan_vport(vport);
-	strncpy(vxlan_port->name, parms->name, IFNAMSIZ);
+	strncpy(vxlan_port->name, parms->name, sizeof(vxlan_port->name) - 1);
 
 	a = nla_find_nested(options, OVS_TUNNEL_ATTR_EXTENSION);
 	if (a) {
-- 
2.1.0




More information about the dev mailing list