Skip to content

Commit

Permalink
Fix a double free error in vr_message_process_response
Browse files Browse the repository at this point in the history
This patch fixes a double free error and introduces
user land unit test for the vrouter code. The cmocka
unit test framework is used for that purpose.

A first unit test is provided that check this fixes.

Change-Id: Ic8c84d076d078a87adaf6d4398373c368db0361c
Closes-bug: #1394147
  • Loading branch information
Sylvain Afchain committed Nov 19, 2014
1 parent 82330a6 commit 9f7b8ca
Show file tree
Hide file tree
Showing 7 changed files with 180 additions and 12 deletions.
13 changes: 12 additions & 1 deletion SConscript
Expand Up @@ -5,6 +5,7 @@
import subprocess
import sys
import os
import copy

AddOption('--kernel-dir', dest = 'kernel-dir', action='store',
help='Linux kernel source directory for vrouter.ko')
Expand All @@ -25,6 +26,16 @@ vr_root = './'
makefile = vr_root + 'Makefile'
dp_dir = Dir(vr_root).srcnode().abspath

def MakeTestCmdFn(self, env, test_name, test_list, deps):
sources = copy.copy(deps)
sources.append(test_name + '.c')
tgt = env.UnitTest(target = test_name, source = sources)
env.Alias('vrouter:'+ test_name, tgt)
test_list.append(tgt)
return tgt

VRouterEnv.AddMethod(MakeTestCmdFn, 'MakeTestCmd')

