[PATCH] Fix possible crash bugs in tdb code.
Michael Adam
ma at sernet.de
Thu Dec 13 08:26:40 GMT 2007
Andreas Schneider wrote:
>
> The first patch removes the code from winbind to restart it if the cache is
> invalid. I've talked to Michael Adam and we both think that a restart isn't
> needed anymore as the signal handlers changed.
First patch applied to v3-2-test in aabe9b33fcaed8af98b1ed6b736253e196d87d48.
Thanks - Michael
> The seconds just checks if the winbindd_invalidate_cache() function fails. On
> an error we close the cache revalidate it and initialize it again.
>
> I've tested it letting winbindd_invalidate_cache() always return -1. From my
> point of view everything worked just fine. Maybe it would be better to restart
> winbind, but then we need a restart function and have to store argv and envp.
>
>
> -- andreas
> From 8a1810ed21bfa18ed589d43ea6587c09b8ed4d14 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <anschneider at suse.de>
> Date: Fri, 23 Nov 2007 10:54:48 +0100
> Subject: [PATCH] Don't restart winbind if a corrupted tdb is found during initialization.
>
> The tdb is validated before it gets initialized. Since then sighandlers changed
> a restart isn't needed anymore.
> ---
> source/winbindd/winbindd.c | 24 +++++++++---------------
> source/winbindd/winbindd_cache.c | 3 +--
> 2 files changed, 10 insertions(+), 17 deletions(-)
>
> diff --git a/source/winbindd/winbindd.c b/source/winbindd/winbindd.c
> index 4194b55..f685256 100644
> --- a/source/winbindd/winbindd.c
> +++ b/source/winbindd/winbindd.c
> @@ -1159,20 +1159,6 @@ int main(int argc, char **argv, char **envp)
>
> pidfile_create("winbindd");
>
> - /* Ensure all cache and idmap caches are consistent
> - before we startup. */
> -
> - if (winbindd_validate_cache()) {
> - /* We have a bad cache, but luckily we
> - just deleted it. Restart ourselves */
> - int i;
> - /* Ensure we have no open low fd's. */
> - for (i = 3; i < 100; i++) {
> - close(i);
> - }
> - return execve(argv[0], argv, envp);
> - }
> -
> #if HAVE_SETPGID
> /*
> * If we're interactive we want to set our own process group for
> @@ -1190,7 +1176,15 @@ int main(int argc, char **argv, char **envp)
> DEBUG(0, ("unable to initialize messaging system\n"));
> exit(1);
> }
> -
> +
> + /*
> + * Ensure all cache and idmap caches are consistent
> + * before we startup.
> + */
> + if (winbindd_validate_cache() < 0) {
> + DEBUG(0, ("corrupted tdb found, trying to restore backup\n"));
> + }
> +
> /* Initialize cache (ensure version is correct). */
> if (!initialize_winbindd_cache()) {
> exit(1);
> diff --git a/source/winbindd/winbindd_cache.c b/source/winbindd/winbindd_cache.c
> index f3aa0fc..7ec8208 100644
> --- a/source/winbindd/winbindd_cache.c
> +++ b/source/winbindd/winbindd_cache.c
> @@ -3338,8 +3338,7 @@ static void validate_panic(const char *const why)
>
> /***********************************************************************
> Try and validate every entry in the winbindd cache. If we fail here,
> - delete the cache tdb and return non-zero - the caller (main winbindd
> - function) will restart us as we don't know if we crashed or not.
> + delete the cache tdb and return non-zero.
> ***********************************************************************/
>
> int winbindd_validate_cache(void)
> --
> 1.5.3.4
>
> From 36670f458fbf5bcd30b12507ee51da1a0a147432 Mon Sep 17 00:00:00 2001
> From: Andreas Schneider <anschneider at suse.de>
> Date: Mon, 26 Nov 2007 11:44:30 +0100
> Subject: [PATCH] Prevent winbindd from segfaulting due to corrupted cache tdb.
>
> If we try to flush the caches and due to a corrupted tdb we and have no tdb
> context close the tdb and validate it. Initialize the cache afterwards again.
> ---
> source/winbindd/winbindd.c | 18 +++++++++++++++++-
> source/winbindd/winbindd_cache.c | 22 +++++++++++++++++++---
> 2 files changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/source/winbindd/winbindd.c b/source/winbindd/winbindd.c
> index f685256..66d3171 100644
> --- a/source/winbindd/winbindd.c
> +++ b/source/winbindd/winbindd.c
> @@ -121,7 +121,23 @@ static void flush_caches(void)
> otherwise cached access denied errors due to restrict anonymous
> hang around until the sequence number changes. */
>
> - wcache_invalidate_cache();
> + if (wcache_invalidate_cache() < 0) {
> + DEBUG(0, ("invalidating the cache failed; revalidate the cache\n"));
> + /* Close the cache to be able to valdite the cache */
> + close_winbindd_cache();
> + /*
> + * Ensure all cache and idmap caches are consistent
> + * before we initialize the cache again.
> + */
> + if (winbindd_validate_cache() < 0) {
> + DEBUG(0, ("corrupted tdb found, trying to restore backup\n"));
> + }
> +
> + /* Initialize cache again. */
> + if (!initialize_winbindd_cache()) {
> + exit(1);
> + }
> + }
> }
>
> /* Handle the signal by unlinking socket and exiting */
> diff --git a/source/winbindd/winbindd_cache.c b/source/winbindd/winbindd_cache.c
> index 7ec8208..07b4d46 100644
> --- a/source/winbindd/winbindd_cache.c
> +++ b/source/winbindd/winbindd_cache.c
> @@ -2249,7 +2249,7 @@ void wcache_invalidate_samlogon(struct winbindd_domain *domain,
> netsamlogon_clear_cached_user(cache->tdb, info3);
> }
>
> -void wcache_invalidate_cache(void)
> +int wcache_invalidate_cache(void)
> {
> struct winbindd_domain *domain;
>
> @@ -2258,9 +2258,15 @@ void wcache_invalidate_cache(void)
>
> DEBUG(10, ("wcache_invalidate_cache: invalidating cache "
> "entries for %s\n", domain->name));
> - if (cache)
> - tdb_traverse(cache->tdb, traverse_fn, NULL);
> + if (cache) {
> + if (cache->tdb) {
> + tdb_traverse(cache->tdb, traverse_fn, NULL);
> + } else {
> + return -1;
> + }
> + }
> }
> + return 0;
> }
>
> bool init_wcache(void)
> @@ -2342,6 +2348,16 @@ bool initialize_winbindd_cache(void)
> return True;
> }
>
> +void close_winbindd_cache()
> +{
> + if (!wcache)
> + return;
> + if (wcache->tdb) {
> + tdb_close(wcache->tdb);
> + wcache->tdb = NULL;
> + }
> +}
> +
> void cache_store_response(pid_t pid, struct winbindd_response *response)
> {
> fstring key_str;
> --
> 1.5.3.4
--
Michael Adam <ma at sernet.de>
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.SerNet.DE, mailto: Info @ SerNet.DE
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 206 bytes
Desc: not available
Url : http://lists.samba.org/archive/samba-technical/attachments/20071213/e58a5f79/attachment.bin
More information about the samba-technical
mailing list