Mailing List Archive

On Friday 24 October 2008 12:02, Fredrik Pettai wrote:

> AFAICS, this fix doesn't seem to be in the latest version of the
> libspf2-1.2.8 source. Maybe someone can add it to libspf2's main CVS branch
> instead?
>

For reasons I'm not quite sure of since I don't do C code, I have access to
the libspf2 svn. I looked and this is at least partially done in SVN, but
also some of the code has been refactored, so I'm not sure the rest is
applicable. I'll make sure the maintainer is aware of the patch.

Scott K


-------------------------------------------
Sender Policy Framework: http://www.openspf.org
Modify Your Subscription: http://www.listbox.com/member/
Archives: https://www.listbox.com/member/archive/1007/=now
RSS Feed: https://www.listbox.com/member/archive/rss/1007/
Powered by Listbox: http://www.listbox.com

Hi,

>>>>> On Fri, 24 Oct 2008 18:02:36 +0200
>>>>> Fredrik Pettai <pettai@nordu.net> said:

pettai> This has been fixed by a patch in the FreeBSD Ports tree. See (and specifically
pettai> patch submission [2]):
pettai> ftp://ftp.freebsd.org/pub/FreeBSD/development/FreeBSD-CVS/ports/mail/libspf2/files/patch-src_libspf2_spf__dns__resolv.c,v

pettai> AFAICS, this fix doesn't seem to be in the latest version of the libspf2-1.2.8
pettai> source. Maybe someone can add it to libspf2's main CVS branch instead?

Here is a patch against 1.2.8:

Index: src/libspf2/spf_dns_resolv.c
diff -u -p src/libspf2/spf_dns_resolv.c.orig src/libspf2/spf_dns_resolv.c
--- src/libspf2/spf_dns_resolv.c.orig Thu Oct 16 07:02:03 2008
+++ src/libspf2/spf_dns_resolv.c Fri Oct 24 12:19:29 2008
@@ -92,7 +92,11 @@ static pthread_key_t res_state_key;
static void
SPF_dns_resolv_thread_term(void *arg)
{
+#ifdef res_ndestroy
+ res_ndestroy( (struct __res_state *)arg );
+#else
res_nclose( (struct __res_state *)arg );
+#endif
free(arg);
}

