GNOME.org
 

gnome-keyring r1665 - in trunk: . daemon egg pam

Author: nnielsen
Date: Wed Mar 11 03:49:47 2009
New Revision: 1665
URL: http://svn.gnome.org/viewvc/gnome-keyring?rev=1665&view=rev

Log:
Implement valgrind support for our memory allocator, and support for
running gnome-keyring-daemon under valgrind. Use --enable-valgrind=run

Modified:
   trunk/ChangeLog
   trunk/configure.in
   trunk/daemon/Makefile.am
   trunk/daemon/gnome-keyring-daemon.desktop.in.in
   trunk/daemon/org.gnome.keyring.service.in
   trunk/egg/egg-secure-memory.c
   trunk/pam/gkr-pam-module.c

Modified: trunk/configure.in
==============================================================================
--- trunk/configure.in	(original)
+++ trunk/configure.in	Wed Mar 11 03:49:47 2009
@@ -79,26 +79,6 @@
 AC_PATH_PROG(GLIB_GENMARSHAL, glib-genmarshal)
 
 # --------------------------------------------------------------------
-# Debug mode
-
-AC_ARG_ENABLE(debug, 
-	    AC_HELP_STRING([--enable-debug],
-	    [Compile binaries in debug mode]))
-
-if test "$enable_debug" = "yes"; then
-  CFLAGS="$CFLAGS -g -O0 -Wall"
-  CFLAGS="$CFLAGS -DG_DISABLE_DEPRECATED -DGDK_PIXBUF_DISABLE_DEPRECATED"
-  CFLAGS="$CFLAGS -DGDK_DISABLE_DEPRECATED -DGTK_DISABLE_DEPRECATED"
-  AC_DEFINE_UNQUOTED(_DEBUG, 1, [In debug mode])
-  echo "enabling debug compile mode"
-  debug_status="yes"
-else 
-  dnl AC_DEFINE_UNQUOTED(G_DISABLE_ASSERT, 1, [Disable glib assertions])
-  echo "disabling debug compile mode"
-  debug_status="no"
-fi
-
-# --------------------------------------------------------------------
 # Check for socklen_t
 # 
 
@@ -406,9 +386,28 @@
 	AC_MSG_ERROR(asn1Parser tool is not installed)
 fi
 
-dnl ==========================================================================
+# --------------------------------------------------------------------
+# Debug mode
 
-dnl Turn on the additional warnings last, so -Werror doesn't affect other tests.
+AC_ARG_ENABLE(debug, 
+	    AC_HELP_STRING([--enable-debug],
+	    [Compile binaries in debug mode]))
+
+if test "$enable_debug" = "yes"; then
+  CFLAGS="$CFLAGS -g -O0 -Wall"
+  CFLAGS="$CFLAGS -DG_DISABLE_DEPRECATED -DGDK_PIXBUF_DISABLE_DEPRECATED"
+  CFLAGS="$CFLAGS -DGDK_DISABLE_DEPRECATED -DGTK_DISABLE_DEPRECATED"
+  AC_DEFINE_UNQUOTED(_DEBUG, 1, [In debug mode])
+  echo "enabling debug compile mode"
+  debug_status="yes"
+else 
+  dnl AC_DEFINE_UNQUOTED(G_DISABLE_ASSERT, 1, [Disable glib assertions])
+  echo "disabling debug compile mode"
+  debug_status="no"
+fi
+
+# -------------------------------------------------------------------
+# More warnings
 
 AC_MSG_CHECKING(for more warnings)
 if test "$GCC" = "yes"; then
@@ -473,6 +472,55 @@
 fi
 
 # ----------------------------------------------------------------------
