Backporting CVE-2016-10739

Backporting CVE-2016-10739

[Aurelien Jarno]

Hi,

I am looking at backporting fixes for CVE-2016-10739 (ie commit
108bc404) in the 2.28 branch first, and probably in the 2.24 branch
later. I would need some guidance how to proceed:

- Is it acceptable to also to backport commit 5e30b8ef ("resolv: 
  Reformat inet_addr, inet_aton to GNU style")? Without this patch,
  there's a lot of conflicts that are a pain to fix.

- According to the commit message 6ca53a24 ("resolv: Do not send queries
  for non-host-names in nss_dns [BZ #24112]"), also needs to be
  backported. Is it fine to do so?

- The commit introduces a new symbol, which is something we usually do
  not want in a stable branch. However the __inet_aton_exact symbol is
  added under GLIBC_PRIVATE. Therefore I wonder if it is acceptable for
  a stable branch.

Thanks,
Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: Backporting CVE-2016-10739

[Florian Weimer]

* Aurelien Jarno:

> I am looking at backporting fixes for CVE-2016-10739 (ie commit
> 108bc404) in the 2.28 branch first, and probably in the 2.24 branch
> later. I would need some guidance how to proceed:

I planned to do it last week, but didn't finish it.

> - Is it acceptable to also to backport commit 5e30b8ef ("resolv: 
>   Reformat inet_addr, inet_aton to GNU style")? Without this patch,
>   there's a lot of conflicts that are a pain to fix.

Yes absolutely.

> - According to the commit message 6ca53a24 ("resolv: Do not send queries
>   for non-host-names in nss_dns [BZ #24112]"), also needs to be
>   backported. Is it fine to do so?

Yes, the queries are pointless.

> - The commit introduces a new symbol, which is something we usually do
>   not want in a stable branch. However the __inet_aton_exact symbol is
>   added under GLIBC_PRIVATE. Therefore I wonder if it is acceptable for
>   a stable branch.

I planned to commit *another* commit which avoids the addition of the
GLIBC_PRIVATE symbol, with some code duplication.  Basically, use
attribute_hidden instead of libc_hidden_proto for the declaration of
__inet_aton_exact, and also build resolv/inet_addr.c for nscd.  We do
not always do that, but it seems easy enough to do it here, and it makes
our QE people happy.

If you want to do it, I can help you with that.

Thanks,
Florian

Re: Backporting CVE-2016-10739

[Aurelien Jarno]

On 2019-02-04 14:56, Florian Weimer wrote:
> * Aurelien Jarno:
> 
> > I am looking at backporting fixes for CVE-2016-10739 (ie commit
> > 108bc404) in the 2.28 branch first, and probably in the 2.24 branch
> > later. I would need some guidance how to proceed:
> 
> I planned to do it last week, but didn't finish it.

[...]
 
> If you want to do it, I can help you with that.

If you are already working on it, I'll just wait, there is no need to
duplicate the work.

Thanks,
Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: Backporting CVE-2016-10739

[Florian Weimer]

* Aurelien Jarno:

> On 2019-02-04 14:56, Florian Weimer wrote:
>> * Aurelien Jarno:
>> 
>> > I am looking at backporting fixes for CVE-2016-10739 (ie commit
>> > 108bc404) in the 2.28 branch first, and probably in the 2.24 branch
>> > later. I would need some guidance how to proceed:
>> 
>> I planned to do it last week, but didn't finish it.
>
> [...]
>  
>> If you want to do it, I can help you with that.
>
> If you are already working on it, I'll just wait, there is no need to
> duplicate the work.

Let me handle the backport the glibc 2.28, and you can take it from
there?

Thanks,
Florian

Re: Backporting CVE-2016-10739

[Aurelien Jarno]

On 2019-02-04 15:24, Florian Weimer wrote:
> * Aurelien Jarno:
> 
> > On 2019-02-04 14:56, Florian Weimer wrote:
> >> * Aurelien Jarno:
> >> 
> >> > I am looking at backporting fixes for CVE-2016-10739 (ie commit
> >> > 108bc404) in the 2.28 branch first, and probably in the 2.24 branch
> >> > later. I would need some guidance how to proceed:
> >> 
> >> I planned to do it last week, but didn't finish it.
> >
> > [...]
> >  
> >> If you want to do it, I can help you with that.
> >
> > If you are already working on it, I'll just wait, there is no need to
> > duplicate the work.
> 
> Let me handle the backport the glibc 2.28, and you can take it from
> there?

Works for me. Thanks.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net

Re: Backporting CVE-2016-10739

[Florian Weimer]

Here is the patch I'm testing to restore ABI after the regular
backports.  This looks fairly reasonable to me as far as such things
go.

I've put all patches on the fw/bug20018-backport branch on sourceware as
well.

Thanks,
Florian

Restore GLIBC_PRIVATE ABI after CVE-2016-10739 fix [BZ #20018]

This commit avoids adding the __inet_aton_exact@GLIBC_PRIVATE
symbol.  In master, the separately-compiled getaddrinfo
implementation in nscd needs it, however such an internal ABI change
is not desirable on a release branch if it can be avoided easily.

2019-02-04  Florian Weimer  <fweimer@redhat.com>

	[BZ #20018]
	Restore GLIBC_PRIVATE ABI after CVE-2016-10739 fix.
	* include/arpa/inet.h (__inet_aton_exact): Declare as hidden.
	* resolv/inet_addr.c (__inet_aton_exact): Remove libc_hidden_def.
	* resolv/Versions (GLIBC_PRIVATE): Do not export
	__inet_aton_exact.
	* nscd/nscd-inet_addr.c: New file.  Build resolv/inet_addr.c for
	nscd, without public symbols.
	* nscd/Makefile (nscd-modules): Add it.

diff --git a/include/arpa/inet.h b/include/arpa/inet.h
index 19aec74275..dce60b4909 100644
--- a/include/arpa/inet.h
+++ b/include/arpa/inet.h
@@ -2,8 +2,8 @@
 
 #ifndef _ISOMAC
 /* Variant of inet_aton which rejects trailing garbage.  */
-extern int __inet_aton_exact (const char *__cp, struct in_addr *__inp);
-libc_hidden_proto (__inet_aton_exact)
+extern int __inet_aton_exact (const char *__cp, struct in_addr *__inp)
+  attribute_hidden;
 
 libc_hidden_proto (inet_ntop)
 libc_hidden_proto (inet_pton)
diff --git a/nscd/Makefile b/nscd/Makefile
index b713a84c49..eb23c01a39 100644
--- a/nscd/Makefile
+++ b/nscd/Makefile
@@ -36,7 +36,7 @@ nscd-modules := nscd connections pwdcache getpwnam_r getpwuid_r grpcache \
 		getsrvbynm_r getsrvbypt_r servicescache \
 		dbg_log nscd_conf nscd_stat cache mem nscd_setup_thread \
 		xmalloc xstrdup aicache initgrcache gai res_hconf \
-		netgroupcache
+		netgroupcache nscd-inet_addr
 
 ifeq ($(build-nscd)$(have-thread-library),yesyes)
 
diff --git a/nscd/nscd-inet_addr.c b/nscd/nscd-inet_addr.c
new file mode 100644
index 0000000000..cfa4ac7462
--- /dev/null
+++ b/nscd/nscd-inet_addr.c
@@ -0,0 +1,24 @@
+/* Legacy IPv4 text-to-address functions.  Version for nscd.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* Do not provide definitions of the public symbols exported from
+   libc.  */
+#undef weak_alias
+#define weak_alias(from, to)
+
+#include <resolv/inet_addr.c>
diff --git a/resolv/Versions b/resolv/Versions
index 9a82704af7..b05778d965 100644
--- a/resolv/Versions
+++ b/resolv/Versions
@@ -27,7 +27,6 @@ libc {
     __h_errno; __resp;
 
     __res_iclose;
-    __inet_aton_exact;
     __inet_pton_length;
     __resolv_context_get;
     __resolv_context_get_preinit;
diff --git a/resolv/inet_addr.c b/resolv/inet_addr.c
index 41b6166a5b..1bc4a2c4d6 100644
--- a/resolv/inet_addr.c
+++ b/resolv/inet_addr.c
@@ -192,7 +192,6 @@ __inet_aton_exact (const char *cp, struct in_addr *addr)
   else
     return 0;
 }
-libc_hidden_def (__inet_aton_exact)
 
 /* inet_aton ignores trailing garbage.  */
 int

Re: Backporting CVE-2016-10739

[Carlos O'Donell]

On 2/4/19 9:53 AM, Florian Weimer wrote:
> Here is the patch I'm testing to restore ABI after the regular
> backports.  This looks fairly reasonable to me as far as such things
> go.
> 
> I've put all patches on the fw/bug20018-backport branch on sourceware as
> well.
> 
> Thanks,
> Florian
> 
> Restore GLIBC_PRIVATE ABI after CVE-2016-10739 fix [BZ #20018]

Shouldn't this say "Remove GLIBC_PRIVATE ABI" ?

I admit this confused me for a while since the patch clearly removes
the ABI from resolv/Versions.

> This commit avoids adding the __inet_aton_exact@GLIBC_PRIVATE
> symbol.  In master, the separately-compiled getaddrinfo
> implementation in nscd needs it, however such an internal ABI change
> is not desirable on a release branch if it can be avoided easily.
> 
> 2019-02-04  Florian Weimer  <fweimer@redhat.com>
> 
> 	[BZ #20018]
> 	Restore GLIBC_PRIVATE ABI after CVE-2016-10739 fix.

s/Restore/Remove/g

> 	* include/arpa/inet.h (__inet_aton_exact): Declare as hidden.
> 	* resolv/inet_addr.c (__inet_aton_exact): Remove libc_hidden_def.
> 	* resolv/Versions (GLIBC_PRIVATE): Do not export
> 	__inet_aton_exact.
> 	* nscd/nscd-inet_addr.c: New file.  Build resolv/inet_addr.c for
> 	nscd, without public symbols.
> 	* nscd/Makefile (nscd-modules): Add it.
> 
> diff --git a/include/arpa/inet.h b/include/arpa/inet.h
> index 19aec74275..dce60b4909 100644
> --- a/include/arpa/inet.h
> +++ b/include/arpa/inet.h
> @@ -2,8 +2,8 @@
>  
>  #ifndef _ISOMAC
>  /* Variant of inet_aton which rejects trailing garbage.  */
> -extern int __inet_aton_exact (const char *__cp, struct in_addr *__inp);
> -libc_hidden_proto (__inet_aton_exact)
> +extern int __inet_aton_exact (const char *__cp, struct in_addr *__inp)
> +  attribute_hidden;

Why do you mark this attribute_hidden?

You should remove this from the public header, and move it to a private
header that we already have.

Again, this might be a balance question, between too many changes in the
release branch with respect to master.

Is your goal here to minimize the changes if we need future backports?

I'm OK with this, but please add a comment here saying this. We should
use comments to help reviewers and mergers undrestand why this is different.

e.g.

/* CVE-2016-10739: Hide this interface on the stable release branch.  */

>  
>  libc_hidden_proto (inet_ntop)
>  libc_hidden_proto (inet_pton)
> diff --git a/nscd/Makefile b/nscd/Makefile
> index b713a84c49..eb23c01a39 100644
> --- a/nscd/Makefile
> +++ b/nscd/Makefile
> @@ -36,7 +36,7 @@ nscd-modules := nscd connections pwdcache getpwnam_r getpwuid_r grpcache \
>  		getsrvbynm_r getsrvbypt_r servicescache \
>  		dbg_log nscd_conf nscd_stat cache mem nscd_setup_thread \
>  		xmalloc xstrdup aicache initgrcache gai res_hconf \
> -		netgroupcache
> +		netgroupcache nscd-inet_addr

OK, add a new object file to the nscd build.

>  
>  ifeq ($(build-nscd)$(have-thread-library),yesyes)
>  
> diff --git a/nscd/nscd-inet_addr.c b/nscd/nscd-inet_addr.c
> new file mode 100644
> index 0000000000..cfa4ac7462
> --- /dev/null
> +++ b/nscd/nscd-inet_addr.c
> @@ -0,0 +1,24 @@
> +/* Legacy IPv4 text-to-address functions.  Version for nscd.
> +   Copyright (C) 2019 Free Software Foundation, Inc.

OK.

> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +/* Do not provide definitions of the public symbols exported from
> +   libc.  */
> +#undef weak_alias
> +#define weak_alias(from, to)

Why do we have to play with the weak aliases?
Because they create visible symbols at the new name?

> +
> +#include <resolv/inet_addr.c>

OK, recompile inet_addr* as nscd-inet_addr*

> diff --git a/resolv/Versions b/resolv/Versions
> index 9a82704af7..b05778d965 100644
> --- a/resolv/Versions
> +++ b/resolv/Versions
> @@ -27,7 +27,6 @@ libc {
>      __h_errno; __resp;
>  
>      __res_iclose;
> -    __inet_aton_exact;

OK, remove from libc's GLIBC_PRIVATE ABI.

>      __inet_pton_length;
>      __resolv_context_get;
>      __resolv_context_get_preinit;
> diff --git a/resolv/inet_addr.c b/resolv/inet_addr.c
> index 41b6166a5b..1bc4a2c4d6 100644
> --- a/resolv/inet_addr.c
> +++ b/resolv/inet_addr.c
> @@ -192,7 +192,6 @@ __inet_aton_exact (const char *cp, struct in_addr *addr)
>    else
>      return 0;
>  }
> -libc_hidden_def (__inet_aton_exact)

Why remove this? Why not redefine it like you did for weak alias?

If we are going to modify inet_addr.c, we can modify it
such that we avoid hacking weak_alias *or* hack both?

Like -DNSCD_INET_ADDR=0 from the Makefile for the normal build
and then #define NSCD_INET_ADDR 1 in the nscd-inet_addr.c sources?

Something cleaner that makes it obvious there is an interface
here between nscd/libc at the source level and that we are employing
source-level reuse. I know it makes the change a bit bigger and that
we need to balance a minimal change on a release branch, but it seems
like we could be a little bit more explicit with our changes.

Likewise if we're going to remove things, I think there is value
in comments explaining why this is missing. That way if you get
a merge failure you have a pointer to the reason, rather than nothing.
It documents intent.

>  
>  /* inet_aton ignores trailing garbage.  */
>  int
> 


-- 
Cheers,
Carlos.

Re: Backporting CVE-2016-10739

[Florian Weimer]

That didn't work due to linking failures in the test suite.

Here's something less ambitious.  It retains the ABI for testing, but
patches nscd to use its own private copy.

(I have not put this on the branch due to Bugzilla spam this would
cause.)

Thanks,
Florian

nscd: Do not use __inet_aton_exact@GLIBC_PRIVATE [BZ #20018]

This commit avoids referencing the __inet_aton_exact@GLIBC_PRIVATE
symbol from nscd.  In master, the separately-compiled getaddrinfo
implementation in nscd needs it, however such an internal ABI change
is not desirable on a release branch if it can be avoided easily.

2019-02-04  Florian Weimer  <fweimer@redhat.com>

	[BZ #20018]
	nscd: Do not rely on new GLIBC_PRIVATE ABI after CVE-2016-10739 fix.
	* nscd/nscd-inet_addr.c: New file.  Build resolv/inet_addr.c for
	nscd, without public symbols.
	* nscd/Makefile (nscd-modules): Add it.

diff --git a/nscd/Makefile b/nscd/Makefile
index b713a84c49..eb23c01a39 100644
--- a/nscd/Makefile
+++ b/nscd/Makefile
@@ -36,7 +36,7 @@ nscd-modules := nscd connections pwdcache getpwnam_r getpwuid_r grpcache \
 		getsrvbynm_r getsrvbypt_r servicescache \
 		dbg_log nscd_conf nscd_stat cache mem nscd_setup_thread \
 		xmalloc xstrdup aicache initgrcache gai res_hconf \
-		netgroupcache
+		netgroupcache nscd-inet_addr
 
 ifeq ($(build-nscd)$(have-thread-library),yesyes)
 
diff --git a/nscd/nscd-inet_addr.c b/nscd/nscd-inet_addr.c
new file mode 100644
index 0000000000..ce42ba3ea8
--- /dev/null
+++ b/nscd/nscd-inet_addr.c
@@ -0,0 +1,30 @@
+/* Legacy IPv4 text-to-address functions.  Version for nscd.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <arpa/inet.h>
+
+/* Declare __inet_aton_exact as hidden, so that it does not get
+   exported from nscd.  */
+__typeof__ (__inet_aton_exact) __inet_aton_exact attribute_hidden;
+
+/* Do not provide definitions of the public symbols exported from
+   libc.  */
+#undef weak_alias
+#define weak_alias(from, to)
+
+#include <resolv/inet_addr.c>

Re: Backporting CVE-2016-10739

[Carlos O'Donell]

On 2/4/19 10:26 AM, Florian Weimer wrote:
> That didn't work due to linking failures in the test suite.
> 
> Here's something less ambitious.  It retains the ABI for testing, but
> patches nscd to use its own private copy.
> 
> (I have not put this on the branch due to Bugzilla spam this would
> cause.)

This was a non-causal v2 of your first patch.

It arrived while I was reviewing the first one, and I like this one
much more.

> Thanks,
> Florian
> 
> nscd: Do not use __inet_aton_exact@GLIBC_PRIVATE [BZ #20018]

OK.

> 
> This commit avoids referencing the __inet_aton_exact@GLIBC_PRIVATE
> symbol from nscd.  In master, the separately-compiled getaddrinfo
> implementation in nscd needs it, however such an internal ABI change
> is not desirable on a release branch if it can be avoided easily.

OK. Good note.

> 2019-02-04  Florian Weimer  <fweimer@redhat.com>
> 
> 	[BZ #20018]
> 	nscd: Do not rely on new GLIBC_PRIVATE ABI after CVE-2016-10739 fix.
> 	* nscd/nscd-inet_addr.c: New file.  Build resolv/inet_addr.c for
> 	nscd, without public symbols.
> 	* nscd/Makefile (nscd-modules): Add it.
> 
> diff --git a/nscd/Makefile b/nscd/Makefile
> index b713a84c49..eb23c01a39 100644
> --- a/nscd/Makefile
> +++ b/nscd/Makefile
> @@ -36,7 +36,7 @@ nscd-modules := nscd connections pwdcache getpwnam_r getpwuid_r grpcache \
>  		getsrvbynm_r getsrvbypt_r servicescache \
>  		dbg_log nscd_conf nscd_stat cache mem nscd_setup_thread \
>  		xmalloc xstrdup aicache initgrcache gai res_hconf \
> -		netgroupcache
> +		netgroupcache nscd-inet_addr

OK. Add a new object to nscd.

>  
>  ifeq ($(build-nscd)$(have-thread-library),yesyes)
>  
> diff --git a/nscd/nscd-inet_addr.c b/nscd/nscd-inet_addr.c
> new file mode 100644
> index 0000000000..ce42ba3ea8
> --- /dev/null
> +++ b/nscd/nscd-inet_addr.c
> @@ -0,0 +1,30 @@
> +/* Legacy IPv4 text-to-address functions.  Version for nscd.
> +   Copyright (C) 2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <arpa/inet.h>
> +

Please add a comment explaining why this is here.


> +/* Declare __inet_aton_exact as hidden, so that it does not get
> +   exported from nscd.  */
> +__typeof__ (__inet_aton_exact) __inet_aton_exact attribute_hidden;
> +
> +/* Do not provide definitions of the public symbols exported from
> +   libc.  */
> +#undef weak_alias
> +#define weak_alias(from, to)
> +
> +#include <resolv/inet_addr.c>
> 

Can we kill the prototype from the public header and use an internal
header? It seems messy to leave that prototype for the GLIBC_PRIVATE
symbol in the public header. It might tempt people to workaround the
linkage protection.

-- 
Cheers,
Carlos.

Re: Backporting CVE-2016-10739

[Florian Weimer]

* Carlos O'Donell:

>> +#include <arpa/inet.h>
>> +
>
> Please add a comment explaining why this is here.

You mean like this?

/* Obtain the prototype for __inet_aton_exact.  */

>> +/* Declare __inet_aton_exact as hidden, so that it does not get
>> +   exported from nscd.  */
>> +__typeof__ (__inet_aton_exact) __inet_aton_exact attribute_hidden;
>> +
>> +/* Do not provide definitions of the public symbols exported from
>> +   libc.  */
>> +#undef weak_alias
>> +#define weak_alias(from, to)
>> +
>> +#include <resolv/inet_addr.c>
>
> Can we kill the prototype from the public header and use an internal
> header? It seems messy to leave that prototype for the GLIBC_PRIVATE
> symbol in the public header. It might tempt people to workaround the
> linkage protection.

I don't understand.  There is no public header.  We can't remove it from
the internal header because it would break the test suite build.

Thanks,
Florian

Re: Backporting CVE-2016-10739

[Carlos O'Donell]

On 2/4/19 10:44 AM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>>> +#include <arpa/inet.h>
>>> +
>>
>> Please add a comment explaining why this is here.
> 
> You mean like this?
> 
> /* Obtain the prototype for __inet_aton_exact.  */

It should reference the bug or CVE to document the intent
of the changes.

Post v3 and I'll sign off?

>>> +/* Declare __inet_aton_exact as hidden, so that it does not get
>>> +   exported from nscd.  */
>>> +__typeof__ (__inet_aton_exact) __inet_aton_exact attribute_hidden;
>>> +
>>> +/* Do not provide definitions of the public symbols exported from
>>> +   libc.  */
>>> +#undef weak_alias
>>> +#define weak_alias(from, to)
>>> +
>>> +#include <resolv/inet_addr.c>
>>
>> Can we kill the prototype from the public header and use an internal
>> header? It seems messy to leave that prototype for the GLIBC_PRIVATE
>> symbol in the public header. It might tempt people to workaround the
>> linkage protection.
> 
> I don't understand.  There is no public header.  We can't remove it from
> the internal header because it would break the test suite build.

Perfect, I thought I saw this in a public header. If it's not then we
don't have anything else to do. Your patch is the best solution.

-- 
Cheers,
Carlos.

Re: Backporting CVE-2016-10739

[Florian Weimer]

* Carlos O'Donell:

> On 2/4/19 10:44 AM, Florian Weimer wrote:
>> * Carlos O'Donell:
>> 
>>>> +#include <arpa/inet.h>
>>>> +
>>>
>>> Please add a comment explaining why this is here.
>> 
>> You mean like this?
>> 
>> /* Obtain the prototype for __inet_aton_exact.  */
>
> It should reference the bug or CVE to document the intent
> of the changes.
>
> Post v3 and I'll sign off?

This approach does not actually work because copying a prototype this
way and adding a hidden visibility attribute does not actually make the
symbol hidden.  The patch below however has the desired effect, mainly
because interposition no longer happens and the __inet_aton_exact_hidden
function is not added to the dynamic symbol table of the nscd
executable.

I suspect I would have had to use __attribute__ ((visibility
("hidden"))) directly because we define attribute_hidden thusly:

#if defined SHARED || defined LIBC_NONSHARED \
  || (BUILD_PIE_DEFAULT && IS_IN (libc))
# define attribute_hidden __attribute__ ((visibility ("hidden")))
#else
# define attribute_hidden
#endif

I can post yet another patch which uses real hidden visibility and
avoids the symbol redirect.

Thanks,
Florian

This commit avoids referencing the __inet_aton_exact@GLIBC_PRIVATE
symbol from nscd.  In master, the separately-compiled getaddrinfo
implementation in nscd needs it, however such an internal ABI change
is not desirable on a release branch if it can be avoided.

2019-02-04  Florian Weimer  <fweimer@redhat.com>

	[BZ #20018]
	nscd: Do not rely on new GLIBC_PRIVATE ABI after CVE-2016-10739 fix.
	* nscd/nscd-inet_addr.c: New file.  Build resolv/inet_addr.c for
	nscd, without public symbols.
	* nscd/Makefile (nscd-modules): Add it.
	* nscd/gai.c: Include <arpa/inet.h> and redirect __inet_aton_exact
	to __inet_aton_exact_hidden.

diff --git a/nscd/Makefile b/nscd/Makefile
index b713a84c49..eb23c01a39 100644
--- a/nscd/Makefile
+++ b/nscd/Makefile
@@ -36,7 +36,7 @@ nscd-modules := nscd connections pwdcache getpwnam_r getpwuid_r grpcache \
 		getsrvbynm_r getsrvbypt_r servicescache \
 		dbg_log nscd_conf nscd_stat cache mem nscd_setup_thread \
 		xmalloc xstrdup aicache initgrcache gai res_hconf \
-		netgroupcache
+		netgroupcache nscd-inet_addr
 
 ifeq ($(build-nscd)$(have-thread-library),yesyes)
 
diff --git a/nscd/gai.c b/nscd/gai.c
index f57f396f57..36cc6ce4c3 100644
--- a/nscd/gai.c
+++ b/nscd/gai.c
@@ -33,6 +33,13 @@
 #define __getifaddrs getifaddrs
 #define __freeifaddrs freeifaddrs
 
+/* We do not want to export __inet_aton_exact.  Get the prototype and
+   change it to hidden visibility, and redirect to the hidden
+   definition.  */
+#include <arpa/inet.h>
+__typeof__ (__inet_aton_exact) __inet_aton_exact_hidden attribute_hidden;
+#define __inet_aton_exact __inet_aton_exact_hidden
+
 /* We are nscd, so we don't want to be talking to ourselves.  */
 #undef  USE_NSCD
 
diff --git a/nscd/nscd-inet_addr.c b/nscd/nscd-inet_addr.c
new file mode 100644
index 0000000000..901fc620b3
--- /dev/null
+++ b/nscd/nscd-inet_addr.c
@@ -0,0 +1,33 @@
+/* Legacy IPv4 text-to-address functions.  Version for nscd.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <arpa/inet.h>
+
+/* We do not want to export __inet_aton_exact.  Get the prototype and
+   change it to hidden visibility, and arrange for a definition under
+   a different name.  */
+#include <arpa/inet.h>
+__typeof__ (__inet_aton_exact) __inet_aton_exact_hidden attribute_hidden;
+#define __inet_aton_exact __inet_aton_exact_hidden
+
+/* Do not provide definitions of the public symbols exported from
+   libc.  */
+#undef weak_alias
+#define weak_alias(from, to)
+
+#include <resolv/inet_addr.c>

Re: Backporting CVE-2016-10739

[Carlos O'Donell]

On 2/4/19 11:18 AM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> On 2/4/19 10:44 AM, Florian Weimer wrote:
>>> * Carlos O'Donell:
>>>
>>>>> +#include <arpa/inet.h>
>>>>> +
>>>>
>>>> Please add a comment explaining why this is here.
>>>
>>> You mean like this?
>>>
>>> /* Obtain the prototype for __inet_aton_exact.  */
>>
>> It should reference the bug or CVE to document the intent
>> of the changes.
>>
>> Post v3 and I'll sign off?
> 
> This approach does not actually work because copying a prototype this
> way and adding a hidden visibility attribute does not actually make the
> symbol hidden.  The patch below however has the desired effect, mainly
> because interposition no longer happens and the __inet_aton_exact_hidden
> function is not added to the dynamic symbol table of the nscd
> executable.
> 
> I suspect I would have had to use __attribute__ ((visibility
> ("hidden"))) directly because we define attribute_hidden thusly:
> 
> #if defined SHARED || defined LIBC_NONSHARED \
>   || (BUILD_PIE_DEFAULT && IS_IN (libc))
> # define attribute_hidden __attribute__ ((visibility ("hidden")))
> #else
> # define attribute_hidden
> #endif
> 
> I can post yet another patch which uses real hidden visibility and
> avoids the symbol redirect.

This version is a little hack-ish, but I don't object to it. It does
the job.

It is certainly a minimal set of changes, and that makes it quite
useful for the release branch.

I don't think you need to go any further unless you think the version
using real hidden visibility is all that much better?


> This commit avoids referencing the __inet_aton_exact@GLIBC_PRIVATE
> symbol from nscd.  In master, the separately-compiled getaddrinfo
> implementation in nscd needs it, however such an internal ABI change
> is not desirable on a release branch if it can be avoided.
> 
> 2019-02-04  Florian Weimer  <fweimer@redhat.com>
> 
> 	[BZ #20018]
> 	nscd: Do not rely on new GLIBC_PRIVATE ABI after CVE-2016-10739 fix.
> 	* nscd/nscd-inet_addr.c: New file.  Build resolv/inet_addr.c for
> 	nscd, without public symbols.
> 	* nscd/Makefile (nscd-modules): Add it.
> 	* nscd/gai.c: Include <arpa/inet.h> and redirect __inet_aton_exact
> 	to __inet_aton_exact_hidden.
> 
> diff --git a/nscd/Makefile b/nscd/Makefile
> index b713a84c49..eb23c01a39 100644
> --- a/nscd/Makefile
> +++ b/nscd/Makefile
> @@ -36,7 +36,7 @@ nscd-modules := nscd connections pwdcache getpwnam_r getpwuid_r grpcache \
>  		getsrvbynm_r getsrvbypt_r servicescache \
>  		dbg_log nscd_conf nscd_stat cache mem nscd_setup_thread \
>  		xmalloc xstrdup aicache initgrcache gai res_hconf \
> -		netgroupcache
> +		netgroupcache nscd-inet_addr
>  
>  ifeq ($(build-nscd)$(have-thread-library),yesyes)
>  
> diff --git a/nscd/gai.c b/nscd/gai.c
> index f57f396f57..36cc6ce4c3 100644
> --- a/nscd/gai.c
> +++ b/nscd/gai.c
> @@ -33,6 +33,13 @@
>  #define __getifaddrs getifaddrs
>  #define __freeifaddrs freeifaddrs
>  
> +/* We do not want to export __inet_aton_exact.  Get the prototype and
> +   change it to hidden visibility, and redirect to the hidden
> +   definition.  */
> +#include <arpa/inet.h>
> +__typeof__ (__inet_aton_exact) __inet_aton_exact_hidden attribute_hidden;
> +#define __inet_aton_exact __inet_aton_exact_hidden
> +
>  /* We are nscd, so we don't want to be talking to ourselves.  */
>  #undef  USE_NSCD
>  
> diff --git a/nscd/nscd-inet_addr.c b/nscd/nscd-inet_addr.c
> new file mode 100644
> index 0000000000..901fc620b3
> --- /dev/null
> +++ b/nscd/nscd-inet_addr.c
> @@ -0,0 +1,33 @@
> +/* Legacy IPv4 text-to-address functions.  Version for nscd.
> +   Copyright (C) 2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <arpa/inet.h>
> +
> +/* We do not want to export __inet_aton_exact.  Get the prototype and
> +   change it to hidden visibility, and arrange for a definition under
> +   a different name.  */
> +#include <arpa/inet.h>
> +__typeof__ (__inet_aton_exact) __inet_aton_exact_hidden attribute_hidden;
> +#define __inet_aton_exact __inet_aton_exact_hidden
> +
> +/* Do not provide definitions of the public symbols exported from
> +   libc.  */
> +#undef weak_alias
> +#define weak_alias(from, to)
> +
> +#include <resolv/inet_addr.c>
> 


-- 
Cheers,
Carlos.

Re: Backporting CVE-2016-10739

[Florian Weimer]

* Carlos O'Donell:

> On 2/4/19 11:18 AM, Florian Weimer wrote:
>> * Carlos O'Donell:
>> 
>>> On 2/4/19 10:44 AM, Florian Weimer wrote:
>>>> * Carlos O'Donell:
>>>>
>>>>>> +#include <arpa/inet.h>
>>>>>> +
>>>>>
>>>>> Please add a comment explaining why this is here.
>>>>
>>>> You mean like this?
>>>>
>>>> /* Obtain the prototype for __inet_aton_exact.  */
>>>
>>> It should reference the bug or CVE to document the intent
>>> of the changes.
>>>
>>> Post v3 and I'll sign off?
>> 
>> This approach does not actually work because copying a prototype this
>> way and adding a hidden visibility attribute does not actually make the
>> symbol hidden.  The patch below however has the desired effect, mainly
>> because interposition no longer happens and the __inet_aton_exact_hidden
>> function is not added to the dynamic symbol table of the nscd
>> executable.
>> 
>> I suspect I would have had to use __attribute__ ((visibility
>> ("hidden"))) directly because we define attribute_hidden thusly:
>> 
>> #if defined SHARED || defined LIBC_NONSHARED \
>>   || (BUILD_PIE_DEFAULT && IS_IN (libc))
>> # define attribute_hidden __attribute__ ((visibility ("hidden")))
>> #else
>> # define attribute_hidden
>> #endif
>> 
>> I can post yet another patch which uses real hidden visibility and
>> avoids the symbol redirect.
>
> This version is a little hack-ish, but I don't object to it. It does
> the job.
>
> It is certainly a minimal set of changes, and that makes it quite
> useful for the release branch.
>
> I don't think you need to go any further unless you think the version
> using real hidden visibility is all that much better?

Well, at least it does not have the completely ineffective
attribute_hidden. 8-)

Patch below.  What do you think?

This one gets the symbol visibility correct.

$ eu-readelf -s ../build/nscd/nscd-inet_addr.o | grep inet_aton_exact
   22: 0000000000000150     56 FUNC    GLOBAL HIDDEN         1 __inet_aton_exact
$ eu-readelf -s ../build/nscd/gai.o | grep inet_aton_exact
   99: 0000000000000000      0 NOTYPE  GLOBAL HIDDEN     UNDEF __inet_aton_exact

And there's no __inet_aton_exact symbol in nscd.  I think this is fairly
standard usage of a hidden symbol, so renaming the symbol should not be
necessary.

Thanks,
Florian

nscd: Do not use __inet_aton_exact@GLIBC_PRIVATE [BZ #20018]

This commit avoids referencing the __inet_aton_exact@GLIBC_PRIVATE
symbol from nscd.  In master, the separately-compiled getaddrinfo
implementation in nscd needs it, however such an internal ABI change
is not desirable on a release branch if it can be avoided.

2019-02-04  Florian Weimer  <fweimer@redhat.com>

	[BZ #20018]
	nscd: Do not rely on new GLIBC_PRIVATE ABI after CVE-2016-10739 fix.
	* nscd/nscd-inet_addr.c: New file.  Build resolv/inet_addr.c for
	nscd, without public symbols.
	* nscd/Makefile (nscd-modules): Add it.
	* nscd/gai.c: Include <arpa/inet.h> and change visibility of
	__inet_aton_exact.

diff --git a/nscd/Makefile b/nscd/Makefile
index b713a84c49..eb23c01a39 100644
--- a/nscd/Makefile
+++ b/nscd/Makefile
@@ -36,7 +36,7 @@ nscd-modules := nscd connections pwdcache getpwnam_r getpwuid_r grpcache \
 		getsrvbynm_r getsrvbypt_r servicescache \
 		dbg_log nscd_conf nscd_stat cache mem nscd_setup_thread \
 		xmalloc xstrdup aicache initgrcache gai res_hconf \
-		netgroupcache
+		netgroupcache nscd-inet_addr
 
 ifeq ($(build-nscd)$(have-thread-library),yesyes)
 
diff --git a/nscd/gai.c b/nscd/gai.c
index f57f396f57..68a4abd30e 100644
--- a/nscd/gai.c
+++ b/nscd/gai.c
@@ -33,6 +33,12 @@
 #define __getifaddrs getifaddrs
 #define __freeifaddrs freeifaddrs
 
+/* We do not want to export __inet_aton_exact.  Get the prototype and
+   change its visibility to hidden.  */
+#include <arpa/inet.h>
+__typeof__ (__inet_aton_exact) __inet_aton_exact
+  __attribute__ ((visibility ("hidden")));
+
 /* We are nscd, so we don't want to be talking to ourselves.  */
 #undef  USE_NSCD
 
diff --git a/nscd/nscd-inet_addr.c b/nscd/nscd-inet_addr.c
new file mode 100644
index 0000000000..f366b9567d
--- /dev/null
+++ b/nscd/nscd-inet_addr.c
@@ -0,0 +1,32 @@
+/* Legacy IPv4 text-to-address functions.  Version for nscd.
+   Copyright (C) 2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <arpa/inet.h>
+
+/* We do not want to export __inet_aton_exact.  Get the prototype and
+   change the visibility to hidden.  */
+#include <arpa/inet.h>
+__typeof__ (__inet_aton_exact) __inet_aton_exact
+  __attribute__ ((visibility ("hidden")));
+
+/* Do not provide definitions of the public symbols exported from
+   libc.  */
+#undef weak_alias
+#define weak_alias(from, to)
+
+#include <resolv/inet_addr.c>

Re: Backporting CVE-2016-10739

[Carlos O'Donell]

On 2/4/19 2:25 PM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>> On 2/4/19 11:18 AM, Florian Weimer wrote:
>>> * Carlos O'Donell:
>>>
>>>> On 2/4/19 10:44 AM, Florian Weimer wrote:
>>>>> * Carlos O'Donell:
>>>>>
>>>>>>> +#include <arpa/inet.h>
>>>>>>> +
>>>>>>
>>>>>> Please add a comment explaining why this is here.
>>>>>
>>>>> You mean like this?
>>>>>
>>>>> /* Obtain the prototype for __inet_aton_exact.  */
>>>>
>>>> It should reference the bug or CVE to document the intent
>>>> of the changes.
>>>>
>>>> Post v3 and I'll sign off?
>>>
>>> This approach does not actually work because copying a prototype this
>>> way and adding a hidden visibility attribute does not actually make the
>>> symbol hidden.  The patch below however has the desired effect, mainly
>>> because interposition no longer happens and the __inet_aton_exact_hidden
>>> function is not added to the dynamic symbol table of the nscd
>>> executable.
>>>
>>> I suspect I would have had to use __attribute__ ((visibility
>>> ("hidden"))) directly because we define attribute_hidden thusly:
>>>
>>> #if defined SHARED || defined LIBC_NONSHARED \
>>>   || (BUILD_PIE_DEFAULT && IS_IN (libc))
>>> # define attribute_hidden __attribute__ ((visibility ("hidden")))
>>> #else
>>> # define attribute_hidden
>>> #endif
>>>
>>> I can post yet another patch which uses real hidden visibility and
>>> avoids the symbol redirect.
>>
>> This version is a little hack-ish, but I don't object to it. It does
>> the job.
>>
>> It is certainly a minimal set of changes, and that makes it quite
>> useful for the release branch.
>>
>> I don't think you need to go any further unless you think the version
>> using real hidden visibility is all that much better?
> 
> Well, at least it does not have the completely ineffective
> attribute_hidden. 8-)

I'm just providing high-level review here, I haven't tested these patches,
I rely on you for that :-}

> Patch below.  What do you think?

This looks good to me, you make direct use of "__attribute__ ((visibility ("hidden")))"
in an exceptional case, and that's fine.

If this becomes less rare for some reason we might want a libc-symbols.h macro to define
something that expresses the intent of the hidden visibility e.g. attr_dup_sym_hidden.

It's rare we have a fix that needs a new private interface, and that we can also
just dup the code to fix the problem. Most likely I expect we *can't* dup the code
and so need the new private interface to fix the bug, or we rework the fix entirely.

> This one gets the symbol visibility correct.

OK.

> $ eu-readelf -s ../build/nscd/nscd-inet_addr.o | grep inet_aton_exact
>    22: 0000000000000150     56 FUNC    GLOBAL HIDDEN         1 __inet_aton_exact
> $ eu-readelf -s ../build/nscd/gai.o | grep inet_aton_exact
>    99: 0000000000000000      0 NOTYPE  GLOBAL HIDDEN     UNDEF __inet_aton_exact

That's good.

> 
> And there's no __inet_aton_exact symbol in nscd.  I think this is fairly
> standard usage of a hidden symbol, so renaming the symbol should not be
> necessary.

Perfect, and interposition is also not possible.

> Thanks,
> Florian
> 
> nscd: Do not use __inet_aton_exact@GLIBC_PRIVATE [BZ #20018]
> 
> This commit avoids referencing the __inet_aton_exact@GLIBC_PRIVATE
> symbol from nscd.  In master, the separately-compiled getaddrinfo
> implementation in nscd needs it, however such an internal ABI change
> is not desirable on a release branch if it can be avoided.
> 
> 2019-02-04  Florian Weimer  <fweimer@redhat.com>
> 
> 	[BZ #20018]
> 	nscd: Do not rely on new GLIBC_PRIVATE ABI after CVE-2016-10739 fix.
> 	* nscd/nscd-inet_addr.c: New file.  Build resolv/inet_addr.c for
> 	nscd, without public symbols.
> 	* nscd/Makefile (nscd-modules): Add it.
> 	* nscd/gai.c: Include <arpa/inet.h> and change visibility of
> 	__inet_aton_exact.

OK for release/2.28/master.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> diff --git a/nscd/Makefile b/nscd/Makefile
> index b713a84c49..eb23c01a39 100644
> --- a/nscd/Makefile
> +++ b/nscd/Makefile
> @@ -36,7 +36,7 @@ nscd-modules := nscd connections pwdcache getpwnam_r getpwuid_r grpcache \
>  		getsrvbynm_r getsrvbypt_r servicescache \
>  		dbg_log nscd_conf nscd_stat cache mem nscd_setup_thread \
>  		xmalloc xstrdup aicache initgrcache gai res_hconf \
> -		netgroupcache
> +		netgroupcache nscd-inet_addr

OK.

>  
>  ifeq ($(build-nscd)$(have-thread-library),yesyes)
>  
> diff --git a/nscd/gai.c b/nscd/gai.c
> index f57f396f57..68a4abd30e 100644
> --- a/nscd/gai.c
> +++ b/nscd/gai.c
> @@ -33,6 +33,12 @@
>  #define __getifaddrs getifaddrs
>  #define __freeifaddrs freeifaddrs
>  
> +/* We do not want to export __inet_aton_exact.  Get the prototype and
> +   change its visibility to hidden.  */
> +#include <arpa/inet.h>
> +__typeof__ (__inet_aton_exact) __inet_aton_exact
> +  __attribute__ ((visibility ("hidden")));

OK.

> +
>  /* We are nscd, so we don't want to be talking to ourselves.  */
>  #undef  USE_NSCD
>  
> diff --git a/nscd/nscd-inet_addr.c b/nscd/nscd-inet_addr.c
> new file mode 100644
> index 0000000000..f366b9567d
> --- /dev/null
> +++ b/nscd/nscd-inet_addr.c
> @@ -0,0 +1,32 @@
> +/* Legacy IPv4 text-to-address functions.  Version for nscd.
> +   Copyright (C) 2019 Free Software Foundation, Inc.

OK.

> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <http://www.gnu.org/licenses/>.  */
> +
> +#include <arpa/inet.h>
> +
> +/* We do not want to export __inet_aton_exact.  Get the prototype and
> +   change the visibility to hidden.  */
> +#include <arpa/inet.h>
> +__typeof__ (__inet_aton_exact) __inet_aton_exact
> +  __attribute__ ((visibility ("hidden")));

OK.

> +
> +/* Do not provide definitions of the public symbols exported from
> +   libc.  */
> +#undef weak_alias
> +#define weak_alias(from, to)

OK.

> +
> +#include <resolv/inet_addr.c>
> 


-- 
Cheers,
Carlos.

Re: Backporting CVE-2016-10739

[Florian Weimer]

* Carlos O'Donell:

>> Patch below.  What do you think?
>
> This looks good to me, you make direct use of "__attribute__
> ((visibility ("hidden")))" in an exceptional case, and that's fine.

> If this becomes less rare for some reason we might want a
> libc-symbols.h macro to define something that expresses the intent of
> the hidden visibility e.g. attr_dup_sym_hidden.

I'm not a firm believer in those macros.  The fact that attribute_hidden
expanded to nothing at all was quite a surprise to me.

I've pushed the last version.

Aurelien, I will not be able to do any more backports before Wednesday,
so please feel free to take over.

Thanks,
Florian

Re: Backporting CVE-2016-10739

[Carlos O'Donell]

On 2/4/19 3:59 PM, Florian Weimer wrote:
> * Carlos O'Donell:
> 
>>> Patch below.  What do you think?
>>
>> This looks good to me, you make direct use of "__attribute__
>> ((visibility ("hidden")))" in an exceptional case, and that's fine.
> 
>> If this becomes less rare for some reason we might want a
>> libc-symbols.h macro to define something that expresses the intent of
>> the hidden visibility e.g. attr_dup_sym_hidden.
> 
> I'm not a firm believer in those macros.  The fact that attribute_hidden
> expanded to nothing at all was quite a surprise to me.

I care about documenting intent, since this is what keeps the code
maintainable and the interfaces working.

You don't have to use the macros, but they self-document without needing
to have a comment.

In your case the comment is what's important, that we don't want to
export the symbol.

> I've pushed the last version.

Thanks!

> Aurelien, I will not be able to do any more backports before Wednesday,
> so please feel free to take over.

-- 
Cheers,
Carlos.

Re: Backporting CVE-2016-10739

[Aurelien Jarno]

On 2019-02-04 21:59, Florian Weimer wrote:
> * Carlos O'Donell:
> 
> >> Patch below.  What do you think?
> >
> > This looks good to me, you make direct use of "__attribute__
> > ((visibility ("hidden")))" in an exceptional case, and that's fine.
> 
> > If this becomes less rare for some reason we might want a
> > libc-symbols.h macro to define something that expresses the intent of
> > the hidden visibility e.g. attr_dup_sym_hidden.
> 
> I'm not a firm believer in those macros.  The fact that attribute_hidden
> expanded to nothing at all was quite a surprise to me.
> 
> I've pushed the last version.
> 
> Aurelien, I will not be able to do any more backports before Wednesday,
> so please feel free to take over.

Thanks, I'll work on that for the 2.24 branch.

Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                 http://www.aurel32.net