* [PATCH] [BZ #20900] Call __res_vinit if _PATH_RESCONF is changed
@ 2016-12-01 18:56 H.J. Lu
2016-12-01 19:02 ` Joseph Myers
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: H.J. Lu @ 2016-12-01 18:56 UTC (permalink / raw)
To: GNU C Library
The content of _PATH_RESCONF may change over time with DHCP.
__res_maybe_init should check if _PATH_RESCONF is changed. Otherwise
a long-running process will get wrong DNS results if _PATH_RESCONF is
changed by DHCP.
Any comments?
H.J.
---
[BZ #20900]
* resolv/res_libc.c: Include <sys/stat.h>.
(__res_maybe_init): Call __res_vinit if _PATH_RESCONF has been
changed since the last time.
---
resolv/res_libc.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/resolv/res_libc.c b/resolv/res_libc.c
index a4b376f..47b2d42 100644
--- a/resolv/res_libc.c
+++ b/resolv/res_libc.c
@@ -23,6 +23,7 @@
#include <sys/types.h>
#include <netinet/in.h>
#include <arpa/nameser.h>
+#include <sys/stat.h>
#include <resolv.h>
#include <libc-lock.h>
@@ -97,6 +98,21 @@ __res_maybe_init (res_state resp, int preinit)
if (resp->nscount > 0)
__res_iclose (resp, true);
return __res_vinit (resp, 1);
+ } else {
+ struct stat buf;
+
+ /* Call __res_vinit if _PATH_RESCONF has been
+ changed since the last time. */
+ if (stat (_PATH_RESCONF, &buf) == 0) {
+ static struct timespec mtime;
+ if (mtime.tv_sec != buf.st_mtim.tv_sec
+ || mtime.tv_nsec != buf.st_mtim.tv_nsec) {
+ mtime = buf.st_mtim;
+ if (resp->nscount > 0)
+ __res_iclose (resp, true);
+ return __res_vinit (resp, 1);
+ }
+ }
}
return 0;
} else if (preinit) {
--
2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [BZ #20900] Call __res_vinit if _PATH_RESCONF is changed
2016-12-01 18:56 [PATCH] [BZ #20900] Call __res_vinit if _PATH_RESCONF is changed H.J. Lu
@ 2016-12-01 19:02 ` Joseph Myers
2016-12-01 19:23 ` H.J. Lu
2016-12-01 19:22 ` Andreas Schwab
2016-12-02 7:35 ` Florian Weimer
2 siblings, 1 reply; 9+ messages in thread
From: Joseph Myers @ 2016-12-01 19:02 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
On Thu, 1 Dec 2016, H.J. Lu wrote:
> The content of _PATH_RESCONF may change over time with DHCP.
> __res_maybe_init should check if _PATH_RESCONF is changed. Otherwise
> a long-running process will get wrong DNS results if _PATH_RESCONF is
> changed by DHCP.
>
> Any comments?
This bug is either a duplicate of or related to bug 984. There are
various patches out there, e.g. one in Debian, and also claims of problems
with those patches.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [BZ #20900] Call __res_vinit if _PATH_RESCONF is changed
2016-12-01 18:56 [PATCH] [BZ #20900] Call __res_vinit if _PATH_RESCONF is changed H.J. Lu
2016-12-01 19:02 ` Joseph Myers
@ 2016-12-01 19:22 ` Andreas Schwab
2016-12-01 19:29 ` H.J. Lu
2016-12-02 7:35 ` Florian Weimer
2 siblings, 1 reply; 9+ messages in thread
From: Andreas Schwab @ 2016-12-01 19:22 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library, H.J. Lu
On Dez 01 2016, "H.J. Lu" <hongjiu.lu@intel.com> wrote:
> @@ -97,6 +98,21 @@ __res_maybe_init (res_state resp, int preinit)
> if (resp->nscount > 0)
> __res_iclose (resp, true);
> return __res_vinit (resp, 1);
> + } else {
> + struct stat buf;
> +
> + /* Call __res_vinit if _PATH_RESCONF has been
> + changed since the last time. */
> + if (stat (_PATH_RESCONF, &buf) == 0) {
> + static struct timespec mtime;
> + if (mtime.tv_sec != buf.st_mtim.tv_sec
> + || mtime.tv_nsec != buf.st_mtim.tv_nsec) {
> + mtime = buf.st_mtim;
> + if (resp->nscount > 0)
> + __res_iclose (resp, true);
> + return __res_vinit (resp, 1);
This isn't thread-safe.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [BZ #20900] Call __res_vinit if _PATH_RESCONF is changed
2016-12-01 19:02 ` Joseph Myers
@ 2016-12-01 19:23 ` H.J. Lu
2016-12-02 9:23 ` Mike Frysinger
0 siblings, 1 reply; 9+ messages in thread
From: H.J. Lu @ 2016-12-01 19:23 UTC (permalink / raw)
To: Joseph Myers; +Cc: GNU C Library
On Thu, Dec 1, 2016 at 11:02 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Thu, 1 Dec 2016, H.J. Lu wrote:
>
>> The content of _PATH_RESCONF may change over time with DHCP.
>> __res_maybe_init should check if _PATH_RESCONF is changed. Otherwise
>> a long-running process will get wrong DNS results if _PATH_RESCONF is
>> changed by DHCP.
>>
>> Any comments?
>
> This bug is either a duplicate of or related to bug 984. There are
> various patches out there, e.g. one in Debian, and also claims of problems
> with those patches.
It looks very similar. Does anyone have a link to Debian's patch?
--
H.J.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [BZ #20900] Call __res_vinit if _PATH_RESCONF is changed
2016-12-01 19:22 ` Andreas Schwab
@ 2016-12-01 19:29 ` H.J. Lu
0 siblings, 0 replies; 9+ messages in thread
From: H.J. Lu @ 2016-12-01 19:29 UTC (permalink / raw)
To: Andreas Schwab; +Cc: H.J. Lu, GNU C Library
On Thu, Dec 1, 2016 at 11:21 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> On Dez 01 2016, "H.J. Lu" <hongjiu.lu@intel.com> wrote:
>
>> @@ -97,6 +98,21 @@ __res_maybe_init (res_state resp, int preinit)
>> if (resp->nscount > 0)
>> __res_iclose (resp, true);
>> return __res_vinit (resp, 1);
>> + } else {
>> + struct stat buf;
>> +
>> + /* Call __res_vinit if _PATH_RESCONF has been
>> + changed since the last time. */
>> + if (stat (_PATH_RESCONF, &buf) == 0) {
>> + static struct timespec mtime;
>> + if (mtime.tv_sec != buf.st_mtim.tv_sec
>> + || mtime.tv_nsec != buf.st_mtim.tv_nsec) {
>> + mtime = buf.st_mtim;
>> + if (resp->nscount > 0)
>> + __res_iclose (resp, true);
>> + return __res_vinit (resp, 1);
>
> This isn't thread-safe.
>
True. We can use __thread:
diff --git a/resolv/res_libc.c b/resolv/res_libc.c
index 47b2d42..f5b40ae 100644
--- a/resolv/res_libc.c
+++ b/resolv/res_libc.c
@@ -104,7 +104,7 @@ __res_maybe_init (res_state resp, int preinit)
/* Call __res_vinit if _PATH_RESCONF has been
changed since the last time. */
if (stat (_PATH_RESCONF, &buf) == 0) {
- static struct timespec mtime;
+ static __thread struct timespec mtime;
if (mtime.tv_sec != buf.st_mtim.tv_sec
|| mtime.tv_nsec != buf.st_mtim.tv_nsec) {
mtime = buf.st_mtim;
--
H.J.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [BZ #20900] Call __res_vinit if _PATH_RESCONF is changed
2016-12-01 18:56 [PATCH] [BZ #20900] Call __res_vinit if _PATH_RESCONF is changed H.J. Lu
2016-12-01 19:02 ` Joseph Myers
2016-12-01 19:22 ` Andreas Schwab
@ 2016-12-02 7:35 ` Florian Weimer
2 siblings, 0 replies; 9+ messages in thread
From: Florian Weimer @ 2016-12-02 7:35 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
On 12/01/2016 07:56 PM, H.J. Lu wrote:
> The content of _PATH_RESCONF may change over time with DHCP.
> __res_maybe_init should check if _PATH_RESCONF is changed. Otherwise
> a long-running process will get wrong DNS results if _PATH_RESCONF is
> changed by DHCP.
>
> Any comments?
This is not the correct approach. You need a pristine copy of _res and
check if the application hasn't made any changes because this is a
public interface and such changes are expected (and in one case, even
encouraged). The existing behavior (re-init all threads if we re-init
one) is buggy for the same reason.
I'm working on a new resolv.conf parser which will address these issues
and also eliminate the search path limit.
Florian
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [BZ #20900] Call __res_vinit if _PATH_RESCONF is changed
2016-12-01 19:23 ` H.J. Lu
@ 2016-12-02 9:23 ` Mike Frysinger
2016-12-02 13:48 ` Andreas Schwab
0 siblings, 1 reply; 9+ messages in thread
From: Mike Frysinger @ 2016-12-02 9:23 UTC (permalink / raw)
To: H.J. Lu; +Cc: Joseph Myers, GNU C Library
On Thu, Dec 1, 2016 at 2:23 PM, H.J. Lu wrote:
> On Thu, Dec 1, 2016 at 11:02 AM, Joseph Myers wrote:
>> On Thu, 1 Dec 2016, H.J. Lu wrote:
>>
>>> The content of _PATH_RESCONF may change over time with DHCP.
>>> __res_maybe_init should check if _PATH_RESCONF is changed. Otherwise
>>> a long-running process will get wrong DNS results if _PATH_RESCONF is
>>> changed by DHCP.
>>>
>>> Any comments?
>>
>> This bug is either a duplicate of or related to bug 984. There are
>> various patches out there, e.g. one in Debian, and also claims of problems
>> with those patches.
>
> It looks very similar. Does anyone have a link to Debian's patch?
here's the patch i've been carrying in Gentoo for almost a decade, and
i took it from SUSE:
https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=727cea2ca77b47681f4678d5fb3180aab89fe9ab
-mike
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [BZ #20900] Call __res_vinit if _PATH_RESCONF is changed
2016-12-02 9:23 ` Mike Frysinger
@ 2016-12-02 13:48 ` Andreas Schwab
2016-12-02 14:53 ` Mike Frysinger
0 siblings, 1 reply; 9+ messages in thread
From: Andreas Schwab @ 2016-12-02 13:48 UTC (permalink / raw)
To: Mike Frysinger; +Cc: H.J. Lu, Joseph Myers, GNU C Library
On Dez 02 2016, Mike Frysinger <vapier@gentoo.org> wrote:
> i took it from SUSE:
> https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=727cea2ca77b47681f4678d5fb3180aab89fe9ab
This is outdated, here is the current version used by openSUSE:
https://build.opensuse.org/package/view_file/openSUSE:Factory/glibc/glibc-resolv-reload.diff?expand=1
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] [BZ #20900] Call __res_vinit if _PATH_RESCONF is changed
2016-12-02 13:48 ` Andreas Schwab
@ 2016-12-02 14:53 ` Mike Frysinger
0 siblings, 0 replies; 9+ messages in thread
From: Mike Frysinger @ 2016-12-02 14:53 UTC (permalink / raw)
To: Andreas Schwab; +Cc: H.J. Lu, Joseph Myers, GNU C Library
On Fri, Dec 2, 2016 at 8:48 AM, Andreas Schwab wrote:
> On Dez 02 2016, Mike Frysinger wrote:
>> i took it from SUSE:
>> https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=727cea2ca77b47681f4678d5fb3180aab89fe9ab
>
> This is outdated, here is the current version used by openSUSE:
>
> https://build.opensuse.org/package/view_file/openSUSE:Factory/glibc/glibc-resolv-reload.diff?expand=1
is the new lock there to deal with possible non-atomic updates to
resconf_mtime ? e.g. a 64-bit time_t on a system w/32-bit registers.
-mike
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-12-02 14:53 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-01 18:56 [PATCH] [BZ #20900] Call __res_vinit if _PATH_RESCONF is changed H.J. Lu
2016-12-01 19:02 ` Joseph Myers
2016-12-01 19:23 ` H.J. Lu
2016-12-02 9:23 ` Mike Frysinger
2016-12-02 13:48 ` Andreas Schwab
2016-12-02 14:53 ` Mike Frysinger
2016-12-01 19:22 ` Andreas Schwab
2016-12-01 19:29 ` H.J. Lu
2016-12-02 7:35 ` Florian Weimer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).