diff mbox

Fix posix_spawn getrlimit64 namespace (bug 17991)

Message ID alpine.DEB.2.10.1502171743370.19294@digraph.polyomino.org.uk
State Committed
Headers

Commit Message

Joseph Myers Feb. 17, 2015, 5:44 p.m. UTC 
  posix_spawn (a standard POSIX function) brings in a use of getrlimit64
(not a standard POSIX function).  This patch fixes this by using
__getrlimit64 and making getrlimit64 a weak alias.

This is more complicated than some such changes because of files that
define getrlimit64 in their own way using symbol versioning after
including the main sysdeps/unix/sysv/linux/getrlimit64.c with a
getrlimit macro defined.  There are various existing patterns for such
cases in glibc; the one I've used here is that a getrlimit64 macro
disables the weak_alias / libc_hidden_weak calls, leaving it to the
including file to define the getrlimit64 name in whatever way is
appropriate.

Tested for x86_64 and x86 that installed stripped shared libraries are
unchanged by this patch.

2015-02-17  Joseph Myers  <joseph@codesourcery.com>

	[BZ #17991]
	* include/sys/resource.h (__getrlimit64): Declare.  Use
	libc_hidden_proto.
	* resource/getrlimit64.c (getrlimit64): Rename to __getrlimit64
	and define as weak alias of __getrlimit64.  Use libc_hidden_weak.
	* sysdeps/posix/spawni.c (__spawni): Call __getrlimit64 instead of
	getrlimit64.
	* sysdeps/unix/sysv/linux/getrlimit64.c (getrlimit64): Rename to
	__getrlimit64.
	[!getrlimit64] (getrlimit64): Define as weak alias of
	__getrlimit64.  Use libc_hidden_weak.
	* sysdeps/unix/sysv/linux/i386/getrlimit64.c (getrlimit64): Define
	using __getrlimit64 not __new_getrlimit64.
	(__GI_getrlimit64): Likewise.
	* sysdeps/unix/sysv/linux/mips/getrlimit64.c (getrlimit64):
	Likewise.
	(__GI_getrlimit64): Likewise.
	(__old_getrlimit64): Use __getrlimit64 not __new_getrlimit64.
	* sysdeps/unix/sysv/linux/powerpc/powerpc64/syscalls.list
	(getrlimit): Add __getrlimit64 alias.
	* sysdeps/unix/sysv/linux/wordsize-64/syscalls.list (getrlimit):
	Likewise.
	* conform/Makefile (test-xfail-XOPEN2K/spawn.h/linknamespace):
	Remove variable.
	(test-xfail-POSIX2008/spawn.h/linknamespace): Likewise.
	(test-xfail-XOPEN2K8/spawn.h/linknamespace): Likewise.

Comments

Rich Felker Feb. 17, 2015, 9:46 p.m. UTC | #1 
On Tue, Feb 17, 2015 at 05:44:55PM +0000, Joseph Myers wrote:
> posix_spawn (a standard POSIX function) brings in a use of getrlimit64
> (not a standard POSIX function).  This patch fixes this by using
> __getrlimit64 and making getrlimit64 a weak alias.
> 
> This is more complicated than some such changes because of files that
> define getrlimit64 in their own way using symbol versioning after
> including the main sysdeps/unix/sysv/linux/getrlimit64.c with a
> getrlimit macro defined.  There are various existing patterns for such
> cases in glibc; the one I've used here is that a getrlimit64 macro
> disables the weak_alias / libc_hidden_weak calls, leaving it to the
> including file to define the getrlimit64 name in whatever way is
> appropriate.
> 
> Tested for x86_64 and x86 that installed stripped shared libraries are
> unchanged by this patch.
> [...]
> @@ -40,4 +40,6 @@ getrlimit64 (enum __rlimit_resource resource, struct rlimit64 *rlimits)
>  
>    return 0;
>  }
> -libc_hidden_def (getrlimit64)
> +libc_hidden_def (__getrlimit64)
> +weak_alias (__getrlimit64, getrlimit64)
> +libc_hidden_weak (getrlimit64)
> diff --git a/sysdeps/posix/spawni.c b/sysdeps/posix/spawni.c
> index 4a25c89..eee9331 100644
> --- a/sysdeps/posix/spawni.c
> +++ b/sysdeps/posix/spawni.c
> @@ -188,7 +188,7 @@ __spawni (pid_t *pid, const char *file,
>  		{
>  		  if (! have_fdlimit)
>  		    {
> -		      getrlimit64 (RLIMIT_NOFILE, &fdlimit);
> +		      __getrlimit64 (RLIMIT_NOFILE, &fdlimit);
>  		      have_fdlimit = true;
>  		    }

Wouldn't it make more sense to use sysconf() to get the limit?

Rich
Joseph Myers Feb. 17, 2015, 10:25 p.m. UTC | #2 
On Tue, 17 Feb 2015, Rich Felker wrote:

> Wouldn't it make more sense to use sysconf() to get the limit?

I don't know if there's some reason for that to be abstractly preferable, 
but in the context of this sort of bug fix it's desirable to avoid 
changing the disassembly of installed shared libraries, to make clear the 
patch is safe at least for the tested configuration, if possible.
Roland McGrath Feb. 18, 2015, 12:15 a.m. UTC | #3 
Looks OK.
Rich Felker Feb. 18, 2015, 12:26 a.m. UTC | #4 
On Tue, Feb 17, 2015 at 10:25:05PM +0000, Joseph Myers wrote:
> On Tue, 17 Feb 2015, Rich Felker wrote:
> 
> > Wouldn't it make more sense to use sysconf() to get the limit?
> 
> I don't know if there's some reason for that to be abstractly preferable, 

Abstractly sysconf seems like the preferred way to get
runtime-variable limits, and it's specified to track changes to the
open files rlimit.

> but in the context of this sort of bug fix it's desirable to avoid 
> changing the disassembly of installed shared libraries, to make clear the 
> patch is safe at least for the tested configuration, if possible.

Understandable.

Rich
diff mbox

Patch

diff --git a/conform/Makefile b/conform/Makefile
index 331590a..c0c0db7 100644
--- a/conform/Makefile
+++ b/conform/Makefile
@@ -391,7 +391,6 @@  test-xfail-XOPEN2K/netdb.h/linknamespace = yes
 test-xfail-XOPEN2K/regex.h/linknamespace = yes
 test-xfail-XOPEN2K/search.h/linknamespace = yes
 test-xfail-XOPEN2K/signal.h/linknamespace = yes
-test-xfail-XOPEN2K/spawn.h/linknamespace = yes
 test-xfail-XOPEN2K/stdlib.h/linknamespace = yes
 test-xfail-XOPEN2K/sys/wait.h/linknamespace = yes
 test-xfail-XOPEN2K/syslog.h/linknamespace = yes
@@ -403,7 +402,6 @@  test-xfail-POSIX2008/grp.h/linknamespace = yes
 test-xfail-POSIX2008/netdb.h/linknamespace = yes
 test-xfail-POSIX2008/regex.h/linknamespace = yes
 test-xfail-POSIX2008/semaphore.h/linknamespace = yes
-test-xfail-POSIX2008/spawn.h/linknamespace = yes
 test-xfail-POSIX2008/unistd.h/linknamespace = yes
 test-xfail-XOPEN2K8/dirent.h/linknamespace = yes
 test-xfail-XOPEN2K8/fmtmsg.h/linknamespace = yes
@@ -412,6 +410,5 @@  test-xfail-XOPEN2K8/netdb.h/linknamespace = yes
 test-xfail-XOPEN2K8/pwd.h/linknamespace = yes
 test-xfail-XOPEN2K8/regex.h/linknamespace = yes
 test-xfail-XOPEN2K8/search.h/linknamespace = yes
-test-xfail-XOPEN2K8/spawn.h/linknamespace = yes
 test-xfail-XOPEN2K8/syslog.h/linknamespace = yes
 test-xfail-XOPEN2K8/unistd.h/linknamespace = yes
diff --git a/include/sys/resource.h b/include/sys/resource.h
index 1ce190f..7622da9 100644
--- a/include/sys/resource.h
+++ b/include/sys/resource.h
@@ -5,6 +5,8 @@ 
 libc_hidden_proto (getpriority)
 libc_hidden_proto (setpriority)
 libc_hidden_proto (getrlimit64)
+extern __typeof (getrlimit64) __getrlimit64;
+libc_hidden_proto (__getrlimit64);
 
 /* Now define the internal interfaces.  */
 extern int __getrlimit (enum __rlimit_resource __resource,
diff --git a/resource/getrlimit64.c b/resource/getrlimit64.c
index 6d22eca..cb494cc 100644
--- a/resource/getrlimit64.c
+++ b/resource/getrlimit64.c
@@ -22,7 +22,7 @@ 
 /* Put the soft and hard limits for RESOURCE in *RLIMITS.
    Returns 0 if successful, -1 if not (and sets errno).  */
 int
-getrlimit64 (enum __rlimit_resource resource, struct rlimit64 *rlimits)
+__getrlimit64 (enum __rlimit_resource resource, struct rlimit64 *rlimits)
 {
   struct rlimit rlimits32;
 
@@ -40,4 +40,6 @@  getrlimit64 (enum __rlimit_resource resource, struct rlimit64 *rlimits)
 
   return 0;
 }
-libc_hidden_def (getrlimit64)
+libc_hidden_def (__getrlimit64)
+weak_alias (__getrlimit64, getrlimit64)
+libc_hidden_weak (getrlimit64)
diff --git a/sysdeps/posix/spawni.c b/sysdeps/posix/spawni.c
index 4a25c89..eee9331 100644
--- a/sysdeps/posix/spawni.c
+++ b/sysdeps/posix/spawni.c
@@ -188,7 +188,7 @@  __spawni (pid_t *pid, const char *file,
 		{
 		  if (! have_fdlimit)
 		    {
-		      getrlimit64 (RLIMIT_NOFILE, &fdlimit);
+		      __getrlimit64 (RLIMIT_NOFILE, &fdlimit);
 		      have_fdlimit = true;
 		    }
 
diff --git a/sysdeps/unix/sysv/linux/getrlimit64.c b/sysdeps/unix/sysv/linux/getrlimit64.c
index 461d345..100ba62 100644
--- a/sysdeps/unix/sysv/linux/getrlimit64.c
+++ b/sysdeps/unix/sysv/linux/getrlimit64.c
@@ -24,7 +24,7 @@ 
 /* Put the soft and hard limits for RESOURCE in *RLIMITS.
    Returns 0 if successful, -1 if not (and sets errno).  */
 int
-getrlimit64 (enum __rlimit_resource resource, struct rlimit64 *rlimits)
+__getrlimit64 (enum __rlimit_resource resource, struct rlimit64 *rlimits)
 {
 #ifdef __ASSUME_PRLIMIT64
   return INLINE_SYSCALL (prlimit64, 4, 0, resource, NULL, rlimits);
@@ -51,4 +51,8 @@  getrlimit64 (enum __rlimit_resource resource, struct rlimit64 *rlimits)
   return 0;
 #endif
 }
-libc_hidden_def (getrlimit64)
+libc_hidden_def (__getrlimit64)
+#ifndef getrlimit64
+weak_alias (__getrlimit64, getrlimit64)
+libc_hidden_weak (getrlimit64)
+#endif
diff --git a/sysdeps/unix/sysv/linux/i386/getrlimit64.c b/sysdeps/unix/sysv/linux/i386/getrlimit64.c
index a771584..8bd3bd9 100644
--- a/sysdeps/unix/sysv/linux/i386/getrlimit64.c
+++ b/sysdeps/unix/sysv/linux/i386/getrlimit64.c
@@ -21,5 +21,5 @@ 
 
 #undef getrlimit64
 #include <shlib-compat.h>
-versioned_symbol (libc, __new_getrlimit64, getrlimit64, GLIBC_2_2);
-strong_alias (__new_getrlimit64, __GI_getrlimit64)
+versioned_symbol (libc, __getrlimit64, getrlimit64, GLIBC_2_2);
+strong_alias (__getrlimit64, __GI_getrlimit64)
diff --git a/sysdeps/unix/sysv/linux/mips/getrlimit64.c b/sysdeps/unix/sysv/linux/mips/getrlimit64.c
index 7080f77..751b11a 100644
--- a/sysdeps/unix/sysv/linux/mips/getrlimit64.c
+++ b/sysdeps/unix/sysv/linux/mips/getrlimit64.c
@@ -25,8 +25,8 @@ 
 # include <sysdeps/unix/sysv/linux/getrlimit64.c>
 # undef getrlimit64
 
-versioned_symbol (libc, __new_getrlimit64, getrlimit64, GLIBC_2_19);
-strong_alias (__new_getrlimit64, __GI_getrlimit64)
+versioned_symbol (libc, __getrlimit64, getrlimit64, GLIBC_2_19);
+strong_alias (__getrlimit64, __GI_getrlimit64)
 
 # if SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_19)
 
@@ -45,7 +45,7 @@  __old_getrlimit64 (enum __rlimit_resource resource,
 {
   struct rlimit64 krlimits;
 
-  if (__new_getrlimit64 (resource, &krlimits) < 0)
+  if (__getrlimit64 (resource, &krlimits) < 0)
     return -1;
 
   if (krlimits.rlim_cur == RLIM64_INFINITY)
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/syscalls.list b/sysdeps/unix/sysv/linux/powerpc/powerpc64/syscalls.list
index aee60bf..6ba6f9b 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/syscalls.list
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/syscalls.list
@@ -1,3 +1,3 @@ 
 # File name	Caller	Syscall name	# args	Strong name	Weak names
 
-getrlimit	-	ugetrlimit	i:ip	__getrlimit	getrlimit getrlimit64
+getrlimit	-	ugetrlimit	i:ip	__getrlimit	getrlimit getrlimit64 __getrlimit64
diff --git a/sysdeps/unix/sysv/linux/wordsize-64/syscalls.list b/sysdeps/unix/sysv/linux/wordsize-64/syscalls.list
index 77aa246..2876bbd 100644
--- a/sysdeps/unix/sysv/linux/wordsize-64/syscalls.list
+++ b/sysdeps/unix/sysv/linux/wordsize-64/syscalls.list
@@ -10,7 +10,7 @@  statfs		-	statfs		i:sp	__statfs	statfs statfs64
 mmap		-	mmap		b:aniiii __mmap		mmap __mmap64 mmap64
 ftruncate	-	ftruncate	i:ii	__ftruncate	ftruncate ftruncate64 __ftruncate64
 truncate	-	truncate	i:si	truncate	truncate64
-getrlimit	-	getrlimit	i:ip	__getrlimit	getrlimit getrlimit64
+getrlimit	-	getrlimit	i:ip	__getrlimit	getrlimit getrlimit64 __getrlimit64
 setrlimit	-	setrlimit	i:ip	__setrlimit	setrlimit setrlimit64
 readahead	-	readahead	i:iii	__readahead	readahead
 sendfile	-	sendfile	i:iipi	sendfile	sendfile64