+# Valgrind
+
+AC_ARG_ENABLE(valgrind,
+	AC_HELP_STRING([--enable-valgrind],
+	[Run gnome-keyring-daemon using valgrind]))
+	
+AC_MSG_CHECKING([valgrind])
+valgrind_status="yes"
+
+AM_CONDITIONAL(WITH_VALGRIND, test "$enable_valgrind" == "run")
+
+# Run gnome-keyring-daemon under valgrind as default
+if test "$enable_valgrind" == "run"; then
+	AC_PATH_PROG(VALGRIND, valgrind, no)
+	if test "$VALGRIND" = "no" ; then
+		AC_MSG_ERROR(valgrind tool is not installed)
+	fi
+	
+	VALGRIND_ARG="--log-file=/tmp/gkr-valgrind.log.%p"
+	VALGRIND_RUN="$VALGRIND $VALGRIND_ARG "
+
+	AC_DEFINE_UNQUOTED(VALGRIND, "$VALGRIND", [Path to valgrind executable])
+	AC_DEFINE_UNQUOTED(VALGRIND_ARG, "$VALGRIND_ARG", [Path to valgrind executable])
+	
+	enable_valgrind="yes"
+	valgrind_status="run"
+fi
+
+AC_SUBST(VALGRIND)
+AC_SUBST(VALGRIND_ARG)
+AC_SUBST(VALGRIND_RUN)
+
+# Build valgrind support into code
+if test "$enable_valgrind" == "yes"; then
+	AC_CHECK_HEADER([valgrind/valgrind.h], have_valgrind=yes, have_valgrind=no)
+	if test "$have_valgrind" == "no"; then
+		AC_MSG_ERROR(The valgrind headers are missing)
+	fi
+	AC_DEFINE_UNQUOTED(WITH_VALGRIND, 1, [Run under valgrind])
+	AC_MSG_RESULT(yes)
+	
+	
+# No valgrind
+else
+	AC_MSG_RESULT(no)
+	valgrind_status="no"
+fi
+
+# ----------------------------------------------------------------------
 
 GP11_LT_RELEASE=$GP11_MAJOR:$GP11_REVISION:$GP11_AGE
 AC_SUBST(GP11_LT_RELEASE)
@@ -497,6 +545,7 @@
 common/tests/Makefile
 daemon/Makefile
 daemon/gnome-keyring-daemon.desktop.in
+daemon/org.gnome.keyring.service
 daemon/data/Makefile
 daemon/keyrings/Makefile
 daemon/keyrings/tests/Makefile
@@ -534,9 +583,6 @@
 library/gnome-keyring-1-uninstalled.pc
 ])
 
-# 
-# gp11/tests/module/Makefile
-
 # ------------------------------------------------------------------------------
 # Summary
 # 
@@ -552,6 +598,7 @@
 echo
 echo "BUILD"
 echo "  Debug Build:          $debug_status"
+echo "  Valgrind:             $valgrind_status"
 echo "  Tests, -Werror:       $tests_status"
 echo
 

Modified: trunk/daemon/Makefile.am
==============================================================================
--- trunk/daemon/Makefile.am	(original)
+++ trunk/daemon/Makefile.am	Wed Mar 11 03:49:47 2009
@@ -56,14 +56,11 @@
 CLEANFILES = \
 	org.gnome.keyring.service \
 	$(desktop_DATA)
-
+	
 servicedir       = $(DBUS_SERVICES_DIR)
 service_in_files = org.gnome.keyring.service.in
 service_DATA     = $(service_in_files:.service.in=.service)
 
-$(service_DATA): $(service_in_files) Makefile
-	@sed -e "s|\ BINDIR\@|$(bindir)|" $< > $@
-
 desktop_in_files = gnome-keyring-daemon.desktop.in
 desktopdir       = $(sysconfdir)/xdg/autostart
 desktop_DATA     = $(desktop_in_files:.desktop.in=.desktop)

Modified: trunk/daemon/gnome-keyring-daemon.desktop.in.in
==============================================================================
--- trunk/daemon/gnome-keyring-daemon.desktop.in.in	(original)
+++ trunk/daemon/gnome-keyring-daemon.desktop.in.in	Wed Mar 11 03:49:47 2009
@@ -1,7 +1,7 @@
 [Desktop Entry]
 Type=Application
 _Name=GNOME Keyring Daemon
-Exec=gnome-keyring-daemon --start
+Exec= VALGRIND_RUN@ gnome-keyring-daemon --start
 OnlyShowIn=GNOME;
 X-GNOME-Autostart-Phase=Initialization
 X-GNOME-AutoRestart=false

Modified: trunk/daemon/org.gnome.keyring.service.in
==============================================================================
--- trunk/daemon/org.gnome.keyring.service.in	(original)
+++ trunk/daemon/org.gnome.keyring.service.in	Wed Mar 11 03:49:47 2009
@@ -1,3 +1,3 @@
 [D-BUS Service]
 Name=org.gnome.keyring
-Exec= BINDIR@/gnome-keyring-daemon --foreground --components=keyring
+Exec= VALGRIND_RUN@ gnome-keyring-daemon --foreground --components=keyring

Modified: trunk/egg/egg-secure-memory.c
==============================================================================
--- trunk/egg/egg-secure-memory.c	(original)
+++ trunk/egg/egg-secure-memory.c	Wed Mar 11 03:49:47 2009
@@ -41,6 +41,11 @@
 #include <unistd.h>
 #include <assert.h>
 
