[PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[H.J. Lu]

If an IFUNC function is called before the providing shared library is
unrelocated, ld.so may segfault.  Add a testcase to verify that ld.so
will issue a diagnostic and won't segfault in this case.

Tested on i686 and x86-64.  OK for master?

H.J.
---
	[BZ #20019]
	* sysdeps/x86/Makefile (tests): Add tst-ifunc1.
	(tests-special): Add $(objpfx)tst-ifunc1.out.
	($(objpfx)tst-ifunc1.out): New rule.
	(extra-test-objs): Add tst-ifunc1 objects.
	($(objpfx)tst-ifunc1): New.
	($(objpfx)tst-ifuncmod1a.so): New rule.
	($(objpfx)tst-ifuncmod1b.so): LIkewise.
	($(objpfx)tst-ifuncmod1a.os): LIkewise.
	($(objpfx)tst-ifuncmod1b.os): LIkewise.
	* sysdeps/x86/tst-ifunc1.c: New file.
	* sysdeps/x86/tst-ifunc1.sh: LIkewise.
	* sysdeps/x86/tst-ifuncmod1a.c: LIkewise.
	* sysdeps/x86/tst-ifuncmod1b.c: LIkewise.
---
 sysdeps/x86/Makefile         | 25 +++++++++++++++++++++++++
 sysdeps/x86/tst-ifunc1.c     | 26 ++++++++++++++++++++++++++
 sysdeps/x86/tst-ifunc1.sh    | 35 +++++++++++++++++++++++++++++++++++
 sysdeps/x86/tst-ifuncmod1a.c | 27 +++++++++++++++++++++++++++
 sysdeps/x86/tst-ifuncmod1b.c | 23 +++++++++++++++++++++++
 5 files changed, 136 insertions(+)
 create mode 100644 sysdeps/x86/tst-ifunc1.c
 create mode 100755 sysdeps/x86/tst-ifunc1.sh
 create mode 100644 sysdeps/x86/tst-ifuncmod1a.c
 create mode 100644 sysdeps/x86/tst-ifuncmod1b.c

diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index 0d0326c2..6907b46 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -7,4 +7,29 @@ sysdep-dl-routines += dl-get-cpu-features
 
 tests += tst-get-cpu-features
 tests-static += tst-get-cpu-features-static
+
+ifeq ($(build-shared),yes)
+tests += tst-ifunc1
+tests-special += $(objpfx)tst-ifunc1.out
+$(objpfx)tst-ifunc1.out: ../sysdeps/x86/tst-ifunc1.sh $(objpfx)tst-ifunc1
+	$(BASH) $< $(objpfx) '$(test-via-rtld-prefix)' \
+	  '$(test-wrapper-env)' '$(run-program-env)'; \
+	$(evaluate-test)
+
+# Since tst-ifuncmod1a.so and tst-ifuncmod1b.so aren't linked against
+# libc.so, use special rules to build them.
+extra-test-objs += tst-ifunc1 tst-ifuncmod1a.os tst-ifuncmod1b.os \
+		   tst-ifuncmod1a.os tst-ifuncmod1b.os
+
+$(objpfx)tst-ifunc1: $(objpfx)tst-ifuncmod1a.so
+$(objpfx)tst-ifuncmod1a.so: $(objpfx)tst-ifuncmod1a.os \
+			    $(objpfx)tst-ifuncmod1b.so
+	$(CC) -Wl,-z,now -shared -nostdlib -nostartfiles -o $@ $^
+$(objpfx)tst-ifuncmod1b.so: $(objpfx)tst-ifuncmod1b.os
+	$(CC) -Wl,-z,now -shared -nostdlib -nostartfiles -o $@ $^
+$(objpfx)tst-ifuncmod1a.os: ../sysdeps/x86/tst-ifuncmod1a.c
+	$(CC) -fPIC -c -o $@ $^
+$(objpfx)tst-ifuncmod1b.os: ../sysdeps/x86/tst-ifuncmod1b.c
+	$(CC) -fPIC -c -o $@ $^
+endif
 endif
diff --git a/sysdeps/x86/tst-ifunc1.c b/sysdeps/x86/tst-ifunc1.c
new file mode 100644
index 0000000..735153f
--- /dev/null
+++ b/sysdeps/x86/tst-ifunc1.c
@@ -0,0 +1,26 @@
+/* Test case for x86 IFUNC.
+   Copyright (C) 2016 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/>.  */
+
+extern void foo (void);
+
+int
+main (void)
+{
+  foo ();
+  return 0;
+}
diff --git a/sysdeps/x86/tst-ifunc1.sh b/sysdeps/x86/tst-ifunc1.sh
new file mode 100755
index 0000000..5f5afe0
--- /dev/null
+++ b/sysdeps/x86/tst-ifunc1.sh
@@ -0,0 +1,35 @@
+#!/bin/bash
+# An x86 IFUNC test.
+# Copyright (C) 2016 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/>.
+
+set -e
+
+objpfx=$1; shift
+test_via_rtld_prefix=$1; shift
+test_wrapper_env=$1; shift
+run_program_env=$1; shift
+logfile=${objpfx}tst-ifunc1.out
+
+> $logfile
+fail=0
+
+${test_wrapper_env} \
+${run_program_env} \
+${objpfx}tst-ifunc1 2>&1 | grep Relink >> $logfile || fail=1
+
+exit $fail
diff --git a/sysdeps/x86/tst-ifuncmod1a.c b/sysdeps/x86/tst-ifuncmod1a.c
new file mode 100644
index 0000000..a1c27ed
--- /dev/null
+++ b/sysdeps/x86/tst-ifuncmod1a.c
@@ -0,0 +1,27 @@
+/* Test case for x86 IFUNC.
+   Copyright (C) 2016 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/>.  */
+
+void bar (void *dst, void *src);
+
+void
+foo (void)
+{
+  char dst[50];
+  char src[50];
+  bar (dst, src);
+}
diff --git a/sysdeps/x86/tst-ifuncmod1b.c b/sysdeps/x86/tst-ifuncmod1b.c
new file mode 100644
index 0000000..e6e9e9b
--- /dev/null
+++ b/sysdeps/x86/tst-ifuncmod1b.c
@@ -0,0 +1,23 @@
+/* Test case for x86 IFUNC.
+   Copyright (C) 2016 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/>.  */
+
+void
+bar (void *dst, void *src)
+{
+  __builtin_memmove (dst, src, 40);
+}
-- 
2.7.4

Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[Joseph Myers]

On Tue, 4 Oct 2016, H.J. Lu wrote:

> If an IFUNC function is called before the providing shared library is
> unrelocated, ld.so may segfault.  Add a testcase to verify that ld.so
> will issue a diagnostic and won't segfault in this case.
> 
> Tested on i686 and x86-64.  OK for master?

I can't see anything x86-specific about these tests.  If they are meant to 
work, they should work on all architectures, and so should be 
architecture-independent.  Is the architecture-specific thing the use of 
memmove as a function that uses IFUNCs?  If so, the tests should still 
work on other architectures, just maybe be less effective as tests (and 
other architecture maintainers could always add a hook to use another 
function instead of memmove).

If you suspect other architectures might have similar bugs requiring fixes 
for these tests to pass but don't want to make such changes yourself, you 
should follow the usual process for changes made only for some 
architectures: post a detailed description of how to tell whether the 
change is needed for your architecture and how to make the change, CC: to 
the relevant architecture maintainers, and add an entry to 
https://sourceware.org/glibc/wiki/PortStatus (once the x86 changes are 
in), listing the possibly affected (i.e. IFUNC-supporting?) architectures 
on that page.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[H.J. Lu]

On Tue, Oct 4, 2016 at 2:27 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Tue, 4 Oct 2016, H.J. Lu wrote:
>
>> If an IFUNC function is called before the providing shared library is
>> unrelocated, ld.so may segfault.  Add a testcase to verify that ld.so
>> will issue a diagnostic and won't segfault in this case.
>>
>> Tested on i686 and x86-64.  OK for master?
>
> I can't see anything x86-specific about these tests.  If they are meant to
> work, they should work on all architectures, and so should be
> architecture-independent.  Is the architecture-specific thing the use of
> memmove as a function that uses IFUNCs?  If so, the tests should still
> work on other architectures, just maybe be less effective as tests (and
> other architecture maintainers could always add a hook to use another
> function instead of memmove).

The result of this test is IFUNC implementation specific.  The older
x86 IFUNC implementation had

# define INIT_ARCH() \
  do                                                    \
    if (__cpu_features.kind == arch_kind_unknown)       \
      __init_cpu_features ();                           \
  while (0)

The new implementation assumed that relocations in libc.so were
processed first.  It is hard for me to tell if other IFUNC implementations
have the similar restriction.

> If you suspect other architectures might have similar bugs requiring fixes
> for these tests to pass but don't want to make such changes yourself, you
> should follow the usual process for changes made only for some
> architectures: post a detailed description of how to tell whether the
> change is needed for your architecture and how to make the change, CC: to
> the relevant architecture maintainers, and add an entry to
> https://sourceware.org/glibc/wiki/PortStatus (once the x86 changes are
> in), listing the possibly affected (i.e. IFUNC-supporting?) architectures
> on that page.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com



-- 
H.J.

Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[Joseph Myers]

On Tue, 4 Oct 2016, H.J. Lu wrote:

> On Tue, Oct 4, 2016 at 2:27 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> > On Tue, 4 Oct 2016, H.J. Lu wrote:
> >
> >> If an IFUNC function is called before the providing shared library is
> >> unrelocated, ld.so may segfault.  Add a testcase to verify that ld.so
> >> will issue a diagnostic and won't segfault in this case.
> >>
> >> Tested on i686 and x86-64.  OK for master?
> >
> > I can't see anything x86-specific about these tests.  If they are meant to
> > work, they should work on all architectures, and so should be
> > architecture-independent.  Is the architecture-specific thing the use of
> > memmove as a function that uses IFUNCs?  If so, the tests should still
> > work on other architectures, just maybe be less effective as tests (and
> > other architecture maintainers could always add a hook to use another
> > function instead of memmove).
> 
> The result of this test is IFUNC implementation specific.  The older
> x86 IFUNC implementation had
> 
> # define INIT_ARCH() \
>   do                                                    \
>     if (__cpu_features.kind == arch_kind_unknown)       \
>       __init_cpu_features ();                           \
>   while (0)
> 
> The new implementation assumed that relocations in libc.so were
> processed first.  It is hard for me to tell if other IFUNC implementations
> have the similar restriction.

You seem to be saying there was some bug in the x86 IFUNC implementation 
such that you don't know whether a corresponding bug might be present for 
other architectures or not.  That is a very strong indication that the 
tests should be architecture-independent, so that people can use the 
results of those tests on other architectures to tell whether those other 
architectures need fixing as well.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[H.J. Lu]

