[ovs-dev] [PATCH 1/2] dpif-netdev: Fix memory leak.

[Ben Pfaff]

On Thu, Aug 16, 2012 at 09:51:34AM -0400, Ed Maste wrote:
> On 15 August 2012 19:12, Ben Pfaff <blp at nicira.com> wrote:
> > upcall->packet is allocated with malloc(), via ofpbuf_new(), but nothing
> > ever frees it.
> >
> > Found by valgrind.
> >
> > CC: Ed Maste <emaste at freebsd.org>
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> >  lib/dpif-netdev.c |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 7fa2720..3d01b17 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -966,6 +966,7 @@ dpif_netdev_recv(struct dpif *dpif, struct dpif_upcall *upcall,
> >
> >          ofpbuf_uninit(buf);
> >          *buf = *upcall->packet;
> > +        free(upcall->packet);
> >
> >          return 0;
> >      } else {
> > --
> > 1.7.2.5
> 
> This looks like it results in a use-after-free in dpif_recv which
> accesses upcall->packet (which may be a moot point after the 2nd
> patch; I'm going to look at it now).

Sorry, that's a dumb mistake.  Here's a corrected version:

--8<--------------------------cut here-------------------------->8--

>From 3150ae27f6022aa6ffdfdf57f85808a5429f7a07 Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp at nicira.com>
Date: Thu, 16 Aug 2012 08:36:42 -0700
Subject: [PATCH] dpif-netdev: Fix memory leak.

upcall->packet is allocated with malloc(), via ofpbuf_new(), but nothing
ever frees it.

Found by valgrind.

CC: Ed Maste <emaste at freebsd.org>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 lib/dpif-netdev.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 7fa2720..63b59a3 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -966,6 +966,8 @@ dpif_netdev_recv(struct dpif *dpif, struct dpif_upcall *upcall,
 
         ofpbuf_uninit(buf);
         *buf = *upcall->packet;
+        free(upcall->packet);
+        upcall->packet = buf;
 
         return 0;
     } else {
-- 
1.7.2.5