[ovs-dev] [PATCH 3/3] datapath: Check device name length more carefully in create_dp().
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
@@ -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)
- devname[IFNAMSIZ - 1] = '\0';
+ else if (retval >= IFNAMSIZ)
+ return -ENAMETOOLONG;
More information about the dev