[BZ,#20900] Call __res_vinit if _PATH_RESCONF is changed
Commit Message
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(+)
Comments
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.
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.
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?
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
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
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.
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
@@ -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) {