[apparmor] [PATCH] parser: Quiet search dir valgrind warning and remove suppression

Tyler Hicks tyhicks at canonical.com
Wed Feb 5 19:51:30 UTC 2014


When passing an include directory on the command line to
apparmor_parser, valgrind emits a warning:

 Invalid read of size 4
    at 0x404DA6: add_search_dir(char const*) (parser_include.c:152)
    by 0x40BB37: process_arg(int, char*) (parser_main.c:457)
    by 0x403D43: main (parser_main.c:590)
  Address 0x572207c is 28 bytes inside a block of size 29 alloc'd
    at 0x4C2A420: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    by 0x53E31C9: strdup (strdup.c:42)
    by 0x404D94: add_search_dir(char const*) (parser_include.c:145)
    by 0x40BB37: process_arg(int, char*) (parser_main.c:457)
    by 0x403D43: main (parser_main.c:590)

This patch quiets the warning by removing strlen() calls on the t char
array. Instead, it only calls strlen() on the dir char array. t is a
dupe of dir and strlen(dir) does not trigger the valgrind warning.

Additionally, this patch adds a bit of defensive programming to the
while loop to ensure that index into the t array is never negative.

Finally, the valgrind suppression is removed from valgrind_simple.py.

Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
---
 parser/parser_include.c       | 12 +++++++++---
 parser/tst/valgrind_simple.py |  8 --------
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/parser/parser_include.c b/parser/parser_include.c
index 0ad865d..3947f29 100644
--- a/parser/parser_include.c
+++ b/parser/parser_include.c
@@ -133,13 +133,19 @@ void set_base_dir(char *dir)
 int add_search_dir(const char *dir)
 {
 	char *t;
+	size_t len;
+
 	if (npath >= MAX_PATH) {
 		PERROR(_("Error: Could not add directory %s to search path.\n"),
 		       dir);
 		return 0;
 	}
 
-	if (!dir || strlen(dir) <= 0)
+	if (!dir)
+		return 1;
+
+	len = strlen(dir);
+	if (len == 0)
 		return 1;
 
 	t = strdup(dir);
@@ -149,8 +155,8 @@ int add_search_dir(const char *dir)
 	}
 
 	/*strip trailing /'s */
-	while (t[strlen(t) - 1] == '/')
-		t[strlen(t) - 1] = 0;
+	while (len > 0 && t[--len] == '/')
+		t[len] = 0;
 	path[npath] = t;
 	npath++;
 
diff --git a/parser/tst/valgrind_simple.py b/parser/tst/valgrind_simple.py
index 15a5bb5..868e3c1 100755
--- a/parser/tst/valgrind_simple.py
+++ b/parser/tst/valgrind_simple.py
@@ -25,14 +25,6 @@ VALGRIND_ARGS = ['--leak-check=full', '--error-exitcode=%d' % (VALGRIND_ERROR_CO
 
 VALGRIND_SUPPRESSIONS = '''
 {
-    valgrind-add_search_dir-obsessive-overreads
-    Memcheck:Addr4
-    fun:_Z*add_search_dir*
-    fun:_Z*process_arg*
-    fun:main
-}
-
-{
     valgrind-serialize_profile-obsessive-overreads
     Memcheck:Addr4
     fun:_Z*sd_serialize_profile*
-- 
1.9.rc1




More information about the AppArmor mailing list