def shellCommand(cmd):
""" Return the output of a shell command
This wrapper is required since check_output is not supported in
Expand All @@ -49,7 +60,7 @@ if sys.platform != 'darwin':
env.Install(src_root, ['LICENSE', 'Makefile', 'GPL-2.0.txt'])
env.Alias('install', src_root)

subdirs = ['linux', 'include', 'dp-core', 'host', 'sandesh', 'utils', 'uvrouter']
subdirs = ['linux', 'include', 'dp-core', 'host', 'sandesh', 'utils', 'uvrouter', 'test']
for sdir in subdirs:
env.SConscript(sdir + '/SConscript',
exports='VRouterEnv',
Expand Down
4 changes: 1 addition & 3 deletions dp-core/vr_message.c
Expand Up @@ -81,7 +81,7 @@ vr_message_queue_response(char *buf, int len)
return -ENOMEM;

response->vr_message_buf = buf;
response->vr_message_len = len;;
response->vr_message_len = len;
vr_queue_enqueue(&message_h.vm_response_queue,
&response->vr_message_queue);

Expand Down Expand Up @@ -171,8 +171,6 @@ vr_message_process_response(int (*cb)(void *, unsigned int, void *),
while ((response = vr_message_dequeue_response())) {
proto->mproto_decode(response->vr_message_buf,
response->vr_message_len, cb, cb_arg);
if (response->vr_message_buf)
vr_mtrans_free(response->vr_message_buf);
vr_message_free(response);
}

Expand Down
33 changes: 25 additions & 8 deletions host/vr_host_message.c
Expand Up @@ -2,7 +2,7 @@
* vr_host_message.c -- messaging infrastructure to interface with dp-core when
* run as a library
*
* Copyright (c) 2013 Juniper Networks, Inc. All rights reserved.
* Copyright (c) 2013 Juniper Networks, Inc. All rights reserved.
*/
#include "vr_os.h"
#include "vr_queue.h"
Expand All @@ -24,7 +24,7 @@ struct diet_message {
};

struct diet_object_md {
/*
/*
* when encoded, how much length will this object take. since encode
* and decode does not do much to the object, encode/decode lengths
* are the same
Expand All @@ -37,7 +37,7 @@ struct diet_object_md {
int (*obj_response)(struct diet_message *, void *,
int (*)(void *, unsigned int, void *), void *);
};

int diet_dropstats_object_copy(char *, unsigned int, void *);
int diet_interface_object_copy(char *, unsigned int, void *);
int diet_nexthop_object_copy(char *, unsigned int, void *);
int diet_mpls_object_copy(char *, unsigned int, void *);
Expand All @@ -53,15 +53,15 @@ struct diet_object_md diet_md[] = {
},
[VR_INTERFACE_OBJECT_ID] = {
.obj_len = sizeof(vr_interface_req) +
VR_ETHER_ALEN,
VR_ETHER_ALEN,

.obj_copy = diet_interface_object_copy,
.obj_request = vr_interface_req_process,
.obj_response = diet_object_response,
},
[VR_NEXTHOP_OBJECT_ID] = {
.obj_len = sizeof(vr_nexthop_req) +
VR_ETHER_HLEN,
VR_ETHER_HLEN,

.obj_copy = diet_nexthop_object_copy,
.obj_request = vr_nexthop_req_process,
Expand All @@ -84,8 +84,25 @@ struct diet_object_md diet_md[] = {
.obj_copy = diet_response_object_copy,
.obj_response = diet_object_response,
},
[VR_DROP_STATS_OBJECT_ID] = {
.obj_len = sizeof(vr_drop_stats_req),
.obj_copy = diet_dropstats_object_copy,
.obj_response = diet_object_response,
}
};

int
diet_dropstats_object_copy(char *dst, unsigned int buf_len, void *object)
{
vr_drop_stats_req *src = (vr_drop_stats_req *)object;

if (buf_len < diet_md[VR_DROP_STATS_OBJECT_ID].obj_len)
return -ENOSPC;

memcpy(dst, src, sizeof(*src));
return sizeof(*src);
}

int
diet_route_object_copy(char *dst, unsigned int buf_len, void *object)
{
Expand Down Expand Up @@ -179,9 +196,9 @@ diet_object_response(struct diet_message *hdr, void *object,
diet_md[hdr->m_oid].obj_len, object);

if (ret > 0) {
if (cb(arg, hdr->m_oid, dst_buf))
vr_mtrans_free(dst_buf);
}
cb(arg, hdr->m_oid, dst_buf);
}
vr_mtrans_free(dst_buf);

return ret;
}
Expand Down
25 changes: 25 additions & 0 deletions test/SConscript
@@ -0,0 +1,25 @@
#
# Copyright (c) 2014 Juniper Networks, Inc. All rights reserved.
#

Import('VRouterEnv')
env = VRouterEnv.Clone()

vrouter_suite = []

# Include paths

# CFLAGS
env.Append(CCFLAGS = '-g')

env.Replace(LIBPATH = env['TOP_LIB'])
env.Append(LIBPATH = ['../host', '../sandesh', '../dp-core'])
env.Replace(LIBS = ['dp_core', 'vrouter', 'dp_sandesh_c', 'vrouter', 'dp_core', 'dp_sandesh_c', 'dp_core', 'sandesh-c', 'cmocka'])

test_dep_srcs = ['common_test.c']

dp_core_test = VRouterEnv.MakeTestCmd(env, 'dp_core_test', vrouter_suite, test_dep_srcs)

test = env.TestSuite('vrouter-test', vrouter_suite)
env.Alias('vrouter:test', test)
Return('vrouter_suite')
20 changes: 20 additions & 0 deletions test/common_test.c
@@ -0,0 +1,20 @@
#include "vr_types.h"

void
get_random_bytes(void *buf, int nbytes)
{
}

uint32_t
jhash(void *key, uint32_t length, uint32_t interval)
{
uint32_t ret;
int i;
unsigned char *data = (unsigned char *)key;

for (i = 0; i < length; i ++)
ret += data[i];

return ret;
}

9 changes: 9 additions & 0 deletions test/common_test.h
@@ -0,0 +1,9 @@
#ifndef __COMMON_TEST_H__
#define __COMMON_TEST_H__

#include "vr_types.h"

void get_random_bytes(void *buf, int nbytes);
uint32_t jhash(void *key, uint32_t length, uint32_t interval);

#endif /* __COMMON_TEST_H__ */
88 changes: 88 additions & 0 deletions test/dp_core_test.c
@@ -0,0 +1,88 @@
#include <stdio.h>
#include <unistd.h>
#include <stdarg.h>
#include <stddef.h>
#include <setjmp.h>
#include <cmocka.h>

#include "vr_types.h"
#include "vr_os.h"
#include "vr_packet.h"
#include "vr_message.h"
#include "vr_interface.h"

#include "host/vr_host.h"
#include "host/vr_host_packet.h"
#include "host/vr_host_interface.h"

extern int vrouter_host_init(unsigned int);
extern unsigned int vr_num_cpus;

unsigned int allocated = 0;

void *alloc_for_test(unsigned int size) {
void *ptr;

ptr = malloc(size);
allocated++;

return ptr;
}

void free_for_test(void *ptr) {
free(ptr);
allocated--;
}

int fake_response_cb(void *ptr1, unsigned int i, void *ptr2) {
return 1;
}

void drop_stats_memory_test(void **state) {
vr_drop_stats_req req = {
.h_op = SANDESH_OP_GET,
.vds_rid = 0
};

/* process request */
vr_drop_stats_req_process(&req);

/* currently one response queued */
assert_int_equal(allocated, 2);

/* dequeue it, and check the alloctions */
vr_message_process_response(fake_response_cb, NULL);
assert_int_equal(allocated, 0);
}

static void setup(void **state) {
vrouter_host->hos_malloc = alloc_for_test;
vrouter_host->hos_zalloc = alloc_for_test;
vrouter_host->hos_free = free_for_test;
}

static void teardown(void **state) {
free(*state);
}

int main(void) {
int ret;

/* test suite */
const UnitTest tests[] = {
unit_test_setup_teardown(drop_stats_memory_test, setup, teardown),
};

vr_diet_message_proto_init();

/* init the vrouter */
ret = vrouter_host_init(VR_MPROTO_SANDESH);
if (ret)
return ret;


/* let's run the test suite */
ret = run_tests(tests);

return ret;
}

0 comments on commit 9f7b8ca

Please sign in to comment.