+#ifdef WITH_VALGRIND
+#include <valgrind/valgrind.h>
+#include <valgrind/memcheck.h>
+#endif
+
 /*
  * Use this to force all memory through malloc
  * for use with valgrind and the like 
@@ -168,7 +173,7 @@
 pool_alloc (void)
 {
 	Pool *pool;
-	void *pages;
+	void *pages, *item;
 	size_t len, i;
 	
 	/* A pool with an available item */
@@ -196,11 +201,21 @@
 		pool->n_items = (len - sizeof (Pool)) / sizeof (Item);
 		for (i = 0; i < pool->n_items; ++i)
 			unused_push (&pool->unused, pool->items + i);
+		
+#ifdef WITH_VALGRIND
+		VALGRIND_CREATE_MEMPOOL(pool, 0, 0);
+#endif
 	}
 
 	++pool->used;
 	ASSERT (unused_peek (&pool->unused));
-	return memset (unused_pop (&pool->unused), 0, sizeof (Item));
+	item = unused_pop (&pool->unused);
+	
+#ifdef WITH_VALGRIND
+	VALGRIND_MEMPOOL_ALLOC (pool, item, sizeof (Item));
+#endif
+	
+	return memset (item, 0, sizeof (Item));
 }
 
 static void
@@ -228,10 +243,20 @@
 	/* No more meta cells used in this block, remove from list, destroy */
 	if (pool->used == 1) {
 		*at = pool->next;
+		
+#ifdef WITH_VALGRIND
+		VALGRIND_DESTROY_MEMPOOL (pool);
+#endif
+		
 		munmap (pool, pool->length);
 		return;
 	}
 	
+#ifdef WITH_VALGRIND
+	VALGRIND_MEMPOOL_FREE (pool, item);
+	VALGRIND_MAKE_MEM_UNDEFINED (item, sizeof (Item));
+#endif
+	
 	--pool->used;
 	memset (item, 0xCD, sizeof (Item));
 	unused_push (&pool->unused, item);
@@ -259,8 +284,9 @@
 /* -----------------------------------------------------------------------------
  * SEC ALLOCATION
  * 
- * Each memory cell begins and ends with a pointer to its metadata. 
- * 
+ * Each memory cell begins and ends with a pointer to its metadata. These are also
+ * used as guards or red zones. Since they're treated as redzones by valgrind we 
+ * have to jump through a few hoops before reading and/or writing them.
  */
 
 static inline size_t
@@ -272,15 +298,35 @@
 static inline void
 sec_write_guards (Cell *cell)
 {
+#ifdef WITH_VALGRIND
+	VALGRIND_MAKE_MEM_UNDEFINED (cell->words, sizeof (word_t));
+	VALGRIND_MAKE_MEM_UNDEFINED (cell->words + cell->n_words - 1, sizeof (word_t));
+#endif
+
 	((void**)cell->words)[0] = (void*)cell;
 	((void**)cell->words)[cell->n_words - 1] = (void*)cell;
+	
+#ifdef WITH_VALGRIND
+	VALGRIND_MAKE_MEM_NOACCESS (cell->words, sizeof (word_t));
+	VALGRIND_MAKE_MEM_NOACCESS (cell->words + cell->n_words - 1, sizeof (word_t));
+#endif	
 }
 
 static inline void
 sec_check_guards (Cell *cell)
 {
+#ifdef WITH_VALGRIND
+	VALGRIND_MAKE_MEM_DEFINED (cell->words, sizeof (word_t));
+	VALGRIND_MAKE_MEM_DEFINED (cell->words + cell->n_words - 1, sizeof (word_t)); 
+#endif	
+	
 	ASSERT(((void**)cell->words)[0] == (void*)cell);
-	ASSERT(((void**)cell->words)[cell->n_words - 1] == (void*)cell);	
+	ASSERT(((void**)cell->words)[cell->n_words - 1] == (void*)cell);
+	
+#ifdef WITH_VALGRIND
+	VALGRIND_MAKE_MEM_NOACCESS (cell->words, sizeof (word_t));
+	VALGRIND_MAKE_MEM_NOACCESS (cell->words + cell->n_words - 1, sizeof (word_t));
+#endif	
 }
 
 static void
@@ -366,8 +412,17 @@
 	if (!sec_is_valid_word (block, word))
 		return NULL;
 
+#ifdef WITH_VALGRIND
+	VALGRIND_MAKE_MEM_DEFINED (word, sizeof (word_t));
+#endif
+	
 	cell = *word;
 	sec_check_guards (cell);