On Tue, Oct 4, 2016 at 3:55 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Tue, 4 Oct 2016, H.J. Lu wrote:
>
>> On Tue, Oct 4, 2016 at 2:27 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>> > On Tue, 4 Oct 2016, H.J. Lu wrote:
>> >
>> >> If an IFUNC function is called before the providing shared library is
>> >> unrelocated, ld.so may segfault.  Add a testcase to verify that ld.so
>> >> will issue a diagnostic and won't segfault in this case.
>> >>
>> >> Tested on i686 and x86-64.  OK for master?
>> >
>> > I can't see anything x86-specific about these tests.  If they are meant to
>> > work, they should work on all architectures, and so should be
>> > architecture-independent.  Is the architecture-specific thing the use of
>> > memmove as a function that uses IFUNCs?  If so, the tests should still
>> > work on other architectures, just maybe be less effective as tests (and
>> > other architecture maintainers could always add a hook to use another
>> > function instead of memmove).
>>
>> The result of this test is IFUNC implementation specific.  The older
>> x86 IFUNC implementation had
>>
>> # define INIT_ARCH() \
>>   do                                                    \
>>     if (__cpu_features.kind == arch_kind_unknown)       \
>>       __init_cpu_features ();                           \
>>   while (0)
>>
>> The new implementation assumed that relocations in libc.so were
>> processed first.  It is hard for me to tell if other IFUNC implementations
>> have the similar restriction.
>
> You seem to be saying there was some bug in the x86 IFUNC implementation

It is a limitation.

> such that you don't know whether a corresponding bug might be present for
> other architectures or not.  That is a very strong indication that the
> tests should be architecture-independent, so that people can use the
> results of those tests on other architectures to tell whether those other
> architectures need fixing as well.
>

I can try __builtin_memcpy, instread of __builtin_memmove.   There are 2
acceptable results.  One is ld.so issues an error and the other is program runs.
On x86, ld.so issues an error.  I don't know what should happen on others.

-- 
H.J.

Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[Joseph Myers]

On Wed, 5 Oct 2016, H.J. Lu wrote:

> I can try __builtin_memcpy, instread of __builtin_memmove.   There are 2
> acceptable results.  One is ld.so issues an error and the other is program runs.
> On x86, ld.so issues an error.  I don't know what should happen on others.

You could make the test pass on either of those results (while failing if 
ld.so crashes).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 624 bytes --]

On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Wed, 5 Oct 2016, H.J. Lu wrote:
>
>> I can try __builtin_memcpy, instread of __builtin_memmove.   There are 2

I changed it to use __builtin_memset.

>> acceptable results.  One is ld.so issues an error and the other is program runs.
>> On x86, ld.so issues an error.  I don't know what should happen on others.
>
> You could make the test pass on either of those results (while failing if
> ld.so crashes).
>

I moved the test to elf.  It passes if the test runs or ld.so issues an
error.  Please try it on arm, powerpc and s390.

-- 
H.J.

