[ovs-dev] [PATCH 3/3] datapath: Check device name length more carefully in create_dp().

Ben Pfaff blp at nicira.com
Tue Apr 27 19:42:20 UTC 2010


On Tue, Apr 27, 2010 at 11:32:42AM -0700, Jesse Gross wrote:
> On Tue, Apr 27, 2010 at 11:20 AM, Ben Pfaff <blp at nicira.com> wrote:
> 
> > On Tue, Apr 27, 2010 at 11:08:48AM -0700, Jesse Gross wrote:
> > > There are some other places where we copy device names from userspace as
> > > well.  I don't think that any of them exhibit this bug with an
> > unterminated
> > > string but they will happily truncate names and go with it.  I'm thinking
> > of
> > > various places in the vport library and things like querying ports.
> >
> > If you point out other problematic cases I'll gladly fix them.
> 
> I skimmed through and you're right, most of them make userspace truncate.
>  However, there is one additional use of strncpy_from_user() in vport_del(),
> which has problems similar to the one in create_dp().

Thanks.  Here's a patch.  How does it look?

>From 21291d67574ae857d028c40b2fcc1116b6be6dfa Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp at nicira.com>
Date: Tue, 27 Apr 2010 12:41:11 -0700
Subject: [PATCH] vport: Better handle too-long network device names in vport_del().

The 'count' argument to strncpy_from_user() is supposed to include space
for the null terminator, so add it in.  Also, refuse names that have more
than IFNAMSIZ-1 characters outright, instead of truncating them.
---
 datapath/vport.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/datapath/vport.c b/datapath/vport.c
index ef4d7db..9ed7cd1 100644
--- a/datapath/vport.c
+++ b/datapath/vport.c
@@ -277,10 +277,13 @@ vport_del(const char __user *udevname)
 	struct vport *vport;
 	struct dp_port *dp_port;
 	int err = 0;
+	int retval;
 
-	if (strncpy_from_user(devname, udevname, IFNAMSIZ - 1) < 0)
+	retval = strncpy_from_user(devname, udevname, IFNAMSIZ);
+	if (retval < 0)
 		return -EFAULT;
-	devname[IFNAMSIZ - 1] = '\0';
+	else if (retval >= IFNAMSIZ)
+		return -ENAMETOOLONG;
 
 	rtnl_lock();
 
-- 
1.6.6.1





More information about the dev mailing list