diff options
| -rw-r--r-- | ChangeLog | 11 | ||||
| -rw-r--r-- | configure.in | 97 | ||||
| -rw-r--r-- | daemon/Makefile.am | 5 | ||||
| -rw-r--r-- | daemon/gnome-keyring-daemon.desktop.in.in | 2 | ||||
| -rw-r--r-- | daemon/org.gnome.keyring.service.in | 2 | ||||
| -rw-r--r-- | egg/egg-secure-memory.c | 186 | ||||
| -rw-r--r-- | pam/gkr-pam-module.c | 19 |
7 files changed, 263 insertions, 59 deletions
@@ -1,3 +1,14 @@ +2009-03-10 Stef Walter <stef@memberwebs.com> + + * configure.in: + * daemon/Makefile.am: + * daemon/gnome-keyring-daemon.desktop.in.in: + * daemon/org.gnome.keyring.service.in: + * egg/egg-secure-memory.c: + * pam/gkr-pam-module.c: Implement valgrind support for our + memory allocator, and support for running gnome-keyring-daemon + under valgrind. Use --enable-valgrind=run + 2009-03-04 Stef Walter <stef@memberwebs.com> * configure.in: diff --git a/configure.in b/configure.in index 432fc2db..2aa6c1c6 100644 --- a/configure.in +++ b/configure.in @@ -79,26 +79,6 @@ AM_GLIB_GNU_GETTEXT 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 @@ if test "$ASN1PARSER" = "no" ; then AC_MSG_ERROR(asn1Parser tool is not installed) fi -dnl ========================================================================== +# -------------------------------------------------------------------- +# 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 -dnl Turn on the additional warnings last, so -Werror doesn't affect other tests. +# ------------------------------------------------------------------- +# More warnings AC_MSG_CHECKING(for more warnings) if test "$GCC" = "yes"; then @@ -473,6 +472,55 @@ if test "$enable_coverage" == "yes"; then 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/Makefile 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.pc library/gnome-keyring-1-uninstalled.pc ]) -# -# gp11/tests/module/Makefile - # ------------------------------------------------------------------------------ # Summary # @@ -552,6 +598,7 @@ echo " Root Certificates: $root_status" echo echo "BUILD" echo " Debug Build: $debug_status" +echo " Valgrind: $valgrind_status" echo " Tests, -Werror: $tests_status" echo diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 478aeb25..6cc89100 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -56,14 +56,11 @@ EXTRA_DIST = \ 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) diff --git a/daemon/gnome-keyring-daemon.desktop.in.in b/daemon/gnome-keyring-daemon.desktop.in.in index 0e55d67d..c4a653fc 100644 --- a/daemon/gnome-keyring-daemon.desktop.in.in +++ b/daemon/gnome-keyring-daemon.desktop.in.in @@ -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 diff --git a/daemon/org.gnome.keyring.service.in b/daemon/org.gnome.keyring.service.in index 13a32be8..c94c5d39 100644 --- a/daemon/org.gnome.keyring.service.in +++ b/daemon/org.gnome.keyring.service.in @@ -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 diff --git a/egg/egg-secure-memory.c b/egg/egg-secure-memory.c index b9501d4f..c2cc7a43 100644 --- a/egg/egg-secure-memory.c +++ b/egg/egg-secure-memory.c @@ -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 @@ static void* 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_alloc (void) 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 @@ pool_free (void* item) /* 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 @@ pool_valid (void* item) /* ----------------------------------------------------------------------------- * 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 @@ sec_size_to_words (size_t length) 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 @@ sec_neighbor_before (Block *block, Cell *cell) 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 @@ sec_neighbor_after (Block *block, Cell *cell) 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 @@ sec_alloc (Block *block, size_t length) { Cell *cell, *other; size_t n_words; + void *memory; ASSERT (block); ASSERT (length); @@ -449,7 +514,13 @@ sec_alloc (Block *block, size_t length) ++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 @@ sec_free (Block *block, void *memory) 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 @@ sec_realloc (Block *block, void *memory, size_t length) /* 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 @@ sec_realloc (Block *block, void *memory, size_t length) /* 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 @@ sec_realloc (Block *block, void *memory, size_t length) 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 @@ sec_allocated (Block *block, void *memory) 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 @@ sec_block_create (size_t size) 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 @@ egg_secure_alloc_full (size_t length, int flags) 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 @@ egg_secure_realloc_full (void *memory, size_t length, int flags) 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 @@ egg_secure_free_full (void *memory, int flags) /* 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 (); diff --git a/pam/gkr-pam-module.c b/pam/gkr-pam-module.c index a453327d..bd0e23dd 100644 --- a/pam/gkr-pam-module.c +++ b/pam/gkr-pam-module.c @@ -279,16 +279,27 @@ static void 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 || |