+
+#ifdef WITH_VALGRIND
+	VALGRIND_MAKE_MEM_NOACCESS (word, sizeof (word_t));
+#endif
+
 	return cell;
 }
 
@@ -383,8 +438,17 @@
 	if (!sec_is_valid_word (block, word))
 		return NULL;
 
+#ifdef WITH_VALGRIND
+	VALGRIND_MAKE_MEM_DEFINED (word, sizeof (word_t));
+#endif
+
 	cell = *word;
 	sec_check_guards (cell);
+	
+#ifdef WITH_VALGRIND
+	VALGRIND_MAKE_MEM_NOACCESS (word, sizeof (word_t));
+#endif
+
 	return cell;
 }
 
@@ -393,6 +457,7 @@
 {
 	Cell *cell, *other;
 	size_t n_words;
+	void *memory;
 	
 	ASSERT (block);
 	ASSERT (length);
@@ -449,7 +514,13 @@
 	
 	++block->used;
 	cell->allocated = length;
-	return memset (sec_cell_to_memory (cell), 0, cell->allocated);
+	memory = sec_cell_to_memory (cell);
+	
+#ifdef WITH_VALGRIND
+	VALGRIND_MAKE_MEM_UNDEFINED (memory, length);
+#endif
+	
+	return memset (memory, 0, length);
 }
 
 static void*
@@ -460,12 +531,22 @@
 	
 	ASSERT (block);
 	ASSERT (memory);
+	
+	word = memory;
+	--word;
+	
+#ifdef WITH_VALGRIND
+	VALGRIND_MAKE_MEM_DEFINED (word, sizeof (word_t));
+#endif
 
 	/* Lookup the meta for this memory block (using guard pointer) */
-	word = memory;
-	ASSERT (sec_is_valid_word (block, word - 1));
-	ASSERT (pool_valid (*(word - 1)));
-	cell = *(word - 1);
+	ASSERT (sec_is_valid_word (block, word));
+	ASSERT (pool_valid (*word));
+	cell = *word;
+
+#ifdef WITH_VALGRIND
+	VALGRIND_MAKE_MEM_DEFINED (cell->words, cell->n_words * sizeof (word_t));
+#endif
 
 	sec_check_guards (cell);
 	sec_clear_memory (memory, 0, cell->allocated);
@@ -522,9 +603,15 @@
 
 	/* Dig out where the meta should be */
 	word = memory;
-	ASSERT (sec_is_valid_word (block, word - 1));
-	ASSERT (pool_valid (*(word - 1)));
-	cell = *(word - 1);
+	--word;
+	
+#ifdef WITH_VALGRIND
+	VALGRIND_MAKE_MEM_DEFINED (word, sizeof (word_t));
+#endif
+
+	ASSERT (sec_is_valid_word (block, word));
+	ASSERT (pool_valid (*word));
+	cell = *word;
 	
 	/* Validate that it's actually for real */
 	sec_check_guards (cell);
@@ -543,7 +630,13 @@
 
 		/* TODO: No shrinking behavior yet */
 		cell->allocated = length;
-		return sec_clear_memory (sec_cell_to_memory (cell), valid, length);
+		alloc = sec_cell_to_memory (cell);
+
+#ifdef WITH_VALGRIND
+		VALGRIND_MAKE_MEM_DEFINED (alloc, length);
+#endif
+		
+		return sec_clear_memory (alloc, valid, length);
 	}
 	
 	/* Need braaaaaiiiiiinsss... */
@@ -573,7 +666,13 @@
 	
 	if (cell->n_words >= n_words) {
 		cell->allocated = length;
-		return sec_clear_memory (sec_cell_to_memory (cell), valid, length);
+		alloc = sec_cell_to_memory (cell);
+		
+#ifdef WITH_VALGRIND
+		VALGRIND_MAKE_MEM_DEFINED (alloc, length);
+#endif
+		
+		return sec_clear_memory (alloc, valid, length);
 	}
 	
 	/* That didn't work, try alloc/free */
@@ -596,17 +695,27 @@
 	ASSERT (block);
 	ASSERT (memory);
 
-	/* Lookup the meta for this memory block (using guard pointer) */
 	word = memory;
