On 10/24/2014 11:57 AM, Laine Stump wrote:
virNetDevLinkDump() gets a message from netlink into
"resp", then
calls nlmsg_parse() to fill the table "tb" with pointers into resp. It
then returns tb to its caller, but not before freeing the buffer at
resp. That means that all the callers of virNetDevLinkDump() are
examining memory that has already been freed. This can be verified by
filling the buffer at resp with garbage prior to freeing it (or, I
suppose, just running libvirtd under valgrind) then performing some
operation that calls virNetDevLinkDump().
The code has been like this ever since virNetDevLinkDump() was written
- the original author didn't notice it, and neither did later
additional users of the function. It has only been pure luck (or maybe
a lack of heavy load, and/or maybe an allocation algorithm in malloc()
that delays re-use of just-freed memory) that has kept this from
causing errors, for example when configuring a PCI passthrough or
macvtap passthrough network interface.
The solution taken in this patch is the simplest - just return resp to
the caller along with tb, then have the caller free it after they are
finished using the data (pointers) in tb. I alternately could have
made a cleaner interface by creating a new struct that put tb and resp
together along with a vir*Free() function for it, but this function is
only used in a couple places, and I'm not sure there will be
additional new uses of virNetDevLinkDump(), so the value of adding a
new type, extra APIs, etc. is dubious.
---
src/util/virnetdev.c | 26 +++++++++++++++++---------
src/util/virnetdev.h | 2 +-
src/util/virnetdevvportprofile.c | 17 ++++++++++++-----
3 files changed, 30 insertions(+), 15 deletions(-)
ACK. For how few callers use it, your approach of making the caller
free the netlink data after use seems fine.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library
http://libvirt.org