[-- Attachment #2: 0001-Add-an-IFUNC-testcase-for-BZ-20019.patch --]
[-- Type: text/x-patch, Size: 7631 bytes --]

From 45fa40391e8e46ccfe3fde6120671e638f54ed45 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 4 Oct 2016 11:29:55 -0700
Subject: [PATCH] Add an IFUNC testcase for [BZ #20019]

If an IFUNC function is called before the providing shared library is
unrelocated, ld.so may segfault.  Add a testcase to verify that the
program won't segfault.  It should either run or ld.so issues a
diagnostic.

	[BZ #20019]
	* elf/Makefile (tests): Add tst-ifunc1.
	(tests-special): Add $(objpfx)tst-ifunc1.out.
	($(objpfx)tst-ifunc1.out): New rule.
	(extra-test-objs): Add tst-ifunc1 objects.
	($(objpfx)tst-ifunc1): New.
	($(objpfx)tst-ifuncmod1a.so): New rule.
	($(objpfx)tst-ifuncmod1b.so): LIkewise.
	($(objpfx)tst-ifuncmod1a.os): LIkewise.
	($(objpfx)tst-ifuncmod1b.os): LIkewise.
	* elf/tst-ifunc1.c: New file.
	* elf/tst-ifunc1.sh: LIkewise.
	* elf/tst-ifuncmod1a.c: LIkewise.
	* elf/tst-ifuncmod1b.c: LIkewise.
---
 elf/Makefile         | 26 ++++++++++++++++++++++++++
 elf/tst-ifunc1.c     | 26 ++++++++++++++++++++++++++
 elf/tst-ifunc1.sh    | 43 +++++++++++++++++++++++++++++++++++++++++++
 elf/tst-ifuncmod1a.c | 26 ++++++++++++++++++++++++++
 elf/tst-ifuncmod1b.c | 23 +++++++++++++++++++++++
 5 files changed, 144 insertions(+)
 create mode 100644 elf/tst-ifunc1.c
 create mode 100755 elf/tst-ifunc1.sh
 create mode 100644 elf/tst-ifuncmod1a.c
 create mode 100644 elf/tst-ifuncmod1b.c

diff --git a/elf/Makefile b/elf/Makefile
index caffd92..6a5eaf0 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -329,6 +329,11 @@ $(objpfx)tst-_dl_addr_inside_object: $(objpfx)dl-addr-obj.os
 CFLAGS-tst-_dl_addr_inside_object.c += $(PIE-ccflag)
 endif
 
+ifeq ($(run-built-tests)$(build-shared),yesyes)
+tests += tst-ifunc1
+tests-special += $(objpfx)tst-ifunc1.out
+endif
+
 include ../Rules
 
 ifeq (yes,$(build-shared))
@@ -965,6 +970,27 @@ CFLAGS-tst-pie2.c += $(pie-ccflag)
 
 $(objpfx)tst-pie1: $(objpfx)tst-piemod1.so
 
+$(objpfx)tst-ifunc1.out: tst-ifunc1.sh $(objpfx)tst-ifunc1
+	$(BASH) $< $(objpfx) '$(test-via-rtld-prefix)' \
+	  '$(test-wrapper-env)' '$(run-program-env)'; \
+	$(evaluate-test)
+
+# Since tst-ifuncmod1a.so and tst-ifuncmod1b.so aren't linked against
+# libc.so, use special rules to build them.
+extra-test-objs += tst-ifunc1 tst-ifuncmod1a.os tst-ifuncmod1b.os \
+		   tst-ifuncmod1a.os tst-ifuncmod1b.os
+
+$(objpfx)tst-ifunc1: $(objpfx)tst-ifuncmod1a.so
+$(objpfx)tst-ifuncmod1a.so: $(objpfx)tst-ifuncmod1a.os \
+			    $(objpfx)tst-ifuncmod1b.so
+	$(CC) -Wl,-z,now -shared -nostdlib -nostartfiles -o $@ $^
+$(objpfx)tst-ifuncmod1b.so: $(objpfx)tst-ifuncmod1b.os
+	$(CC) -Wl,-z,now -shared -nostdlib -nostartfiles -o $@ $^
+$(objpfx)tst-ifuncmod1a.os: tst-ifuncmod1a.c
+	$(CC) -fPIC -c -o $@ $<
+$(objpfx)tst-ifuncmod1b.os: tst-ifuncmod1b.c
+	$(CC) -fPIC -c -o $@ $<
+
 ifeq (yes,$(build-shared))
 all-built-dso := $(common-objpfx)elf/ld.so $(common-objpfx)libc.so \
 		 $(filter-out $(common-objpfx)linkobj/libc.so, \
diff --git a/elf/tst-ifunc1.c b/elf/tst-ifunc1.c
new file mode 100644
index 0000000..d6a54aa
--- /dev/null
+++ b/elf/tst-ifunc1.c
@@ -0,0 +1,26 @@
+/* Test case for IFUNC.
+   Copyright (C) 2016 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/>.  */
+
+extern void foo (void);
+
+int
+main (void)
+{
+  foo ();
+  return 0;
+}
diff --git a/elf/tst-ifunc1.sh b/elf/tst-ifunc1.sh
new file mode 100755
index 0000000..7152d79
--- /dev/null
+++ b/elf/tst-ifunc1.sh
@@ -0,0 +1,43 @@
+#!/bin/bash
+# An IFUNC test.
+# Copyright (C) 2016 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/>.
+
+set -e
+
+objpfx=$1; shift
+test_via_rtld_prefix=$1; shift
+test_wrapper_env=$1; shift
+run_program_env=$1; shift
+logfile=${objpfx}tst-ifunc1.out
+
+> $logfile
+fail=0
+
+${test_wrapper_env} \
+${run_program_env} \
+${objpfx}tst-ifunc1 2>&1 || fail=1
+
+if test $fail = 1; then
+  # If it fails to run, check for the expected error from ld.so.
+  fail=0
+  ${test_wrapper_env} \
+  ${run_program_env} \
+  ${objpfx}tst-ifunc1 2>&1 | grep Relink >> $logfile || fail=1
+fi
+
+exit $fail
diff --git a/elf/tst-ifuncmod1a.c b/elf/tst-ifuncmod1a.c
new file mode 100644
index 0000000..a5b818d
--- /dev/null
+++ b/elf/tst-ifuncmod1a.c
@@ -0,0 +1,26 @@
+/* Test case for IFUNC.
+   Copyright (C) 2016 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/>.  */
+
+void bar (void *dst, unsigned int);
+
+void
+foo (void)
+{
+  char dst[50];
+  bar (dst, sizeof (dst));
+}
diff --git a/elf/tst-ifuncmod1b.c b/elf/tst-ifuncmod1b.c
new file mode 100644
index 0000000..b7a6315
--- /dev/null
+++ b/elf/tst-ifuncmod1b.c
@@ -0,0 +1,23 @@
+/* Test case for IFUNC.
+   Copyright (C) 2016 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/>.  */
+
+void
+bar (void *dst, unsigned int len)
+{
+  __builtin_memset (dst, 3, len);
+}
-- 
2.7.4

Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[Adhemerval Zanella]

On 05/10/2016 15:16, H.J. Lu wrote:
> On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>> On Wed, 5 Oct 2016, H.J. Lu wrote:
>>
>>> I can try __builtin_memcpy, instread of __builtin_memmove.   There are 2
> 
> I changed it to use __builtin_memset.
> 
>>> acceptable results.  One is ld.so issues an error and the other is program runs.
>>> On x86, ld.so issues an error.  I don't know what should happen on others.
>>
>> You could make the test pass on either of those results (while failing if
>> ld.so crashes).
>>
> 
> I moved the test to elf.  It passes if the test runs or ld.so issues an
> error.  Please try it on arm, powerpc and s390.
> 

It shows no issue neither on arm (gcc 4.8.4, binutils 2.24) or powerpc64le
(gcc 5.4.0, binutils 2.26.1).

Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[Carlos O'Donell]

On 10/05/2016 02:16 PM, H.J. Lu wrote:
> On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>> On Wed, 5 Oct 2016, H.J. Lu wrote:
>>
>>> I can try __builtin_memcpy, instread of __builtin_memmove.   There are 2
> 
> I changed it to use __builtin_memset.
> 
>>> acceptable results.  One is ld.so issues an error and the other is program runs.
>>> On x86, ld.so issues an error.  I don't know what should happen on others.
>>
>> You could make the test pass on either of those results (while failing if
>> ld.so crashes).
>>
> 
> I moved the test to elf.  It passes if the test runs or ld.so issues an
> error.  Please try it on arm, powerpc and s390.
 
This is the wrong way to test this.

The point of this test is this:

- Verify that an unversioned symbol reference in DSO A which has no DT_NEEDED
  on DSO B, when resolved to a symbol definition in DSO B, when the symbol in 
  DSO B is an IFUNC with a resolver, that DSO B is relocated _before_ the IFUNC
  resolver is called, because DSO B's resolver might need global data to make
  the IFUNC decision e.g. GOT setup.

The invariant we want to hold true for IFUNC is that to call the resolver
function you must have relocated the DSO which contains the resolver. This _should_
have been done by a symbol reocation dependency analysis, but that isn't working
correctly IMO or needs deeper analysis in the dynamic loader.

The solution we want in place today is to issue some kind of diagnostic until we
fix the real problem.

The test should look like this:

- DSO A with an unversioned symbol reference to 'foo'.
- DSO B with a symbol definition of 'foo' as an ifunc with 'foo_resolver' as the
  resolver function which references global data from DSO C to decide which of
  two functions to return.
- DSO C with global data set to a value.

The point is that DSO B depends on DSO C and has DT_NEEDED on it, so C will get
relocated first, then B, such that B's GOT is setup to access C's global data.

When handling the reference to 'foo' in DSO A we should on x86_64 and i686
get the error about needing to relink DSO A so it depends on DSO B, to form
the initialization order of C->B->A.

I expect this test case will now crash the other arches, rather than just
avoiding the crash by relying on internal libc.so details about which ifuncs
you're using.

This is one step towards a better definition of IFUNC semantics, which need to
be more clearly defined (something I wish I had time to define and fix so
more projects could use them).

-- 
Cheers,
Carlos.

Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[H.J. Lu]

On Tue, Oct 11, 2016 at 10:45 PM, Carlos O'Donell <carlos@redhat.com> wrote:
> On 10/05/2016 02:16 PM, H.J. Lu wrote:
>> On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>>> On Wed, 5 Oct 2016, H.J. Lu wrote:
>>>
>>>> I can try __builtin_memcpy, instread of __builtin_memmove.   There are 2
>>
>> I changed it to use __builtin_memset.
>>
>>>> acceptable results.  One is ld.so issues an error and the other is program runs.
>>>> On x86, ld.so issues an error.  I don't know what should happen on others.
>>>
>>> You could make the test pass on either of those results (while failing if
>>> ld.so crashes).
>>>
>>
>> I moved the test to elf.  It passes if the test runs or ld.so issues an
>> error.  Please try it on arm, powerpc and s390.
>
> This is the wrong way to test this.
>
> The point of this test is this:
>
> - Verify that an unversioned symbol reference in DSO A which has no DT_NEEDED
>   on DSO B, when resolved to a symbol definition in DSO B, when the symbol in
>   DSO B is an IFUNC with a resolver, that DSO B is relocated _before_ the IFUNC
>   resolver is called, because DSO B's resolver might need global data to make
>   the IFUNC decision e.g. GOT setup.
>
> The invariant we want to hold true for IFUNC is that to call the resolver
> function you must have relocated the DSO which contains the resolver. This _should_
> have been done by a symbol reocation dependency analysis, but that isn't working
> correctly IMO or needs deeper analysis in the dynamic loader.
>
> The solution we want in place today is to issue some kind of diagnostic until we
> fix the real problem.
>
> The test should look like this:
>
> - DSO A with an unversioned symbol reference to 'foo'.
> - DSO B with a symbol definition of 'foo' as an ifunc with 'foo_resolver' as the
>   resolver function which references global data from DSO C to decide which of
>   two functions to return.
> - DSO C with global data set to a value.
>
> The point is that DSO B depends on DSO C and has DT_NEEDED on it, so C will get
> relocated first, then B, such that B's GOT is setup to access C's global data.
>
> When handling the reference to 'foo' in DSO A we should on x86_64 and i686
> get the error about needing to relink DSO A so it depends on DSO B, to form
> the initialization order of C->B->A.
>
> I expect this test case will now crash the other arches, rather than just
> avoiding the crash by relying on internal libc.so details about which ifuncs
> you're using.
>
> This is one step towards a better definition of IFUNC semantics, which need to
> be more clearly defined (something I wish I had time to define and fix so
> more projects could use them).

IFUNC resolver can fail for various reasons.  My goal is to make sure
that IFUNC inside of glibc works correctly or an error message is given
when glibc isn't used properly.  In case of x86,  CPU feature info is
retrieved and stored in ld.so very early at startup, which is used by IFUNC
and only accessible in libc.so and libm.so after they have been relocated.
My change in x86 ld.so checks it and my test verifies the check.  My fix
won't cover other possible IFUNC failures.

-- 
H.J.

Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 3695 bytes --]

On Wed, Oct 12, 2016 at 3:19 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Tue, Oct 11, 2016 at 10:45 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>> On 10/05/2016 02:16 PM, H.J. Lu wrote:
>>> On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>>>> On Wed, 5 Oct 2016, H.J. Lu wrote:
>>>>
>>>>> I can try __builtin_memcpy, instread of __builtin_memmove.   There are 2
>>>
>>> I changed it to use __builtin_memset.
>>>
>>>>> acceptable results.  One is ld.so issues an error and the other is program runs.
>>>>> On x86, ld.so issues an error.  I don't know what should happen on others.
>>>>
>>>> You could make the test pass on either of those results (while failing if
>>>> ld.so crashes).
>>>>
>>>
>>> I moved the test to elf.  It passes if the test runs or ld.so issues an
>>> error.  Please try it on arm, powerpc and s390.
>>
>> This is the wrong way to test this.
>>
>> The point of this test is this:
>>
>> - Verify that an unversioned symbol reference in DSO A which has no DT_NEEDED
>>   on DSO B, when resolved to a symbol definition in DSO B, when the symbol in
>>   DSO B is an IFUNC with a resolver, that DSO B is relocated _before_ the IFUNC
>>   resolver is called, because DSO B's resolver might need global data to make
>>   the IFUNC decision e.g. GOT setup.
>>
>> The invariant we want to hold true for IFUNC is that to call the resolver
>> function you must have relocated the DSO which contains the resolver. This _should_
>> have been done by a symbol reocation dependency analysis, but that isn't working
>> correctly IMO or needs deeper analysis in the dynamic loader.
>>
>> The solution we want in place today is to issue some kind of diagnostic until we
>> fix the real problem.
>>
>> The test should look like this:
>>
>> - DSO A with an unversioned symbol reference to 'foo'.
>> - DSO B with a symbol definition of 'foo' as an ifunc with 'foo_resolver' as the
>>   resolver function which references global data from DSO C to decide which of
>>   two functions to return.
>> - DSO C with global data set to a value.
>>
>> The point is that DSO B depends on DSO C and has DT_NEEDED on it, so C will get
>> relocated first, then B, such that B's GOT is setup to access C's global data.
>>
>> When handling the reference to 'foo' in DSO A we should on x86_64 and i686
>> get the error about needing to relink DSO A so it depends on DSO B, to form
>> the initialization order of C->B->A.
>>
>> I expect this test case will now crash the other arches, rather than just
>> avoiding the crash by relying on internal libc.so details about which ifuncs
>> you're using.
>>
>> This is one step towards a better definition of IFUNC semantics, which need to
>> be more clearly defined (something I wish I had time to define and fix so
>> more projects could use them).
>
> IFUNC resolver can fail for various reasons.  My goal is to make sure
> that IFUNC inside of glibc works correctly or an error message is given
> when glibc isn't used properly.  In case of x86,  CPU feature info is
> retrieved and stored in ld.so very early at startup, which is used by IFUNC
> and only accessible in libc.so and libm.so after they have been relocated.
> My change in x86 ld.so checks it and my test verifies the check.  My fix
> won't cover other possible IFUNC failures.
>

When the IFUNC relocation is performed before the providing shared
library is unrelocated, the returned function address will be 0 and
program will segfault when the function is called.

Please apply this patch and run the test if your platform has IFUNC.  I only
enabled the unsafe resolver check for i386 and x86-64.  It is straightforward
to add check for other platforms.


H.J.

[-- Attachment #2: 0001-Add-an-IFUNC-testcase-for-BZ-20019.patch --]
[-- Type: text/x-patch, Size: 13423 bytes --]

From 5db1cdaef209ba5b742308d5dcb299aaacdf3f92 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 12 Jan 2017 15:27:05 -0800
Subject: [PATCH] Add an IFUNC testcase for [BZ #20019]

When the IFUNC relocation is performed before the providing shared
library is unrelocated, the returned function address will be 0 and
program will segfault when the function is called.  Add a testcase
to verify that ld.so issues a diagnostic.

	[BZ #20019]
	* elf/Makefile (modules-names): Add tst-ifuncmod1a,
	tst-ifuncmod1b, tst-ifuncmod1c and tst-ifuncmod1d.
	(tests): Add tst-ifunc1.
	(tests-special): Add $(objpfx)tst-ifunc1.out.
	($(objpfx)tst-ifunc1.out): New rule.
	($(objpfx)tst-ifunc1): New dependency.
	($(objpfx)tst-ifuncmod1a.so): LIkewise.
	($(objpfx)tst-ifuncmod1b.so): LIkewise.
	($(objpfx)tst-ifuncmod1c.so): LIkewise.
	(LDFLAGS-tst-ifuncmod1b.so): New.
	* elf/tst-ifunc1.c: New file.
	* elf/tst-ifunc1.sh: LIkewise.
	* elf/tst-ifuncmod1a.c: LIkewise.
	* elf/tst-ifuncmod1b.c: LIkewise.
	* elf/tst-ifuncmod1c.c: LIkewise.
	* elf/tst-ifuncmod1d.c: LIkewise.
	* sysdeps/generic/ldsodefs.h (dl_check_ifunc_resolver): New
	function.
	* sysdeps/i386/dl-machine.h (elf_machine_rel): Use it.
	* sysdeps/x86_64/dl-machine.h (elf_machine_rela): LIkewise.
---
 elf/Makefile                | 19 +++++++++++++++++--
 elf/tst-ifunc1.c            | 26 ++++++++++++++++++++++++++
 elf/tst-ifunc1.sh           | 43 +++++++++++++++++++++++++++++++++++++++++++
 elf/tst-ifuncmod1a.c        | 25 +++++++++++++++++++++++++
 elf/tst-ifuncmod1b.c        | 25 +++++++++++++++++++++++++
 elf/tst-ifuncmod1c.c        | 27 +++++++++++++++++++++++++++
 elf/tst-ifuncmod1d.c        | 24 ++++++++++++++++++++++++
 sysdeps/generic/ldsodefs.h  | 26 ++++++++++++++++++++++++++
 sysdeps/i386/dl-machine.h   | 13 +------------
 sysdeps/x86_64/dl-machine.h | 13 +------------
 10 files changed, 215 insertions(+), 26 deletions(-)
 create mode 100644 elf/tst-ifunc1.c
 create mode 100755 elf/tst-ifunc1.sh
 create mode 100644 elf/tst-ifuncmod1a.c
 create mode 100644 elf/tst-ifuncmod1b.c
 create mode 100644 elf/tst-ifuncmod1c.c
 create mode 100644 elf/tst-ifuncmod1d.c

diff --git a/elf/Makefile b/elf/Makefile
index c7a2969..259362d 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -307,9 +307,11 @@ tests += ifuncmain1 ifuncmain1pic ifuncmain1vis ifuncmain1vispic \
 	 ifuncmain1staticpic \
 	 ifuncmain2 ifuncmain2pic ifuncmain3 ifuncmain4 \
 	 ifuncmain5 ifuncmain5pic ifuncmain5staticpic \
-	 ifuncmain7 ifuncmain7pic
+	 ifuncmain7 ifuncmain7pic tst-ifunc1
+tests-special += $(objpfx)tst-ifunc1.out
 ifunc-test-modules = ifuncdep1 ifuncdep1pic ifuncdep2 ifuncdep2pic \
-		     ifuncdep5 ifuncdep5pic
+		     ifuncdep5 ifuncdep5pic tst-ifuncmod1a \
+		     tst-ifuncmod1b tst-ifuncmod1c tst-ifuncmod1d
 extra-test-objs += $(ifunc-test-modules:=.o)
 test-extras += $(ifunc-test-modules)
 ifeq (yes,$(have-fpie))
@@ -1026,6 +1028,19 @@ CFLAGS-tst-pie2.c += $(pie-ccflag)
 $(objpfx)tst-piemod1.so: $(libsupport)
 $(objpfx)tst-pie1: $(objpfx)tst-piemod1.so
 
+$(objpfx)tst-ifunc1.out: tst-ifunc1.sh $(objpfx)tst-ifunc1
+	$(BASH) $< $(objpfx) '$(test-via-rtld-prefix)' \
+	  '$(test-wrapper-env)' '$(run-program-env)'; \
+	$(evaluate-test)
+
+$(objpfx)tst-ifunc1: $(objpfx)tst-ifuncmod1a.so \
+		     $(objpfx)tst-ifuncmod1c.so \
+		     $(objpfx)tst-ifuncmod1d.so
+$(objpfx)tst-ifuncmod1a.so: $(objpfx)tst-ifuncmod1b.so
+$(objpfx)tst-ifuncmod1b.so: $(objpfx)tst-ifuncmod1d.so
+$(objpfx)tst-ifuncmod1c.so: $(objpfx)tst-ifuncmod1d.so
+LDFLAGS-tst-ifuncmod1b.so = -Wl,-z,now
+
 ifeq (yes,$(build-shared))
 all-built-dso := $(common-objpfx)elf/ld.so $(common-objpfx)libc.so \
 		 $(filter-out $(common-objpfx)linkobj/libc.so, \
diff --git a/elf/tst-ifunc1.c b/elf/tst-ifunc1.c
new file mode 100644
index 0000000..e44dbc3
--- /dev/null
+++ b/elf/tst-ifunc1.c
@@ -0,0 +1,26 @@
+/* Test case for unsafe IFUNC resolver.
+   Copyright (C) 2017 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/>.  */
+
+extern void foo (void);
+
+int
+main (void)
+{
+  foo ();
+  return 0;
+}
diff --git a/elf/tst-ifunc1.sh b/elf/tst-ifunc1.sh
new file mode 100755
index 0000000..e523b44
--- /dev/null
+++ b/elf/tst-ifunc1.sh
@@ -0,0 +1,43 @@
+#!/bin/bash
+# A Test case for unsafe IFUNC resolver.
+# Copyright (C) 2017 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/>.
+
+set -e
+
+objpfx=$1; shift
+test_via_rtld_prefix=$1; shift
+test_wrapper_env=$1; shift
+run_program_env=$1; shift
+logfile=${objpfx}tst-ifunc1.out
+
+> $logfile
+fail=0
+
+${test_wrapper_env} \
+${run_program_env} \
+${objpfx}tst-ifunc1 2>&1 || fail=1
+
+if test $fail = 1; then
+  # If it fails to run, check for the expected error from ld.so.
+  fail=0
+  ${test_wrapper_env} \
+  ${run_program_env} \
+  ${objpfx}tst-ifunc1 2>&1 | grep Relink >> $logfile || fail=1
+fi
+
+exit $fail
diff --git a/elf/tst-ifuncmod1a.c b/elf/tst-ifuncmod1a.c
new file mode 100644
index 0000000..5e2183c
--- /dev/null
+++ b/elf/tst-ifuncmod1a.c
@@ -0,0 +1,25 @@
+/* Test case for unsafe IFUNC resolver.
+   Copyright (C) 2017 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/>.  */
+
+extern void bar (void);
+
+void
+foo (void)
+{
+  bar ();
+}
diff --git a/elf/tst-ifuncmod1b.c b/elf/tst-ifuncmod1b.c
new file mode 100644
index 0000000..0c6fe10
--- /dev/null
+++ b/elf/tst-ifuncmod1b.c
@@ -0,0 +1,25 @@
+/* Test case for unsafe IFUNC resolver.
+   Copyright (C) 2017 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/>.  */
+
+extern void xxx (void);
+
+void
+bar (void)
+{
+  xxx ();
+}
diff --git a/elf/tst-ifuncmod1c.c b/elf/tst-ifuncmod1c.c
new file mode 100644
index 0000000..81cd31c
--- /dev/null
+++ b/elf/tst-ifuncmod1c.c
@@ -0,0 +1,27 @@
+/* Test case for unsafe IFUNC resolver.
+   Copyright (C) 2017 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/>.  */
+
+extern void real_xxx (void);
+
+static void *
+xxx_resolver (void)
+{
+  return &real_xxx;
+}
+
+extern void xxx (void) __attribute__ ((ifunc ("xxx_resolver")));
diff --git a/elf/tst-ifuncmod1d.c b/elf/tst-ifuncmod1d.c
new file mode 100644
index 0000000..5715ded
--- /dev/null
+++ b/elf/tst-ifuncmod1d.c
@@ -0,0 +1,24 @@
+/* Test case for unsafe IFUNC resolver.
+   Copyright (C) 2017 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/>.  */
+
+void
+xxx (void)
+{
+}
+
+__typeof (xxx) real_xxx __attribute__ ((alias("xxx")));
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index f26a8b2..0c53c70 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -1085,6 +1085,32 @@ extern void _dl_non_dynamic_init (void) internal_function;
 extern void _dl_aux_init (ElfW(auxv_t) *av) internal_function;
 
 
+# ifdef STDERR_FILENO
+/* Define here after RTLD_PROGNAME is defined and if STDERR_FILENO is
+   defined.  Since it is unsafe for IFUNC resolver to reference external
+   symbols, issue a fatal error when it happens.  */
+static __always_inline void
+dl_check_ifunc_resolver (struct link_map *sym_map, struct link_map *map,
+			 const ElfW(Sym) *const refsym)
+{
+  if (sym_map != map
+      && sym_map->l_type != lt_executable
+      && !sym_map->l_relocated)
+    {
+      const char *strtab
+	= (const char *) D_PTR (map, l_info[DT_STRTAB]);
+      _dl_fatal_printf ("\
+%s: Relink `%s' with `%s' or place `%s' before `%s' for IFUNC symbol `%s'\n",
+			RTLD_PROGNAME,
+			map->l_libname->name,
+			sym_map->l_libname->name,
+			sym_map->l_libname->name,
+			map->l_libname->name,
+			strtab + refsym->st_name);
+    }
+}
+# endif
+
 __END_DECLS
 
 #endif /* ldsodefs.h */
diff --git a/sysdeps/i386/dl-machine.h b/sysdeps/i386/dl-machine.h
index 6eca69d..b13d97c 100644
--- a/sysdeps/i386/dl-machine.h
+++ b/sysdeps/i386/dl-machine.h
@@ -323,18 +323,7 @@ elf_machine_rel (struct link_map *map, const Elf32_Rel *reloc,
 	  && __builtin_expect (!skip_ifunc, 1))
 	{
 # ifndef RTLD_BOOTSTRAP
-	  if (sym_map != map
-	      && sym_map->l_type != lt_executable
-	      && !sym_map->l_relocated)
-	    {
-	      const char *strtab
-		= (const char *) D_PTR (map, l_info[DT_STRTAB]);
-	      _dl_fatal_printf ("\
-%s: Relink `%s' with `%s' for IFUNC symbol `%s'\n",
-				RTLD_PROGNAME, map->l_name,
-				sym_map->l_name,
-				strtab + refsym->st_name);
-	    }
+	  dl_check_ifunc_resolver (sym_map, map, refsym);
 # endif
 	  value = ((Elf32_Addr (*) (void)) value) ();
 	}
diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
index 3e7ae22..5737955 100644
--- a/sysdeps/x86_64/dl-machine.h
+++ b/sysdeps/x86_64/dl-machine.h
@@ -333,18 +333,7 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
 	  && __builtin_expect (!skip_ifunc, 1))
 	{
 # ifndef RTLD_BOOTSTRAP
-	  if (sym_map != map
-	      && sym_map->l_type != lt_executable
-	      && !sym_map->l_relocated)
-	    {
-	      const char *strtab
-		= (const char *) D_PTR (map, l_info[DT_STRTAB]);
-	      _dl_fatal_printf ("\
-%s: Relink `%s' with `%s' for IFUNC symbol `%s'\n",
-				RTLD_PROGNAME, map->l_name,
-				sym_map->l_name,
-				strtab + refsym->st_name);
-	    }
+	  dl_check_ifunc_resolver (sym_map, map, refsym);
 # endif
 	  value = ((ElfW(Addr) (*) (void)) value) ();
 	}
-- 
2.9.3

Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[Khem Raj]

On 1/13/17 11:03 AM, H.J. Lu wrote:
> On Wed, Oct 12, 2016 at 3:19 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Tue, Oct 11, 2016 at 10:45 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>>> On 10/05/2016 02:16 PM, H.J. Lu wrote:
>>>> On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>>>>> On Wed, 5 Oct 2016, H.J. Lu wrote:
>>>>>
>>>>>> I can try __builtin_memcpy, instread of __builtin_memmove.   There are 2
>>>>
>>>> I changed it to use __builtin_memset.
>>>>
>>>>>> acceptable results.  One is ld.so issues an error and the other is program runs.
>>>>>> On x86, ld.so issues an error.  I don't know what should happen on others.
>>>>>
>>>>> You could make the test pass on either of those results (while failing if
>>>>> ld.so crashes).
>>>>>
>>>>
>>>> I moved the test to elf.  It passes if the test runs or ld.so issues an
>>>> error.  Please try it on arm, powerpc and s390.
>>>
>>> This is the wrong way to test this.
>>>
>>> The point of this test is this:
>>>
>>> - Verify that an unversioned symbol reference in DSO A which has no DT_NEEDED
>>>   on DSO B, when resolved to a symbol definition in DSO B, when the symbol in
>>>   DSO B is an IFUNC with a resolver, that DSO B is relocated _before_ the IFUNC
>>>   resolver is called, because DSO B's resolver might need global data to make
>>>   the IFUNC decision e.g. GOT setup.
>>>
>>> The invariant we want to hold true for IFUNC is that to call the resolver
>>> function you must have relocated the DSO which contains the resolver. This _should_
>>> have been done by a symbol reocation dependency analysis, but that isn't working
>>> correctly IMO or needs deeper analysis in the dynamic loader.
>>>
>>> The solution we want in place today is to issue some kind of diagnostic until we
>>> fix the real problem.
>>>
>>> The test should look like this:
>>>
>>> - DSO A with an unversioned symbol reference to 'foo'.
>>> - DSO B with a symbol definition of 'foo' as an ifunc with 'foo_resolver' as the
>>>   resolver function which references global data from DSO C to decide which of
>>>   two functions to return.
>>> - DSO C with global data set to a value.
>>>
>>> The point is that DSO B depends on DSO C and has DT_NEEDED on it, so C will get
>>> relocated first, then B, such that B's GOT is setup to access C's global data.
>>>
>>> When handling the reference to 'foo' in DSO A we should on x86_64 and i686
>>> get the error about needing to relink DSO A so it depends on DSO B, to form
>>> the initialization order of C->B->A.
>>>
>>> I expect this test case will now crash the other arches, rather than just
>>> avoiding the crash by relying on internal libc.so details about which ifuncs
>>> you're using.
>>>
>>> This is one step towards a better definition of IFUNC semantics, which need to
>>> be more clearly defined (something I wish I had time to define and fix so
>>> more projects could use them).
>>
>> IFUNC resolver can fail for various reasons.  My goal is to make sure
>> that IFUNC inside of glibc works correctly or an error message is given
>> when glibc isn't used properly.  In case of x86,  CPU feature info is
>> retrieved and stored in ld.so very early at startup, which is used by IFUNC
>> and only accessible in libc.so and libm.so after they have been relocated.
>> My change in x86 ld.so checks it and my test verifies the check.  My fix
>> won't cover other possible IFUNC failures.
>>
> 
> When the IFUNC relocation is performed before the providing shared
> library is unrelocated, the returned function address will be 0 and
> program will segfault when the function is called.
> 
> Please apply this patch and run the test if your platform has IFUNC.  I only
> enabled the unsafe resolver check for i386 and x86-64.  It is straightforward
> to add check for other platforms.

I will test it out shortly. One thing I see, the runner script for test
is calling out for /bin/bash and the script does not use any bash
extentions perhaps using /bin/sh is enough.



> 
> 
> H.J.
> 

Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 4208 bytes --]

On Fri, Jan 13, 2017 at 11:30 AM, Khem Raj <raj.khem@gmail.com> wrote:
>
>
> On 1/13/17 11:03 AM, H.J. Lu wrote:
>> On Wed, Oct 12, 2016 at 3:19 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Tue, Oct 11, 2016 at 10:45 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>>>> On 10/05/2016 02:16 PM, H.J. Lu wrote:
>>>>> On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>>>>>> On Wed, 5 Oct 2016, H.J. Lu wrote:
>>>>>>
>>>>>>> I can try __builtin_memcpy, instread of __builtin_memmove.   There are 2
>>>>>
>>>>> I changed it to use __builtin_memset.
>>>>>
>>>>>>> acceptable results.  One is ld.so issues an error and the other is program runs.
>>>>>>> On x86, ld.so issues an error.  I don't know what should happen on others.
>>>>>>
>>>>>> You could make the test pass on either of those results (while failing if
>>>>>> ld.so crashes).
>>>>>>
>>>>>
>>>>> I moved the test to elf.  It passes if the test runs or ld.so issues an
>>>>> error.  Please try it on arm, powerpc and s390.
>>>>
>>>> This is the wrong way to test this.
>>>>
>>>> The point of this test is this:
>>>>
>>>> - Verify that an unversioned symbol reference in DSO A which has no DT_NEEDED
>>>>   on DSO B, when resolved to a symbol definition in DSO B, when the symbol in
>>>>   DSO B is an IFUNC with a resolver, that DSO B is relocated _before_ the IFUNC
>>>>   resolver is called, because DSO B's resolver might need global data to make
>>>>   the IFUNC decision e.g. GOT setup.
>>>>
>>>> The invariant we want to hold true for IFUNC is that to call the resolver
>>>> function you must have relocated the DSO which contains the resolver. This _should_
>>>> have been done by a symbol reocation dependency analysis, but that isn't working
>>>> correctly IMO or needs deeper analysis in the dynamic loader.
>>>>
>>>> The solution we want in place today is to issue some kind of diagnostic until we
>>>> fix the real problem.
>>>>
>>>> The test should look like this:
>>>>
>>>> - DSO A with an unversioned symbol reference to 'foo'.
>>>> - DSO B with a symbol definition of 'foo' as an ifunc with 'foo_resolver' as the
>>>>   resolver function which references global data from DSO C to decide which of
>>>>   two functions to return.
>>>> - DSO C with global data set to a value.
>>>>
>>>> The point is that DSO B depends on DSO C and has DT_NEEDED on it, so C will get
>>>> relocated first, then B, such that B's GOT is setup to access C's global data.
>>>>
>>>> When handling the reference to 'foo' in DSO A we should on x86_64 and i686
>>>> get the error about needing to relink DSO A so it depends on DSO B, to form
>>>> the initialization order of C->B->A.
>>>>
>>>> I expect this test case will now crash the other arches, rather than just
>>>> avoiding the crash by relying on internal libc.so details about which ifuncs
>>>> you're using.
>>>>
>>>> This is one step towards a better definition of IFUNC semantics, which need to
>>>> be more clearly defined (something I wish I had time to define and fix so
>>>> more projects could use them).
>>>
>>> IFUNC resolver can fail for various reasons.  My goal is to make sure
>>> that IFUNC inside of glibc works correctly or an error message is given
>>> when glibc isn't used properly.  In case of x86,  CPU feature info is
>>> retrieved and stored in ld.so very early at startup, which is used by IFUNC
>>> and only accessible in libc.so and libm.so after they have been relocated.
>>> My change in x86 ld.so checks it and my test verifies the check.  My fix
>>> won't cover other possible IFUNC failures.
>>>
>>
>> When the IFUNC relocation is performed before the providing shared
>> library is unrelocated, the returned function address will be 0 and
>> program will segfault when the function is called.
>>
>> Please apply this patch and run the test if your platform has IFUNC.  I only
>> enabled the unsafe resolver check for i386 and x86-64.  It is straightforward
>> to add check for other platforms.
>
> I will test it out shortly. One thing I see, the runner script for test
> is calling out for /bin/bash and the script does not use any bash
> extentions perhaps using /bin/sh is enough.
>

Here is the updated patch with some fixes.

-- 
H.J.

[-- Attachment #2: 0001-Add-an-IFUNC-testcase-for-BZ-20019.patch --]
[-- Type: text/x-patch, Size: 14166 bytes --]

From 0f93f4f0706f2e0f3e4b562907f4701216276808 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 12 Jan 2017 15:27:05 -0800
Subject: [PATCH] Add an IFUNC testcase for [BZ #20019]

When the IFUNC relocation is performed before the providing shared
library is relocated, the returned function address will be 0 and
program will segfault when the function is called.  Add a testcase
to verify that ld.so issues a diagnostic.

	[BZ #20019]
	* elf/Makefile (tests): Add tst-ifunc1.
	(tests-special): Add $(objpfx)tst-ifunc1.out.
	(modules-names): Add tst-ifuncmod1a, tst-ifuncmod1b,
	tst-ifuncmod1c and tst-ifuncmod1d.
	($(objpfx)tst-ifunc1.out): New rule.
	($(objpfx)tst-ifunc1): New dependency.
	($(objpfx)tst-ifuncmod1a.so): LIkewise.
	($(objpfx)tst-ifuncmod1b.so): LIkewise.
	($(objpfx)tst-ifuncmod1c.so): LIkewise.
	(LDFLAGS-tst-ifuncmod1b.so): New.
	* elf/tst-ifunc1.c: New file.
	* elf/tst-ifunc1.sh: LIkewise.
	* elf/tst-ifuncmod1a.c: LIkewise.
	* elf/tst-ifuncmod1b.c: LIkewise.
	* elf/tst-ifuncmod1c.c: LIkewise.
	* elf/tst-ifuncmod1d.c: LIkewise.
	* sysdeps/generic/ldsodefs.h (dl_check_ifunc_resolver): New
	function.
	* sysdeps/i386/dl-machine.h (elf_machine_rel): Use it.
	(elf_machine_rela): Likewise.
	* sysdeps/x86_64/dl-machine.h (elf_machine_rela): Likewise.
---
 elf/Makefile                | 20 ++++++++++++++++++--
 elf/tst-ifunc1.c            | 26 ++++++++++++++++++++++++++
 elf/tst-ifunc1.sh           | 43 +++++++++++++++++++++++++++++++++++++++++++
 elf/tst-ifuncmod1a.c        | 25 +++++++++++++++++++++++++
 elf/tst-ifuncmod1b.c        | 25 +++++++++++++++++++++++++
 elf/tst-ifuncmod1c.c        | 27 +++++++++++++++++++++++++++
 elf/tst-ifuncmod1d.c        | 24 ++++++++++++++++++++++++
 sysdeps/generic/ldsodefs.h  | 26 ++++++++++++++++++++++++++
 sysdeps/i386/dl-machine.h   | 20 +++++++-------------
 sysdeps/x86_64/dl-machine.h | 13 +------------
 10 files changed, 222 insertions(+), 27 deletions(-)
 create mode 100644 elf/tst-ifunc1.c
 create mode 100755 elf/tst-ifunc1.sh
 create mode 100644 elf/tst-ifuncmod1a.c
 create mode 100644 elf/tst-ifuncmod1b.c
 create mode 100644 elf/tst-ifuncmod1c.c
 create mode 100644 elf/tst-ifuncmod1d.c

diff --git a/elf/Makefile b/elf/Makefile
index c7a2969..df2728f 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -307,7 +307,8 @@ tests += ifuncmain1 ifuncmain1pic ifuncmain1vis ifuncmain1vispic \
 	 ifuncmain1staticpic \
 	 ifuncmain2 ifuncmain2pic ifuncmain3 ifuncmain4 \
 	 ifuncmain5 ifuncmain5pic ifuncmain5staticpic \
-	 ifuncmain7 ifuncmain7pic
+	 ifuncmain7 ifuncmain7pic tst-ifunc1
+tests-special += $(objpfx)tst-ifunc1.out
 ifunc-test-modules = ifuncdep1 ifuncdep1pic ifuncdep2 ifuncdep2pic \
 		     ifuncdep5 ifuncdep5pic
 extra-test-objs += $(ifunc-test-modules:=.o)
@@ -318,7 +319,9 @@ ifunc-pie-tests = ifuncmain1pie ifuncmain1vispie ifuncmain1staticpie \
 tests += $(ifunc-pie-tests)
 tests-pie += $(ifunc-pie-tests)
 endif
-modules-names += ifuncmod1 ifuncmod3 ifuncmod5 ifuncmod6
+modules-names += ifuncmod1 ifuncmod3 ifuncmod5 ifuncmod6 \
+		 tst-ifuncmod1a tst-ifuncmod1b tst-ifuncmod1c \
+		 tst-ifuncmod1d
 endif
 endif
 
@@ -1026,6 +1029,19 @@ CFLAGS-tst-pie2.c += $(pie-ccflag)
 $(objpfx)tst-piemod1.so: $(libsupport)
 $(objpfx)tst-pie1: $(objpfx)tst-piemod1.so
 
+$(objpfx)tst-ifunc1.out: tst-ifunc1.sh $(objpfx)tst-ifunc1
+	$(BASH) $< $(objpfx) '$(test-via-rtld-prefix)' \
+	  '$(test-wrapper-env)' '$(run-program-env)'; \
+	$(evaluate-test)
+
+$(objpfx)tst-ifunc1: $(objpfx)tst-ifuncmod1a.so \
+		     $(objpfx)tst-ifuncmod1c.so \
+		     $(objpfx)tst-ifuncmod1d.so
+$(objpfx)tst-ifuncmod1a.so: $(objpfx)tst-ifuncmod1b.so
+$(objpfx)tst-ifuncmod1b.so: $(objpfx)tst-ifuncmod1d.so
+$(objpfx)tst-ifuncmod1c.so: $(objpfx)tst-ifuncmod1d.so
+LDFLAGS-tst-ifuncmod1b.so = -Wl,-z,now
+
 ifeq (yes,$(build-shared))
 all-built-dso := $(common-objpfx)elf/ld.so $(common-objpfx)libc.so \
 		 $(filter-out $(common-objpfx)linkobj/libc.so, \
diff --git a/elf/tst-ifunc1.c b/elf/tst-ifunc1.c
new file mode 100644
index 0000000..e44dbc3
--- /dev/null
+++ b/elf/tst-ifunc1.c
@@ -0,0 +1,26 @@
+/* Test case for unsafe IFUNC resolver.
+   Copyright (C) 2017 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/>.  */
+
+extern void foo (void);
+
+int
+main (void)
+{
+  foo ();
+  return 0;
+}
diff --git a/elf/tst-ifunc1.sh b/elf/tst-ifunc1.sh
new file mode 100755
index 0000000..8ee3f72
--- /dev/null
+++ b/elf/tst-ifunc1.sh
@@ -0,0 +1,43 @@
+#!/bin/sh
+# A Test case for unsafe IFUNC resolver.
+# Copyright (C) 2017 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/>.
+
+set -e
+
+objpfx=$1; shift
+test_via_rtld_prefix=$1; shift
+test_wrapper_env=$1; shift
+run_program_env=$1; shift
+logfile=${objpfx}tst-ifunc1.out
+
+> $logfile
+fail=0
+
+${test_wrapper_env} \
+${run_program_env} \
+${objpfx}tst-ifunc1 2>&1 || fail=1
+
+if test $fail = 1; then
+  # If it fails to run, check for the expected error from ld.so.
+  fail=0
+  ${test_wrapper_env} \
+  ${run_program_env} \
+  ${objpfx}tst-ifunc1 2>&1 | grep Relink >> $logfile || fail=1
+fi
+
+exit $fail
diff --git a/elf/tst-ifuncmod1a.c b/elf/tst-ifuncmod1a.c
new file mode 100644
index 0000000..5e2183c
--- /dev/null
+++ b/elf/tst-ifuncmod1a.c
@@ -0,0 +1,25 @@
+/* Test case for unsafe IFUNC resolver.
+   Copyright (C) 2017 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/>.  */
+
+extern void bar (void);
+
+void
+foo (void)
+{
+  bar ();
+}
diff --git a/elf/tst-ifuncmod1b.c b/elf/tst-ifuncmod1b.c
new file mode 100644
index 0000000..0c6fe10
--- /dev/null
+++ b/elf/tst-ifuncmod1b.c
@@ -0,0 +1,25 @@
+/* Test case for unsafe IFUNC resolver.
+   Copyright (C) 2017 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/>.  */
+
+extern void xxx (void);
+
+void
+bar (void)
+{
+  xxx ();
+}
diff --git a/elf/tst-ifuncmod1c.c b/elf/tst-ifuncmod1c.c
new file mode 100644
index 0000000..81cd31c
--- /dev/null
+++ b/elf/tst-ifuncmod1c.c
@@ -0,0 +1,27 @@
+/* Test case for unsafe IFUNC resolver.
+   Copyright (C) 2017 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/>.  */
+
+extern void real_xxx (void);
+
+static void *
+xxx_resolver (void)
+{
+  return &real_xxx;
+}
+
+extern void xxx (void) __attribute__ ((ifunc ("xxx_resolver")));
diff --git a/elf/tst-ifuncmod1d.c b/elf/tst-ifuncmod1d.c
new file mode 100644
index 0000000..5715ded
--- /dev/null
+++ b/elf/tst-ifuncmod1d.c
@@ -0,0 +1,24 @@
+/* Test case for unsafe IFUNC resolver.
+   Copyright (C) 2017 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/>.  */
+
+void
+xxx (void)
+{
+}
+
+__typeof (xxx) real_xxx __attribute__ ((alias("xxx")));
diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
index f26a8b2..0c53c70 100644
--- a/sysdeps/generic/ldsodefs.h
+++ b/sysdeps/generic/ldsodefs.h
@@ -1085,6 +1085,32 @@ extern void _dl_non_dynamic_init (void) internal_function;
 extern void _dl_aux_init (ElfW(auxv_t) *av) internal_function;
 
 
+# ifdef STDERR_FILENO
+/* Define here after RTLD_PROGNAME is defined and if STDERR_FILENO is
+   defined.  Since it is unsafe for IFUNC resolver to reference external
+   symbols, issue a fatal error when it happens.  */
+static __always_inline void
+dl_check_ifunc_resolver (struct link_map *sym_map, struct link_map *map,
+			 const ElfW(Sym) *const refsym)
+{
+  if (sym_map != map
+      && sym_map->l_type != lt_executable
+      && !sym_map->l_relocated)
+    {
+      const char *strtab
+	= (const char *) D_PTR (map, l_info[DT_STRTAB]);
+      _dl_fatal_printf ("\
+%s: Relink `%s' with `%s' or place `%s' before `%s' for IFUNC symbol `%s'\n",
+			RTLD_PROGNAME,
+			map->l_libname->name,
+			sym_map->l_libname->name,
+			sym_map->l_libname->name,
+			map->l_libname->name,
+			strtab + refsym->st_name);
+    }
+}
+# endif
+
 __END_DECLS
 
 #endif /* ldsodefs.h */
diff --git a/sysdeps/i386/dl-machine.h b/sysdeps/i386/dl-machine.h
index 6eca69d..3e6b995 100644
--- a/sysdeps/i386/dl-machine.h
+++ b/sysdeps/i386/dl-machine.h
@@ -323,18 +323,7 @@ elf_machine_rel (struct link_map *map, const Elf32_Rel *reloc,
 	  && __builtin_expect (!skip_ifunc, 1))
 	{
 # ifndef RTLD_BOOTSTRAP
-	  if (sym_map != map
-	      && sym_map->l_type != lt_executable
-	      && !sym_map->l_relocated)
-	    {
-	      const char *strtab
-		= (const char *) D_PTR (map, l_info[DT_STRTAB]);
-	      _dl_fatal_printf ("\
-%s: Relink `%s' with `%s' for IFUNC symbol `%s'\n",
-				RTLD_PROGNAME, map->l_name,
-				sym_map->l_name,
-				strtab + refsym->st_name);
-	    }
+	  dl_check_ifunc_resolver (sym_map, map, refsym);
 # endif
 	  value = ((Elf32_Addr (*) (void)) value) ();
 	}
@@ -501,7 +490,12 @@ elf_machine_rela (struct link_map *map, const Elf32_Rela *reloc,
 	  && __builtin_expect (sym->st_shndx != SHN_UNDEF, 1)
 	  && __builtin_expect (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC, 0)
 	  && __builtin_expect (!skip_ifunc, 1))
-	value = ((Elf32_Addr (*) (void)) value) ();
+	{
+# ifndef RESOLVE_CONFLICT_FIND_MAP
+	  dl_check_ifunc_resolver (sym_map, map, refsym);
+# endif
+	  value = ((Elf32_Addr (*) (void)) value) ();
+	}
 
       switch (ELF32_R_TYPE (reloc->r_info))
 	{
diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
index 3e7ae22..5737955 100644
--- a/sysdeps/x86_64/dl-machine.h
+++ b/sysdeps/x86_64/dl-machine.h
@@ -333,18 +333,7 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
 	  && __builtin_expect (!skip_ifunc, 1))
 	{
 # ifndef RTLD_BOOTSTRAP
-	  if (sym_map != map
-	      && sym_map->l_type != lt_executable
-	      && !sym_map->l_relocated)
-	    {
-	      const char *strtab
-		= (const char *) D_PTR (map, l_info[DT_STRTAB]);
-	      _dl_fatal_printf ("\
-%s: Relink `%s' with `%s' for IFUNC symbol `%s'\n",
-				RTLD_PROGNAME, map->l_name,
-				sym_map->l_name,
-				strtab + refsym->st_name);
-	    }
+	  dl_check_ifunc_resolver (sym_map, map, refsym);
 # endif
 	  value = ((ElfW(Addr) (*) (void)) value) ();
 	}
-- 
2.9.3

Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[Khem Raj]

On Fri, Jan 13, 2017 at 12:15 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Jan 13, 2017 at 11:30 AM, Khem Raj <raj.khem@gmail.com> wrote:
>>
>>
>> On 1/13/17 11:03 AM, H.J. Lu wrote:
>>> On Wed, Oct 12, 2016 at 3:19 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Tue, Oct 11, 2016 at 10:45 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>>>>> On 10/05/2016 02:16 PM, H.J. Lu wrote:
>>>>>> On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>>>>>>> On Wed, 5 Oct 2016, H.J. Lu wrote:
>>>>>>>
>>>>>>>> I can try __builtin_memcpy, instread of __builtin_memmove.   There are 2
>>>>>>
>>>>>> I changed it to use __builtin_memset.
>>>>>>
>>>>>>>> acceptable results.  One is ld.so issues an error and the other is program runs.
>>>>>>>> On x86, ld.so issues an error.  I don't know what should happen on others.
>>>>>>>
>>>>>>> You could make the test pass on either of those results (while failing if
>>>>>>> ld.so crashes).
>>>>>>>
>>>>>>
>>>>>> I moved the test to elf.  It passes if the test runs or ld.so issues an
>>>>>> error.  Please try it on arm, powerpc and s390.
>>>>>
>>>>> This is the wrong way to test this.
>>>>>
>>>>> The point of this test is this:
>>>>>
>>>>> - Verify that an unversioned symbol reference in DSO A which has no DT_NEEDED
>>>>>   on DSO B, when resolved to a symbol definition in DSO B, when the symbol in
>>>>>   DSO B is an IFUNC with a resolver, that DSO B is relocated _before_ the IFUNC
>>>>>   resolver is called, because DSO B's resolver might need global data to make
>>>>>   the IFUNC decision e.g. GOT setup.
>>>>>
>>>>> The invariant we want to hold true for IFUNC is that to call the resolver
>>>>> function you must have relocated the DSO which contains the resolver. This _should_
>>>>> have been done by a symbol reocation dependency analysis, but that isn't working
>>>>> correctly IMO or needs deeper analysis in the dynamic loader.
>>>>>
>>>>> The solution we want in place today is to issue some kind of diagnostic until we
>>>>> fix the real problem.
>>>>>
>>>>> The test should look like this:
>>>>>
>>>>> - DSO A with an unversioned symbol reference to 'foo'.
>>>>> - DSO B with a symbol definition of 'foo' as an ifunc with 'foo_resolver' as the
>>>>>   resolver function which references global data from DSO C to decide which of
>>>>>   two functions to return.
>>>>> - DSO C with global data set to a value.
>>>>>
>>>>> The point is that DSO B depends on DSO C and has DT_NEEDED on it, so C will get
>>>>> relocated first, then B, such that B's GOT is setup to access C's global data.
>>>>>
>>>>> When handling the reference to 'foo' in DSO A we should on x86_64 and i686
>>>>> get the error about needing to relink DSO A so it depends on DSO B, to form
>>>>> the initialization order of C->B->A.
>>>>>
>>>>> I expect this test case will now crash the other arches, rather than just
>>>>> avoiding the crash by relying on internal libc.so details about which ifuncs
>>>>> you're using.
>>>>>
>>>>> This is one step towards a better definition of IFUNC semantics, which need to
>>>>> be more clearly defined (something I wish I had time to define and fix so
>>>>> more projects could use them).
>>>>
>>>> IFUNC resolver can fail for various reasons.  My goal is to make sure
>>>> that IFUNC inside of glibc works correctly or an error message is given
>>>> when glibc isn't used properly.  In case of x86,  CPU feature info is
>>>> retrieved and stored in ld.so very early at startup, which is used by IFUNC
>>>> and only accessible in libc.so and libm.so after they have been relocated.
>>>> My change in x86 ld.so checks it and my test verifies the check.  My fix
>>>> won't cover other possible IFUNC failures.
>>>>
>>>
>>> When the IFUNC relocation is performed before the providing shared
>>> library is unrelocated, the returned function address will be 0 and
>>> program will segfault when the function is called.
>>>
>>> Please apply this patch and run the test if your platform has IFUNC.  I only
>>> enabled the unsafe resolver check for i386 and x86-64.  It is straightforward
>>> to add check for other platforms.
>>
>> I will test it out shortly. One thing I see, the runner script for test
>> is calling out for /bin/bash and the script does not use any bash
>> extentions perhaps using /bin/sh is enough.
>>
>
> Here is the updated patch with some fixes.

This still failed on 32bit in same way.

>
> --
> H.J.

Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[H.J. Lu]

On Thu, Jan 19, 2017 at 10:42 AM, Khem Raj <raj.khem@gmail.com> wrote:
> On Fri, Jan 13, 2017 at 12:15 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Fri, Jan 13, 2017 at 11:30 AM, Khem Raj <raj.khem@gmail.com> wrote:
>>>
>>>
>>> On 1/13/17 11:03 AM, H.J. Lu wrote:
>>>> On Wed, Oct 12, 2016 at 3:19 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Tue, Oct 11, 2016 at 10:45 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>>>>>> On 10/05/2016 02:16 PM, H.J. Lu wrote:
>>>>>>> On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>>>>>>>> On Wed, 5 Oct 2016, H.J. Lu wrote:
>>>>>>>>
>>>>>>>>> I can try __builtin_memcpy, instread of __builtin_memmove.   There are 2
>>>>>>>
>>>>>>> I changed it to use __builtin_memset.
>>>>>>>
>>>>>>>>> acceptable results.  One is ld.so issues an error and the other is program runs.
>>>>>>>>> On x86, ld.so issues an error.  I don't know what should happen on others.
>>>>>>>>
>>>>>>>> You could make the test pass on either of those results (while failing if
>>>>>>>> ld.so crashes).
>>>>>>>>
>>>>>>>
>>>>>>> I moved the test to elf.  It passes if the test runs or ld.so issues an
>>>>>>> error.  Please try it on arm, powerpc and s390.
>>>>>>
>>>>>> This is the wrong way to test this.
>>>>>>
>>>>>> The point of this test is this:
>>>>>>
>>>>>> - Verify that an unversioned symbol reference in DSO A which has no DT_NEEDED
>>>>>>   on DSO B, when resolved to a symbol definition in DSO B, when the symbol in
>>>>>>   DSO B is an IFUNC with a resolver, that DSO B is relocated _before_ the IFUNC
>>>>>>   resolver is called, because DSO B's resolver might need global data to make
>>>>>>   the IFUNC decision e.g. GOT setup.
>>>>>>
>>>>>> The invariant we want to hold true for IFUNC is that to call the resolver
>>>>>> function you must have relocated the DSO which contains the resolver. This _should_
>>>>>> have been done by a symbol reocation dependency analysis, but that isn't working
>>>>>> correctly IMO or needs deeper analysis in the dynamic loader.
>>>>>>
>>>>>> The solution we want in place today is to issue some kind of diagnostic until we
>>>>>> fix the real problem.
>>>>>>
>>>>>> The test should look like this:
>>>>>>
>>>>>> - DSO A with an unversioned symbol reference to 'foo'.
>>>>>> - DSO B with a symbol definition of 'foo' as an ifunc with 'foo_resolver' as the
>>>>>>   resolver function which references global data from DSO C to decide which of
>>>>>>   two functions to return.
>>>>>> - DSO C with global data set to a value.
>>>>>>
>>>>>> The point is that DSO B depends on DSO C and has DT_NEEDED on it, so C will get
>>>>>> relocated first, then B, such that B's GOT is setup to access C's global data.
>>>>>>
>>>>>> When handling the reference to 'foo' in DSO A we should on x86_64 and i686
>>>>>> get the error about needing to relink DSO A so it depends on DSO B, to form
>>>>>> the initialization order of C->B->A.
>>>>>>
>>>>>> I expect this test case will now crash the other arches, rather than just
>>>>>> avoiding the crash by relying on internal libc.so details about which ifuncs
>>>>>> you're using.
>>>>>>
>>>>>> This is one step towards a better definition of IFUNC semantics, which need to
>>>>>> be more clearly defined (something I wish I had time to define and fix so
>>>>>> more projects could use them).
>>>>>
>>>>> IFUNC resolver can fail for various reasons.  My goal is to make sure
>>>>> that IFUNC inside of glibc works correctly or an error message is given
>>>>> when glibc isn't used properly.  In case of x86,  CPU feature info is
>>>>> retrieved and stored in ld.so very early at startup, which is used by IFUNC
>>>>> and only accessible in libc.so and libm.so after they have been relocated.
>>>>> My change in x86 ld.so checks it and my test verifies the check.  My fix
>>>>> won't cover other possible IFUNC failures.
>>>>>
>>>>
>>>> When the IFUNC relocation is performed before the providing shared
>>>> library is unrelocated, the returned function address will be 0 and
>>>> program will segfault when the function is called.
>>>>
>>>> Please apply this patch and run the test if your platform has IFUNC.  I only
>>>> enabled the unsafe resolver check for i386 and x86-64.  It is straightforward
>>>> to add check for other platforms.
>>>
>>> I will test it out shortly. One thing I see, the runner script for test
>>> is calling out for /bin/bash and the script does not use any bash
>>> extentions perhaps using /bin/sh is enough.
>>>
>>
>> Here is the updated patch with some fixes.
>
> This still failed on 32bit in same way.
>

Did you get segfault?



-- 
H.J.

Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[Khem Raj]

On Fri, Jan 20, 2017 at 9:00 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Jan 19, 2017 at 10:42 AM, Khem Raj <raj.khem@gmail.com> wrote:
>> On Fri, Jan 13, 2017 at 12:15 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Fri, Jan 13, 2017 at 11:30 AM, Khem Raj <raj.khem@gmail.com> wrote:
>>>>
>>>>
>>>> On 1/13/17 11:03 AM, H.J. Lu wrote:
>>>>> On Wed, Oct 12, 2016 at 3:19 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>> On Tue, Oct 11, 2016 at 10:45 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>>>>>>> On 10/05/2016 02:16 PM, H.J. Lu wrote:
>>>>>>>> On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>>>>>>>>> On Wed, 5 Oct 2016, H.J. Lu wrote:
>>>>>>>>>
>>>>>>>>>> I can try __builtin_memcpy, instread of __builtin_memmove.   There are 2
>>>>>>>>
>>>>>>>> I changed it to use __builtin_memset.
>>>>>>>>
>>>>>>>>>> acceptable results.  One is ld.so issues an error and the other is program runs.
>>>>>>>>>> On x86, ld.so issues an error.  I don't know what should happen on others.
>>>>>>>>>
>>>>>>>>> You could make the test pass on either of those results (while failing if
>>>>>>>>> ld.so crashes).
>>>>>>>>>
>>>>>>>>
>>>>>>>> I moved the test to elf.  It passes if the test runs or ld.so issues an
>>>>>>>> error.  Please try it on arm, powerpc and s390.
>>>>>>>
>>>>>>> This is the wrong way to test this.
>>>>>>>
>>>>>>> The point of this test is this:
>>>>>>>
>>>>>>> - Verify that an unversioned symbol reference in DSO A which has no DT_NEEDED
>>>>>>>   on DSO B, when resolved to a symbol definition in DSO B, when the symbol in
>>>>>>>   DSO B is an IFUNC with a resolver, that DSO B is relocated _before_ the IFUNC
>>>>>>>   resolver is called, because DSO B's resolver might need global data to make
>>>>>>>   the IFUNC decision e.g. GOT setup.
>>>>>>>
>>>>>>> The invariant we want to hold true for IFUNC is that to call the resolver
>>>>>>> function you must have relocated the DSO which contains the resolver. This _should_
>>>>>>> have been done by a symbol reocation dependency analysis, but that isn't working
>>>>>>> correctly IMO or needs deeper analysis in the dynamic loader.
>>>>>>>
>>>>>>> The solution we want in place today is to issue some kind of diagnostic until we
>>>>>>> fix the real problem.
>>>>>>>
>>>>>>> The test should look like this:
>>>>>>>
>>>>>>> - DSO A with an unversioned symbol reference to 'foo'.
>>>>>>> - DSO B with a symbol definition of 'foo' as an ifunc with 'foo_resolver' as the
>>>>>>>   resolver function which references global data from DSO C to decide which of
>>>>>>>   two functions to return.
>>>>>>> - DSO C with global data set to a value.
>>>>>>>
>>>>>>> The point is that DSO B depends on DSO C and has DT_NEEDED on it, so C will get
>>>>>>> relocated first, then B, such that B's GOT is setup to access C's global data.
>>>>>>>
>>>>>>> When handling the reference to 'foo' in DSO A we should on x86_64 and i686
>>>>>>> get the error about needing to relink DSO A so it depends on DSO B, to form
>>>>>>> the initialization order of C->B->A.
>>>>>>>
>>>>>>> I expect this test case will now crash the other arches, rather than just
>>>>>>> avoiding the crash by relying on internal libc.so details about which ifuncs
>>>>>>> you're using.
>>>>>>>
>>>>>>> This is one step towards a better definition of IFUNC semantics, which need to
>>>>>>> be more clearly defined (something I wish I had time to define and fix so
>>>>>>> more projects could use them).
>>>>>>
>>>>>> IFUNC resolver can fail for various reasons.  My goal is to make sure
>>>>>> that IFUNC inside of glibc works correctly or an error message is given
>>>>>> when glibc isn't used properly.  In case of x86,  CPU feature info is
>>>>>> retrieved and stored in ld.so very early at startup, which is used by IFUNC
>>>>>> and only accessible in libc.so and libm.so after they have been relocated.
>>>>>> My change in x86 ld.so checks it and my test verifies the check.  My fix
>>>>>> won't cover other possible IFUNC failures.
>>>>>>
>>>>>
>>>>> When the IFUNC relocation is performed before the providing shared
>>>>> library is unrelocated, the returned function address will be 0 and
>>>>> program will segfault when the function is called.
>>>>>
>>>>> Please apply this patch and run the test if your platform has IFUNC.  I only
>>>>> enabled the unsafe resolver check for i386 and x86-64.  It is straightforward
>>>>> to add check for other platforms.
>>>>
>>>> I will test it out shortly. One thing I see, the runner script for test
>>>> is calling out for /bin/bash and the script does not use any bash
>>>> extentions perhaps using /bin/sh is enough.
>>>>
>>>
>>> Here is the updated patch with some fixes.
>>
>> This still failed on 32bit in same way.
>>
>
> Did you get segfault?

No, I was testing it for https://sourceware.org/bugzilla/show_bug.cgi?id=21041
I am getting same ldso IFUNC messages

Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[H.J. Lu]

On Fri, Jan 20, 2017 at 10:35 AM, Khem Raj <raj.khem@gmail.com> wrote:
> On Fri, Jan 20, 2017 at 9:00 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Thu, Jan 19, 2017 at 10:42 AM, Khem Raj <raj.khem@gmail.com> wrote:
>>> On Fri, Jan 13, 2017 at 12:15 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>> On Fri, Jan 13, 2017 at 11:30 AM, Khem Raj <raj.khem@gmail.com> wrote:
>>>>>
>>>>>
>>>>> On 1/13/17 11:03 AM, H.J. Lu wrote:
>>>>>> On Wed, Oct 12, 2016 at 3:19 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>> On Tue, Oct 11, 2016 at 10:45 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>>>>>>>> On 10/05/2016 02:16 PM, H.J. Lu wrote:
>>>>>>>>> On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>>>>>>>>>> On Wed, 5 Oct 2016, H.J. Lu wrote:
>>>>>>>>>>
>>>>>>>>>>> I can try __builtin_memcpy, instread of __builtin_memmove.   There are 2
>>>>>>>>>
>>>>>>>>> I changed it to use __builtin_memset.
>>>>>>>>>
>>>>>>>>>>> acceptable results.  One is ld.so issues an error and the other is program runs.
>>>>>>>>>>> On x86, ld.so issues an error.  I don't know what should happen on others.
>>>>>>>>>>
>>>>>>>>>> You could make the test pass on either of those results (while failing if
>>>>>>>>>> ld.so crashes).
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I moved the test to elf.  It passes if the test runs or ld.so issues an
>>>>>>>>> error.  Please try it on arm, powerpc and s390.
>>>>>>>>
>>>>>>>> This is the wrong way to test this.
>>>>>>>>
>>>>>>>> The point of this test is this:
>>>>>>>>
>>>>>>>> - Verify that an unversioned symbol reference in DSO A which has no DT_NEEDED
>>>>>>>>   on DSO B, when resolved to a symbol definition in DSO B, when the symbol in
>>>>>>>>   DSO B is an IFUNC with a resolver, that DSO B is relocated _before_ the IFUNC
>>>>>>>>   resolver is called, because DSO B's resolver might need global data to make
>>>>>>>>   the IFUNC decision e.g. GOT setup.
>>>>>>>>
>>>>>>>> The invariant we want to hold true for IFUNC is that to call the resolver
>>>>>>>> function you must have relocated the DSO which contains the resolver. This _should_
>>>>>>>> have been done by a symbol reocation dependency analysis, but that isn't working
>>>>>>>> correctly IMO or needs deeper analysis in the dynamic loader.
>>>>>>>>
>>>>>>>> The solution we want in place today is to issue some kind of diagnostic until we
>>>>>>>> fix the real problem.
>>>>>>>>
>>>>>>>> The test should look like this:
>>>>>>>>
>>>>>>>> - DSO A with an unversioned symbol reference to 'foo'.
>>>>>>>> - DSO B with a symbol definition of 'foo' as an ifunc with 'foo_resolver' as the
>>>>>>>>   resolver function which references global data from DSO C to decide which of
>>>>>>>>   two functions to return.
>>>>>>>> - DSO C with global data set to a value.
>>>>>>>>
>>>>>>>> The point is that DSO B depends on DSO C and has DT_NEEDED on it, so C will get
>>>>>>>> relocated first, then B, such that B's GOT is setup to access C's global data.
>>>>>>>>
>>>>>>>> When handling the reference to 'foo' in DSO A we should on x86_64 and i686
>>>>>>>> get the error about needing to relink DSO A so it depends on DSO B, to form
>>>>>>>> the initialization order of C->B->A.
>>>>>>>>
>>>>>>>> I expect this test case will now crash the other arches, rather than just
>>>>>>>> avoiding the crash by relying on internal libc.so details about which ifuncs
>>>>>>>> you're using.
>>>>>>>>
>>>>>>>> This is one step towards a better definition of IFUNC semantics, which need to
>>>>>>>> be more clearly defined (something I wish I had time to define and fix so
>>>>>>>> more projects could use them).
>>>>>>>
>>>>>>> IFUNC resolver can fail for various reasons.  My goal is to make sure
>>>>>>> that IFUNC inside of glibc works correctly or an error message is given
>>>>>>> when glibc isn't used properly.  In case of x86,  CPU feature info is
>>>>>>> retrieved and stored in ld.so very early at startup, which is used by IFUNC
>>>>>>> and only accessible in libc.so and libm.so after they have been relocated.
>>>>>>> My change in x86 ld.so checks it and my test verifies the check.  My fix
>>>>>>> won't cover other possible IFUNC failures.
>>>>>>>
>>>>>>
>>>>>> When the IFUNC relocation is performed before the providing shared
>>>>>> library is unrelocated, the returned function address will be 0 and
>>>>>> program will segfault when the function is called.
>>>>>>
>>>>>> Please apply this patch and run the test if your platform has IFUNC.  I only
>>>>>> enabled the unsafe resolver check for i386 and x86-64.  It is straightforward
>>>>>> to add check for other platforms.
>>>>>
>>>>> I will test it out shortly. One thing I see, the runner script for test
>>>>> is calling out for /bin/bash and the script does not use any bash
>>>>> extentions perhaps using /bin/sh is enough.
>>>>>
>>>>
>>>> Here is the updated patch with some fixes.
>>>
>>> This still failed on 32bit in same way.
>>>
>>
>> Did you get segfault?
>
> No, I was testing it for https://sourceware.org/bugzilla/show_bug.cgi?id=21041
> I am getting same ldso IFUNC messages

I updated ld.so for i686 and x86-64 so that ld.so issues an error message,
instead of segfautl. Have you tested it on non-x86 machines?

-- 
H.J.

Re: [PATCH 2/2] Add an x86 IFUNC testcase for [BZ #20019]

[Khem Raj]

On Fri, Jan 20, 2017 at 11:02 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, Jan 20, 2017 at 10:35 AM, Khem Raj <raj.khem@gmail.com> wrote:
>> On Fri, Jan 20, 2017 at 9:00 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Thu, Jan 19, 2017 at 10:42 AM, Khem Raj <raj.khem@gmail.com> wrote:
>>>> On Fri, Jan 13, 2017 at 12:15 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>> On Fri, Jan 13, 2017 at 11:30 AM, Khem Raj <raj.khem@gmail.com> wrote:
>>>>>>
>>>>>>
>>>>>> On 1/13/17 11:03 AM, H.J. Lu wrote:
>>>>>>> On Wed, Oct 12, 2016 at 3:19 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>> On Tue, Oct 11, 2016 at 10:45 PM, Carlos O'Donell <carlos@redhat.com> wrote:
>>>>>>>>> On 10/05/2016 02:16 PM, H.J. Lu wrote:
>>>>>>>>>> On Tue, Oct 4, 2016 at 5:09 PM, Joseph Myers <joseph@codesourcery.com> wrote:
>>>>>>>>>>> On Wed, 5 Oct 2016, H.J. Lu wrote:
>>>>>>>>>>>
>>>>>>>>>>>> I can try __builtin_memcpy, instread of __builtin_memmove.   There are 2
>>>>>>>>>>
>>>>>>>>>> I changed it to use __builtin_memset.
>>>>>>>>>>
>>>>>>>>>>>> acceptable results.  One is ld.so issues an error and the other is program runs.
>>>>>>>>>>>> On x86, ld.so issues an error.  I don't know what should happen on others.
>>>>>>>>>>>
>>>>>>>>>>> You could make the test pass on either of those results (while failing if
>>>>>>>>>>> ld.so crashes).
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I moved the test to elf.  It passes if the test runs or ld.so issues an
>>>>>>>>>> error.  Please try it on arm, powerpc and s390.
>>>>>>>>>
>>>>>>>>> This is the wrong way to test this.
>>>>>>>>>
>>>>>>>>> The point of this test is this:
>>>>>>>>>
>>>>>>>>> - Verify that an unversioned symbol reference in DSO A which has no DT_NEEDED
>>>>>>>>>   on DSO B, when resolved to a symbol definition in DSO B, when the symbol in
>>>>>>>>>   DSO B is an IFUNC with a resolver, that DSO B is relocated _before_ the IFUNC
>>>>>>>>>   resolver is called, because DSO B's resolver might need global data to make
>>>>>>>>>   the IFUNC decision e.g. GOT setup.
>>>>>>>>>
>>>>>>>>> The invariant we want to hold true for IFUNC is that to call the resolver
>>>>>>>>> function you must have relocated the DSO which contains the resolver. This _should_
>>>>>>>>> have been done by a symbol reocation dependency analysis, but that isn't working
>>>>>>>>> correctly IMO or needs deeper analysis in the dynamic loader.
>>>>>>>>>
>>>>>>>>> The solution we want in place today is to issue some kind of diagnostic until we
>>>>>>>>> fix the real problem.
>>>>>>>>>
>>>>>>>>> The test should look like this:
>>>>>>>>>
>>>>>>>>> - DSO A with an unversioned symbol reference to 'foo'.
>>>>>>>>> - DSO B with a symbol definition of 'foo' as an ifunc with 'foo_resolver' as the
>>>>>>>>>   resolver function which references global data from DSO C to decide which of
>>>>>>>>>   two functions to return.
>>>>>>>>> - DSO C with global data set to a value.
>>>>>>>>>
>>>>>>>>> The point is that DSO B depends on DSO C and has DT_NEEDED on it, so C will get
>>>>>>>>> relocated first, then B, such that B's GOT is setup to access C's global data.
>>>>>>>>>
>>>>>>>>> When handling the reference to 'foo' in DSO A we should on x86_64 and i686
>>>>>>>>> get the error about needing to relink DSO A so it depends on DSO B, to form
>>>>>>>>> the initialization order of C->B->A.
>>>>>>>>>
>>>>>>>>> I expect this test case will now crash the other arches, rather than just
>>>>>>>>> avoiding the crash by relying on internal libc.so details about which ifuncs
>>>>>>>>> you're using.
>>>>>>>>>
>>>>>>>>> This is one step towards a better definition of IFUNC semantics, which need to
>>>>>>>>> be more clearly defined (something I wish I had time to define and fix so
>>>>>>>>> more projects could use them).
>>>>>>>>
>>>>>>>> IFUNC resolver can fail for various reasons.  My goal is to make sure
>>>>>>>> that IFUNC inside of glibc works correctly or an error message is given
>>>>>>>> when glibc isn't used properly.  In case of x86,  CPU feature info is
>>>>>>>> retrieved and stored in ld.so very early at startup, which is used by IFUNC
>>>>>>>> and only accessible in libc.so and libm.so after they have been relocated.
>>>>>>>> My change in x86 ld.so checks it and my test verifies the check.  My fix
>>>>>>>> won't cover other possible IFUNC failures.
>>>>>>>>
>>>>>>>
>>>>>>> When the IFUNC relocation is performed before the providing shared
>>>>>>> library is unrelocated, the returned function address will be 0 and
>>>>>>> program will segfault when the function is called.
>>>>>>>
>>>>>>> Please apply this patch and run the test if your platform has IFUNC.  I only
>>>>>>> enabled the unsafe resolver check for i386 and x86-64.  It is straightforward
>>>>>>> to add check for other platforms.
>>>>>>
>>>>>> I will test it out shortly. One thing I see, the runner script for test
>>>>>> is calling out for /bin/bash and the script does not use any bash
>>>>>> extentions perhaps using /bin/sh is enough.
>>>>>>
>>>>>
>>>>> Here is the updated patch with some fixes.
>>>>
>>>> This still failed on 32bit in same way.
>>>>
>>>
>>> Did you get segfault?
>>
>> No, I was testing it for https://sourceware.org/bugzilla/show_bug.cgi?id=21041
>> I am getting same ldso IFUNC messages
>
> I updated ld.so for i686 and x86-64 so that ld.so issues an error message,
> instead of segfautl. Have you tested it on non-x86 machines?

I think its working ok on others. I do not see segfaults

>
> --
> H.J.