@@ -615,7 +619,7 @@ SPF_dns_resolv_new(SPF_dns_server_t *lay
#if HAVE_DECL_RES_NINIT
pthread_once(&res_state_control, SPF_dns_resolv_init_key);
#else
- if ( res_init() != 0 ) {
+ if ((_res.options & RES_INIT) == 0 && res_init() != 0) {
perror("res_init");
return NULL;
}


Sincerely,

--
Hajimu UMEMOTO @ Internet Mutual Aid Society Yokohama, Japan
ume@mahoroba.org ume@{,jp.}FreeBSD.org
http://www.imasy.org/~ume/


-------------------------------------------
Sender Policy Framework: http://www.openspf.org
Modify Your Subscription: http://www.listbox.com/member/
Archives: https://www.listbox.com/member/archive/1007/=now
RSS Feed: https://www.listbox.com/member/archive/rss/1007/
Powered by Listbox: http://www.listbox.com

Hi!

On Sat, Oct 25, 2008 at 01:48:32AM +0900, Hajimu UMEMOTO wrote:
>>>>>> On Fri, 24 Oct 2008 18:02:36 +0200
>>>>>> Fredrik Pettai <pettai@nordu.net> said:

>pettai> This has been fixed by a patch in the FreeBSD Ports tree. See (and specifically
>pettai> patch submission [2]):
>pettai> ftp://ftp.freebsd.org/pub/FreeBSD/development/FreeBSD-CVS/ports/mail/libspf2/files/patch-src_libspf2_spf__dns__resolv.c,v

>pettai> AFAICS, this fix doesn't seem to be in the latest version of the libspf2-1.2.8
>pettai> source. Maybe someone can add it to libspf2's main CVS branch instead?

>Here is a patch against 1.2.8:

>Index: src/libspf2/spf_dns_resolv.c
>diff -u -p src/libspf2/spf_dns_resolv.c.orig src/libspf2/spf_dns_resolv.c
>--- src/libspf2/spf_dns_resolv.c.orig Thu Oct 16 07:02:03 2008
>+++ src/libspf2/spf_dns_resolv.c Fri Oct 24 12:19:29 2008
>@@ -92,7 +92,11 @@ static pthread_key_t res_state_key;
> static void
> SPF_dns_resolv_thread_term(void *arg)
> {
>+#ifdef res_ndestroy
>+ res_ndestroy( (struct __res_state *)arg );
>+#else
> res_nclose( (struct __res_state *)arg );
>+#endif
> free(arg);
> }

I see this, but...

>@@ -615,7 +619,7 @@ SPF_dns_resolv_new(SPF_dns_server_t *lay
> #if HAVE_DECL_RES_NINIT
> pthread_once(&res_state_control, SPF_dns_resolv_init_key);
> #else
>- if ( res_init() != 0 ) {
>+ if ((_res.options & RES_INIT) == 0 && res_init() != 0) {
> perror("res_init");
> return NULL;
> }

Why is that needed, and how portable is that in fact, compared to
calling res_init unconditionally in that place?

Kind regards,

Hannah.


-------------------------------------------
Sender Policy Framework: http://www.openspf.org
Modify Your Subscription: http://www.listbox.com/member/
Archives: https://www.listbox.com/member/archive/1007/=now
RSS Feed: https://www.listbox.com/member/archive/rss/1007/
Powered by Listbox: http://www.listbox.com

On Sat, Oct 25, 2008 at 01:48:32AM +0900, Hajimu UMEMOTO wrote:
> Hi,
>
> >>>>> On Fri, 24 Oct 2008 18:02:36 +0200
> >>>>> Fredrik Pettai <pettai@nordu.net> said:
>
> pettai> This has been fixed by a patch in the FreeBSD Ports tree. See (and specifically
> pettai> patch submission [2]):
> pettai> ftp://ftp.freebsd.org/pub/FreeBSD/development/FreeBSD-CVS/ports/mail/libspf2/files/patch-src_libspf2_spf__dns__resolv.c,v
>
> pettai> AFAICS, this fix doesn't seem to be in the latest version of the libspf2-1.2.8
> pettai> source. Maybe someone can add it to libspf2's main CVS branch instead?
>
> Here is a patch against 1.2.8:
>
> Index: src/libspf2/spf_dns_resolv.c
> diff -u -p src/libspf2/spf_dns_resolv.c.orig src/libspf2/spf_dns_resolv.c
> --- src/libspf2/spf_dns_resolv.c.orig Thu Oct 16 07:02:03 2008
> +++ src/libspf2/spf_dns_resolv.c Fri Oct 24 12:19:29 2008
> @@ -92,7 +92,11 @@ static pthread_key_t res_state_key;
> static void
> SPF_dns_resolv_thread_term(void *arg)
> {
> +#ifdef res_ndestroy
> + res_ndestroy( (struct __res_state *)arg );
> +#else
> res_nclose( (struct __res_state *)arg );
> +#endif
> free(arg);
> }
>
> @@ -615,7 +619,7 @@ SPF_dns_resolv_new(SPF_dns_server_t *lay
> #if HAVE_DECL_RES_NINIT
> pthread_once(&res_state_control, SPF_dns_resolv_init_key);
> #else
> - if ( res_init() != 0 ) {
> + if ((_res.options & RES_INIT) == 0 && res_init() != 0) {
> perror("res_init");
> return NULL;
> }

Sorry, but this patch won't work on platform where res_* functions are
real library functions, e.g on Solaris at least up to Solaris 9 ... The
above fix is targeting mainly Linux or maybe NetBSD where res_* is just
a macro layer. Even with checking the __RES macro value (resolver
release date) this could not be catched (e.g. Solaris 9 has
res_ndestroy, Solaris 8 not but both have the same __RES value!).

AFIAK some autoconf stuff is needed to get the information if
res_ndestroy can (should) be used!


Johann Klasek



-------------------------------------------
Sender Policy Framework: http://www.openspf.org
Modify Your Subscription: http://www.listbox.com/member/
Archives: https://www.listbox.com/member/archive/1007/=now
RSS Feed: https://www.listbox.com/member/archive/rss/1007/
Powered by Listbox: http://www.listbox.com

Hi!

On Tue, Oct 28, 2008 at 03:15:20AM +0100, Johann E. Klasek wrote:
>On Sat, Oct 25, 2008 at 01:48:32AM +0900, Hajimu UMEMOTO wrote:
>> >>>>> On Fri, 24 Oct 2008 18:02:36 +0200
>> >>>>> Fredrik Pettai <pettai@nordu.net> said:

>> pettai> This has been fixed by a patch in the FreeBSD Ports tree. See (and specifically
>> pettai> patch submission [2]):
>> pettai> ftp://ftp.freebsd.org/pub/FreeBSD/development/FreeBSD-CVS/ports/mail/libspf2/files/patch-src_libspf2_spf__dns__resolv.c,v

>> pettai> AFAICS, this fix doesn't seem to be in the latest version of the libspf2-1.2.8
>> pettai> source. Maybe someone can add it to libspf2's main CVS branch instead?

>> Here is a patch against 1.2.8:

>> Index: src/libspf2/spf_dns_resolv.c
>> diff -u -p src/libspf2/spf_dns_resolv.c.orig src/libspf2/spf_dns_resolv.c
>> --- src/libspf2/spf_dns_resolv.c.orig Thu Oct 16 07:02:03 2008
>> +++ src/libspf2/spf_dns_resolv.c Fri Oct 24 12:19:29 2008
>> @@ -92,7 +92,11 @@ static pthread_key_t res_state_key;
>> static void
>> SPF_dns_resolv_thread_term(void *arg)
>> {
>> +#ifdef res_ndestroy
>> + res_ndestroy( (struct __res_state *)arg );
>> +#else
>> res_nclose( (struct __res_state *)arg );
>> +#endif
>> free(arg);
>> }

>> @@ -615,7 +619,7 @@ SPF_dns_resolv_new(SPF_dns_server_t *lay
>> #if HAVE_DECL_RES_NINIT
>> pthread_once(&res_state_control, SPF_dns_resolv_init_key);
>> #else
>> - if ( res_init() != 0 ) {
>> + if ((_res.options & RES_INIT) == 0 && res_init() != 0) {
>> perror("res_init");
>> return NULL;
>> }

>Sorry, but this patch won't work on platform where res_* functions are
>real library functions, e.g on Solaris at least up to Solaris 9 ... The
>above fix is targeting mainly Linux or maybe NetBSD where res_* is just
>a macro layer. Even with checking the __RES macro value (resolver
>release date) this could not be catched (e.g. Solaris 9 has
>res_ndestroy, Solaris 8 not but both have the same __RES value!).

>AFIAK some autoconf stuff is needed to get the information if
>res_ndestroy can (should) be used!

Makes sense.

>Johann Klasek

Kind regards,

Hannah.


-------------------------------------------
Sender Policy Framework: http://www.openspf.org
Modify Your Subscription: http://www.listbox.com/member/
Archives: https://www.listbox.com/member/archive/1007/=now
RSS Feed: https://www.listbox.com/member/archive/rss/1007/
Powered by Listbox: http://www.listbox.com

Hi!

On Tue, Oct 28, 2008 at 02:27:28PM +0100, Hannah Schroeter wrote:
>On Tue, Oct 28, 2008 at 03:15:20AM +0100, Johann E. Klasek wrote:
>>On Sat, Oct 25, 2008 at 01:48:32AM +0900, Hajimu UMEMOTO wrote:
>[...]

>>AFIAK some autoconf stuff is needed to get the information if
>>res_ndestroy can (should) be used!

>Makes sense.

And I see that in the svn repository it has been done just that way
(with autoconf), for that one first diff chunk.

Kind regards,

Hannah.


-------------------------------------------
Sender Policy Framework: http://www.openspf.org
Modify Your Subscription: http://www.listbox.com/member/
Archives: https://www.listbox.com/member/archive/1007/=now
RSS Feed: https://www.listbox.com/member/archive/rss/1007/
Powered by Listbox: http://www.listbox.com

Hi,

>>>>> On Mon, 27 Oct 2008 14:52:15 +0100
>>>>> Hannah Schroeter <hannah@schlund.de> said:

>@@ -615,7 +619,7 @@ SPF_dns_resolv_new(SPF_dns_server_t *lay
> #if HAVE_DECL_RES_NINIT
> pthread_once(&res_state_control, SPF_dns_resolv_init_key);
> #else
>- if ( res_init() != 0 ) {
>+ if ((_res.options & RES_INIT) == 0 && res_init() != 0) {
> perror("res_init");
> return NULL;
> }

hannah> Why is that needed, and how portable is that in fact, compared to
hannah> calling res_init unconditionally in that place?

Because, the libspf2 is not only consumer of the resolver. The
res_init() might be called already before calling it from libspf2. It
is enough to call res_init() once.
Basically, res_init() is used in this manner on BSDs. The impact of
calling res_init() again and again is unclear.
I'm not sure about portability, but the _res and RES_INIT is described
in resolver(8) on at least BSDs and CentOS.

Sincerely,

--
Hajimu UMEMOTO @ Internet Mutual Aid Society Yokohama, Japan
ume@mahoroba.org ume@{,jp.}FreeBSD.org
http://www.imasy.org/~ume/


-------------------------------------------
Sender Policy Framework: http://www.openspf.org
Modify Your Subscription: http://www.listbox.com/member/
Archives: https://www.listbox.com/member/archive/1007/=now
RSS Feed: https://www.listbox.com/member/archive/rss/1007/
Powered by Listbox: http://www.listbox.com

On Thu, 2008-10-30 at 02:39 +0900, Hajimu UMEMOTO wrote:
> Hi,
>
> >>>>> On Mon, 27 Oct 2008 14:52:15 +0100
> >>>>> Hannah Schroeter <hannah@schlund.de> said:
>
> >@@ -615,7 +619,7 @@ SPF_dns_resolv_new(SPF_dns_server_t *lay
> > #if HAVE_DECL_RES_NINIT
> > pthread_once(&res_state_control, SPF_dns_resolv_init_key);
> > #else
> >- if ( res_init() != 0 ) {
> >+ if ((_res.options & RES_INIT) == 0 && res_init() != 0) {
> > perror("res_init");
> > return NULL;
> > }
>
> hannah> Why is that needed, and how portable is that in fact, compared to
> hannah> calling res_init unconditionally in that place?
>
> Because, the libspf2 is not only consumer of the resolver. The
> res_init() might be called already before calling it from libspf2. It
> is enough to call res_init() once.
> Basically, res_init() is used in this manner on BSDs. The impact of
> calling res_init() again and again is unclear.
> I'm not sure about portability, but the _res and RES_INIT is described
> in resolver(8) on at least BSDs and CentOS.

I do not believe we are allowed to look inside _res. I have certainly
included the patch for res_ndestroy, but I believe the library authors
have to make calling res_init() multiple times safe. Perhaps you could
check this in your system source code? I have checked two
implementations, including the one at
http://www.koders.com/c/fid6F05A2EABB6A6CFB02D691F0F98DFB676F0B15E7.aspx?s=sort#L212 and both appear safe.

Please see 1.2.9. Further patches will make 1.2.10 in a week or two.

S.



-------------------------------------------
Sender Policy Framework: http://www.openspf.org
Modify Your Subscription: http://www.listbox.com/member/
Archives: https://www.listbox.com/member/archive/1007/=now
RSS Feed: https://www.listbox.com/member/archive/rss/1007/
Powered by Listbox: http://www.listbox.com