-	ASSERT (sec_is_valid_word (block, word - 1));
-	ASSERT (pool_valid (*(word - 1)));
-	cell = *(word - 1);
+	--word;
+
+#ifdef WITH_VALGRIND
+	VALGRIND_MAKE_MEM_DEFINED (word, sizeof (word_t));
+#endif
+	
+	/* Lookup the meta for this memory block (using guard pointer) */
+	ASSERT (sec_is_valid_word (block, word));
+	ASSERT (pool_valid (*word));
+	cell = *word;
 	
 	sec_check_guards (cell);
 	ASSERT (cell->next == NULL);
 	ASSERT (cell->prev == NULL);
 	ASSERT (cell->allocated > 0);
 	
+#ifdef WITH_VALGRIND
+	VALGRIND_MAKE_MEM_NOACCESS (word, sizeof (word_t));
+#endif
+	
 	return cell->allocated;
 }
 
@@ -720,6 +829,10 @@
 		return NULL;
 	}
 	
+#ifdef WITH_VALGRIND
+	VALGRIND_MAKE_MEM_DEFINED (block->words, size);
+#endif
+	
 	/* The first cell to allocate from */
 	cell->words = block->words;
 	cell->n_words = block->n_words;
@@ -807,6 +920,11 @@
 			if (block)
 				memory = sec_alloc (block, length);
 		}
+		
+#ifdef WITH_VALGRIND
+		if (memory != NULL)
+			VALGRIND_MALLOCLIKE_BLOCK (memory, length, sizeof (void*), 1);
+#endif
 	
 	DO_UNLOCK ();
 	
@@ -856,7 +974,20 @@
 		for (block = all_blocks; block; block = block->next) {
 			if (sec_is_valid_word (block, memory)) {
 				previous = sec_allocated (block, memory);
+
+#ifdef WITH_VALGRIND
+				/* Let valgrind think we are unallocating so that it'll validate */
+				VALGRIND_FREELIKE_BLOCK (memory, sizeof (word_t));
+#endif
+
 				alloc = sec_realloc (block, memory, length);
+				
+#ifdef WITH_VALGRIND
+				/* Now tell valgrind about either the new block or old one */
+				VALGRIND_MALLOCLIKE_BLOCK (alloc ? alloc : memory, 
+				                           alloc ? length : previous, 
+				                           sizeof (word_t), 1);
+#endif					
 				break;
 			}
 		}
@@ -917,14 +1048,21 @@
 	
 		/* Find out where it belongs to */
 		for (block = all_blocks; block; block = block->next) {
-			if (sec_is_valid_word (block, memory)) {
-				sec_free (block, memory);
+			if (sec_is_valid_word (block, memory))
 				break;
-			}
 		}
 
-		if (block && block->used == 0)
-			sec_block_destroy (block);
+#ifdef WITH_VALGRIND
+		/* We like valgrind's warnings, so give it a first whack at checking for errors */
+		if (block != NULL || !(flags & GKR_SECURE_USE_FALLBACK))
+			VALGRIND_FREELIKE_BLOCK (memory, sizeof (word_t));
+#endif
+
+		if (block != NULL) {
+			sec_free (block, memory);
+			if (block->used == 0)
+				sec_block_destroy (block);
+		}
 			
 	DO_UNLOCK ();
 	

Modified: trunk/pam/gkr-pam-module.c
==============================================================================
--- trunk/pam/gkr-pam-module.c	(original)
+++ trunk/pam/gkr-pam-module.c	Wed Mar 11 03:49:47 2009
@@ -279,16 +279,27 @@
 setup_child (int inp[2], int outp[2], int errp[2], 
              pam_handle_t *ph, struct passwd *pwd, const char *password)
 {
-	char *args[] = { GNOME_KEYRING_DAEMON, "--daemonize", "--login", NULL};
 	const char* display;
 	int i, ret;
+
+	/* The --login argument comes last, because of code below */
+	
+#ifdef VALGRIND 	
+	char *args[] = { VALGRIND, VALGRIND_ARG, GNOME_KEYRING_DAEMON, "--daemonize", "--login", NULL};
+#else
+	char *args[] = { GNOME_KEYRING_DAEMON, "--daemonize", "--login", NULL};
+#endif
 	
 	assert (pwd);
 	assert (pwd->pw_dir);
 
-	/* If no password, don't pas in --login */
-	if (password == NULL)
-		args[2] = NULL;
+	/* If no password, don't pass in --login */
+	if (password == NULL) {
+		for (i = 0; args[i]; ++i) {
+			if (strcmp ("--login", args[i]) == 0)
+				args[i] = NULL;
+		}
+	}
 
 	/* Fix up our end of the pipes */
 	if (dup2 (inp[READ_END], STDIN) < 0 ||
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]