From patchwork Fri Jun 30 23:21:03 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 21378 Received: (qmail 80519 invoked by alias); 30 Jun 2017 23:21:11 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 80502 invoked by uid 89); 30 Jun 2017 23:21:10 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 6BC808008D Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=fweimer@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 6BC808008D Subject: Re: [PATCH] resolv: Automatically reload a changed /etc/resolv.conf file [BZ #984] To: libc-alpha@sourceware.org References: <20170630193825.35DFF439942F0@oldenburg.str.redhat.com> From: Florian Weimer Message-ID: Date: Sat, 1 Jul 2017 01:21:03 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: <20170630193825.35DFF439942F0@oldenburg.str.redhat.com> This patch actually builds on 32-bit architectures. (I missed the custom atomics infrastructure in resolv/res_libc.c.) Thanks, Florian resolv: Automatically reload a changed /etc/resolv.conf file [BZ #984] This commit enhances the stub resolver to reload the configuration in the per-thread _res object if the /etc/resolv.conf file has changed. The resolver checks whether the application has modified _res and will not overwrite the _res object in that case. The struct resolv_context mechanism is used to check the configuration file only once per name lookup. 2017-06-30 Florian Weimer [BZ #984] Reload /etc/resolv.conf if has been changed on disk. * resolv/resolv_conf.h (resolv_conf): Remove initstamp member. (__resolv_conf_load, __resolv_conf_get_current): Declare. * resolv/resolv_conf.c (struct resolv_conf_global): Add conf_current, conf_mtime, conf_ctime, conf_size, conf_ino members. (decrement, __resolv_conf_get_current): New functions. (resolv_conf_get_1): Call decrement. (__resolv_conf_allocate): Do not initialize initstamp. Call decrement. (freeres): Deallocate global.conf_current. * resolv/resolv.h (RES_NORELOAD): Define. * resolv/res_debug.c (p_option): Add RES_NORELOAD. * resolv/resolv-internal.h (__res_initstamp): Remove declaration. * resolv/resolv_context.c (replicated_configuration_matches): New. (maybe_init): Call it. Use __resolv_conf_get_current to obtain the current configration. * resolv/res_init.c (__res_initstamp): Remove variable definition. (has_preinit_values): New function. (__resolv_conf_load): Renamed from __res_vinit. Drop res_state parameter and do not call __resolv_conf_attach. (__res_vinit): Reimplement based __resolv_conf_load. (res_options): Handle no-reload. * resolv/res_libc.c (atomicinclock, atomicincunlock, atomicinc) (lock): Remove. (res_int): Do not update __res_initstamp. * resolv/tst-resolv-res_init-skeleton.c (print_resp): Handle RES_NORELOAD. (test_cases): Test no-reload. diff --git a/NEWS b/NEWS index bf2b5df..6f0322e 100644 --- a/NEWS +++ b/NEWS @@ -245,6 +245,10 @@ Version 2.26 array, which is limited to six entries) will only use the first six search domains, as before. +* The glibc DNS stub now automatically reloads /etc/resolv.conf to pick up + changed configuration settings. The new “no-reload” (RES_NORELOAD) + resolver option disables this behavior. + Security related changes: * The DNS stub resolver limits the advertised UDP buffer size to 1200 bytes, diff --git a/resolv/res_debug.c b/resolv/res_debug.c index 55d1fe9..919b86e 100644 --- a/resolv/res_debug.c +++ b/resolv/res_debug.c @@ -613,6 +613,7 @@ p_option(u_long option) { case RES_SNGLKUPREOP: return "single-request-reopen"; case RES_USE_DNSSEC: return "dnssec"; case RES_NOTLDQUERY: return "no-tld-query"; + case RES_NORELOAD: return "no-reload"; /* XXX nonreentrant */ default: sprintf(nbuf, "?0x%lx?", (u_long)option); return (nbuf); diff --git a/resolv/res_init.c b/resolv/res_init.c index 80a21fb..fa46ce7 100644 --- a/resolv/res_init.c +++ b/resolv/res_init.c @@ -106,8 +106,6 @@ static uint32_t net_mask (struct in_addr); -unsigned long long int __res_initstamp; - int res_ninit (res_state statp) { @@ -164,6 +162,16 @@ struct resolv_conf_parser struct resolv_conf template; }; +/* Return true if *PREINIT contains actual preinitialization. */ +static bool +has_preinit_values (const struct __res_state *preinit) +{ + return (preinit->retrans != 0 && preinit->retrans != RES_TIMEOUT) + || (preinit->retry != 0 && preinit->retry != RES_DFLRETRY) + || (preinit->options != 0 + && (preinit->options & ~RES_INIT) != RES_DEFAULT); +} + static void resolv_conf_parser_init (struct resolv_conf_parser *parser, const struct __res_state *preinit) @@ -531,14 +539,8 @@ res_vinit_1 (FILE *fp, struct resolv_conf_parser *parser) return true; } -/* Set up default settings. If the /etc/resolv.conf configuration - file exist, the values there will have precedence. Otherwise, the - server address is set to INADDR_LOOPBACK and the default domain - name comes from gethostname. The RES_OPTIONS and LOCALDOMAIN - environment variables can be used to override some settings. - Return 0 if completes successfully, -1 on error. */ -int -__res_vinit (res_state statp, int preinit) +struct resolv_conf * +__resolv_conf_load (struct __res_state *preinit) { /* Ensure that /etc/hosts.conf has been loaded (once). */ _res_hconf_init (); @@ -559,20 +561,14 @@ __res_vinit (res_state statp, int preinit) default: /* Other errors refer to resource allocation problems and need to be handled by the application. */ - return -1; + return NULL; } struct resolv_conf_parser parser; - if (preinit) - { - resolv_conf_parser_init (&parser, statp); - statp->id = res_randomid (); - } - else - resolv_conf_parser_init (&parser, NULL); + resolv_conf_parser_init (&parser, preinit); - bool ok = res_vinit_1 (fp, &parser); - if (ok) + struct resolv_conf *conf = NULL; + if (res_vinit_1 (fp, &parser)) { parser.template.nameserver_list = nameserver_list_begin (&parser.nameserver_list); @@ -583,21 +579,42 @@ __res_vinit (res_state statp, int preinit) = search_list_size (&parser.search_list); parser.template.sort_list = sort_list_begin (&parser.sort_list); parser.template.sort_list_size = sort_list_size (&parser.sort_list); - struct resolv_conf *conf = __resolv_conf_allocate (&parser.template); - if (conf == NULL) - ok = false; - else - { - ok = __resolv_conf_attach (statp, conf); - __resolv_conf_put (conf); - } + conf = __resolv_conf_allocate (&parser.template); } resolv_conf_parser_free (&parser); - if (!ok) + return conf; +} + +/* Set up default settings. If the /etc/resolv.conf configuration + file exist, the values there will have precedence. Otherwise, the + server address is set to INADDR_LOOPBACK and the default domain + name comes from gethostname. The RES_OPTIONS and LOCALDOMAIN + environment variables can be used to override some settings. + Return 0 if completes successfully, -1 on error. */ +int +__res_vinit (res_state statp, int preinit) +{ + struct resolv_conf *conf; + if (preinit && has_preinit_values (statp)) + /* For the preinit case, we cannot use the cached configuration + because some settings could be different. */ + conf = __resolv_conf_load (statp); + else + conf = __resolv_conf_get_current (); + if (conf == NULL) return -1; + + bool ok = __resolv_conf_attach (statp, conf); + __resolv_conf_put (conf); + if (ok) + { + if (preinit) + statp->id = res_randomid (); + return 0; + } else - return 0; + return -1; } static void @@ -652,6 +669,7 @@ res_setoptions (struct resolv_conf_parser *parser, const char *options) { STRnLEN ("single-request"), 0, RES_SNGLKUP }, { STRnLEN ("no_tld_query"), 0, RES_NOTLDQUERY }, { STRnLEN ("no-tld-query"), 0, RES_NOTLDQUERY }, + { STRnLEN ("no-reload"), 0, RES_NORELOAD }, { STRnLEN ("use-vc"), 0, RES_USEVC } }; #define noptions (sizeof (options) / sizeof (options[0])) diff --git a/resolv/res_libc.c b/resolv/res_libc.c index 5066983..9f2d3c3 100644 --- a/resolv/res_libc.c +++ b/resolv/res_libc.c @@ -42,18 +42,6 @@ #include #include -/* We have atomic increment operations on 64-bit platforms. */ -#if __WORDSIZE == 64 -# define atomicinclock(lock) (void) 0 -# define atomicincunlock(lock) (void) 0 -# define atomicinc(var) catomic_increment (&(var)) -#else -__libc_lock_define_initialized (static, lock); -# define atomicinclock(lock) __libc_lock_lock (lock) -# define atomicincunlock(lock) __libc_lock_unlock (lock) -# define atomicinc(var) ++var -#endif - int res_init (void) { @@ -90,12 +78,6 @@ res_init (void) if (!_res.id) _res.id = res_randomid (); - atomicinclock (lock); - /* Request all threads to re-initialize their resolver states, - resolv.conf might have changed. */ - atomicinc (__res_initstamp); - atomicincunlock (lock); - return __res_vinit (&_res, 1); } diff --git a/resolv/resolv-internal.h b/resolv/resolv-internal.h index 9246497..32dc447 100644 --- a/resolv/resolv-internal.h +++ b/resolv/resolv-internal.h @@ -97,7 +97,4 @@ int __res_nopt (struct resolv_context *, int n0, int __inet_pton_length (int af, const char *src, size_t srclen, void *); libc_hidden_proto (__inet_pton_length) -/* Used to propagate the effect of res_init calls across threads. */ -extern unsigned long long int __res_initstamp attribute_hidden; - #endif /* _RESOLV_INTERNAL_H */ diff --git a/resolv/resolv.h b/resolv/resolv.h index 1fb0ad4..c80eb4c 100644 --- a/resolv/resolv.h +++ b/resolv/resolv.h @@ -134,6 +134,7 @@ struct res_sym { #define RES_USE_DNSSEC 0x00800000 /* use DNSSEC using OK bit in OPT */ #define RES_NOTLDQUERY 0x01000000 /* Do not look up unqualified name as a TLD. */ +#define RES_NORELOAD 0x02000000 /* No automatic configuration reload. */ #define RES_DEFAULT (RES_RECURSE|RES_DEFNAMES|RES_DNSRCH) diff --git a/resolv/resolv_conf.c b/resolv/resolv_conf.c index cd2f60d..3cd8ae9 100644 --- a/resolv/resolv_conf.c +++ b/resolv/resolv_conf.c @@ -22,6 +22,7 @@ #include #include #include +#include /* _res._u._ext.__glibc_extension_index is used as an index into a struct resolv_conf_array object. The intent of this construction @@ -54,6 +55,16 @@ struct resolv_conf_global the array element is overwritten with NULL. */ struct resolv_conf_array array; + /* Cached current configuration object for /etc/resolv.conf. */ + struct resolv_conf *conf_current; + + /* These properties of /etc/resolv.conf are used to check if the + configuration needs reloading. */ + struct timespec conf_mtime; + struct timespec conf_ctime; + off64_t conf_size; + ino64_t conf_ino; + /* The lock synchronizes access to the other members of this struct. It also protects the __refcount member of struct resolv_conf. */ __libc_lock_define (, lock); @@ -80,6 +91,84 @@ put_locked_global (struct resolv_conf_global *global_copy) __libc_lock_unlock (global_copy->lock); } +/* Decrement the reference counter for *CONF. */ +static void +decrement (struct resolv_conf *conf) +{ + assert (conf->__refcount > 0); + if (--conf->__refcount == 0) + free (conf); +} + +struct resolv_conf * +__resolv_conf_get_current (void) +{ + struct stat64 st; + if (stat64 (_PATH_RESCONF, &st) != 0) + { + switch (errno) + { + case EACCES: + case EISDIR: + case ELOOP: + case ENOENT: + case ENOTDIR: + case EPERM: + /* Ignore errors due to file system contents. */ + memset (&st, 0, sizeof (st)); + break; + default: + /* Other errors are fatal. */ + return NULL; + } + } + + struct resolv_conf_global *global_copy = get_locked_global (); + if (global_copy == NULL) + return NULL; + struct resolv_conf *conf; + if (global_copy->conf_current != NULL + && (global_copy->conf_mtime.tv_sec == st.st_mtim.tv_sec + && global_copy->conf_mtime.tv_nsec == st.st_mtim.tv_nsec + && global_copy->conf_ctime.tv_sec == st.st_ctim.tv_sec + && global_copy->conf_ctime.tv_nsec == st.st_ctim.tv_nsec + && global_copy->conf_ino == st.st_ino + && global_copy->conf_size == st.st_size)) + /* We can reuse the cached configuration object. */ + conf = global_copy->conf_current; + else + { + /* Parse configuration while holding the lock. This avoids + duplicate work. */ + conf = __resolv_conf_load (NULL); + if (conf != NULL) + { + if (global_copy->conf_current != NULL) + decrement (global_copy->conf_current); + global_copy->conf_current = conf; /* Takes ownership. */ + + /* Update file modification stamps. The configuration we + read could be a newer version of the file, but this does + not matter because this will lead to an extraneous reload + later. */ + global_copy->conf_mtime = st.st_mtim; + global_copy->conf_ctime = st.st_ctim; + global_copy->conf_ino = st.st_ino; + global_copy->conf_size = st.st_size; + } + } + + if (conf != NULL) + { + /* Return an additional reference. */ + assert (conf->__refcount > 0); + ++conf->__refcount; + assert (conf->__refcount > 0); + } + put_locked_global (global_copy); + return conf; +} + /* Internal implementation of __resolv_conf_get, without validation against *RESP. */ static struct resolv_conf * @@ -241,13 +330,9 @@ __resolv_conf_put (struct resolv_conf *conf) if (conf == NULL) return; - void *to_deallocate = NULL; __libc_lock_lock (global.lock); - if (--conf->__refcount == 0) - to_deallocate = conf; + decrement (conf); __libc_lock_unlock (global.lock); - - free (to_deallocate); } struct resolv_conf * @@ -302,7 +387,6 @@ __resolv_conf_allocate (const struct resolv_conf *init) conf->retry = init->retry; conf->options = init->options; conf->ndots = init->ndots; - conf->initstamp = __res_initstamp; /* Allocate the arrays with pointers. These must come first because they have the highets alignment. */ @@ -461,9 +545,7 @@ decrement_at_index (struct resolv_conf_global *global_copy, size_t index) struct resolv_conf *conf = *slot; if (conf != NULL) { - assert (conf->__refcount > 0); - if (--conf->__refcount == 0) - free (conf); + decrement (conf); /* Clear the slot even if the reference count is positive. Slots are not shared. */ *slot = NULL; @@ -559,10 +641,17 @@ static void __attribute__ ((section ("__libc_thread_freeres_fn"))) freeres (void) { /* No locking because this function is supposed to be called when - the process has turned single-threaded. Note that this frees - only the array itself. The pointed-to configuration objects - should have been deallocated by res_nclose and per-thread cleanup - functions. */ + the process has turned single-threaded. */ + struct resolv_conf_global *global_copy = &global; + if (global_copy->conf_current != NULL) + { + decrement (global_copy->conf_current); + global_copy->conf_current = NULL; + } + + /* Note that this frees only the array itself. The pointed-to + configuration objects should have been deallocated by res_nclose + and per-thread cleanup functions. */ resolv_conf_array_free (&global.array); } text_set_element (__libc_subfreeres, freeres); diff --git a/resolv/resolv_conf.h b/resolv/resolv_conf.h index 7ca80cd..0ff8bd7 100644 --- a/resolv/resolv_conf.h +++ b/resolv/resolv_conf.h @@ -35,11 +35,6 @@ struct resolv_sortlist_entry object. */ struct resolv_conf { - /* Used to propagate the effect of res_init across threads. This - member is mutable and prevents sharing of the same struct - resolv_conf object among multiple struct __res_state objects. */ - unsigned long long int initstamp; - /* Reference counter. The object is deallocated once it reaches zero. For internal use within resolv_conf only. */ size_t __refcount; @@ -69,6 +64,18 @@ struct resolv_conf struct __res_state; +/* Read /etc/resolv.conf and return a configuration object, or NULL if + /etc/resolv.conf cannot be read due to memory allocation errors. + If PREINIT is not NULL, some configuration values are taken from the + struct __res_state object. */ +struct resolv_conf *__resolv_conf_load (struct __res_state *preinit) + attribute_hidden __attribute__ ((warn_unused_result)); + +/* Return a configuration object for the current /etc/resolv.conf + settings, or NULL on failure. The object is cached. */ +struct resolv_conf *__resolv_conf_get_current (void) + attribute_hidden __attribute__ ((warn_unused_result)); + /* Return the extended resolver state for *RESP, or NULL if it cannot be determined. A call to this function must be paired with a call to __resolv_conf_put. */ diff --git a/resolv/resolv_context.c b/resolv/resolv_context.c index 0ee2184..35d4b3d 100644 --- a/resolv/resolv_context.c +++ b/resolv/resolv_context.c @@ -51,6 +51,20 @@ resolver state. */ static __thread struct resolv_context *current attribute_tls_model_ie; +/* The resolv_conf handling will gives us a ctx->conf pointer even if + these fields do not match because a mis-match does not cause a loss + of state (_res objects can store the full information). This + function checks to ensure that there is a full patch, to prevent + overwriting a patched configuration. */ +static bool +replicated_configuration_matches (const struct resolv_context *ctx) +{ + return ctx->resp->options == ctx->conf->options + && ctx->resp->retrans == ctx->conf->retrans + && ctx->resp->retry == ctx->conf->retry + && ctx->resp->ndots == ctx->conf->ndots; +} + /* Initialize *RESP if RES_INIT is not yet set in RESP->options, or if res_init in some other thread requested re-initializing. */ static __attribute__ ((warn_unused_result)) bool @@ -59,27 +73,36 @@ maybe_init (struct resolv_context *ctx, bool preinit) struct __res_state *resp = ctx->resp; if (resp->options & RES_INIT) { + if (resp->options & RES_NORELOAD) + /* Configuration reloading was explicitly disabled. */ + return true; + /* If there is no associated resolv_conf object despite the initialization, something modified *ctx->resp. Do not override those changes. */ - if (ctx->conf != NULL && ctx->conf->initstamp != __res_initstamp) + if (ctx->conf != NULL && replicated_configuration_matches (ctx)) { - if (resp->nscount > 0) - /* This call will detach the extended resolver state. */ - __res_iclose (resp, true); - /* And this call will attach it again. */ - if (__res_vinit (resp, 1) < 0) + struct resolv_conf *current = __resolv_conf_get_current (); + if (current == NULL) + return false; + + /* Check if the configuration changed. */ + if (current != ctx->conf) { - /* The configuration no longer matches after failed - initialization. */ - __resolv_conf_put (ctx->conf); - ctx->conf = NULL; - return false; + /* This call will detach the extended resolver state. */ + if (resp->nscount > 0) + __res_iclose (resp, true); + /* Reattach the current configuration. */ + if (__resolv_conf_attach (ctx->resp, current)) + { + __resolv_conf_put (ctx->conf); + /* ctx takes ownership, so we do not release current. */ + ctx->conf = current; + } } - /* Delay the release of the old configuration until this - point, so that __res_vinit can reuse it if possible. */ - __resolv_conf_put (ctx->conf); - ctx->conf = __resolv_conf_get (ctx->resp); + else + /* No change. Drop the reference count for current. */ + __resolv_conf_put (current); } return true; } diff --git a/resolv/tst-resolv-res_init-skeleton.c b/resolv/tst-resolv-res_init-skeleton.c index aace94d..3ee5a99 100644 --- a/resolv/tst-resolv-res_init-skeleton.c +++ b/resolv/tst-resolv-res_init-skeleton.c @@ -151,6 +151,7 @@ print_resp (FILE *fp, res_state resp) print_option_flag (fp, &options, RES_SNGLKUPREOP, "single-request-reopen"); print_option_flag (fp, &options, RES_NOTLDQUERY, "no-tld-query"); + print_option_flag (fp, &options, RES_NORELOAD, "no-reload"); fputc ('\n', fp); if (options != 0) fprintf (fp, "; error: unresolved option bits: 0x%x\n", options); @@ -470,6 +471,28 @@ struct test_case test_cases[] = "nameserver 192.0.2.1\n" "; nameserver[0]: [192.0.2.1]:53\n" }, + {.name = "basic no-reload", + .conf = "options no-reload\n" + "search corp.example.com example.com\n" + "nameserver 192.0.2.1\n", + .expected = "options no-reload\n" + "search corp.example.com example.com\n" + "; search[0]: corp.example.com\n" + "; search[1]: example.com\n" + "nameserver 192.0.2.1\n" + "; nameserver[0]: [192.0.2.1]:53\n" + }, + {.name = "basic no-reload via RES_OPTIONS", + .conf = "search corp.example.com example.com\n" + "nameserver 192.0.2.1\n", + .expected = "options no-reload\n" + "search corp.example.com example.com\n" + "; search[0]: corp.example.com\n" + "; search[1]: example.com\n" + "nameserver 192.0.2.1\n" + "; nameserver[0]: [192.0.2.1]:53\n", + .res_options = "no-reload" + }, {.name = "whitespace", .conf = "# This test covers comment and whitespace processing " " (trailing whitespace,\n"