| Message ID | CAMe9rOpfYBeM+kmL38zVgrPa-b-hk2rOLd7Rf_8hRPXPphJbWw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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.
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.
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
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.
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.
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