[Bug libc/11120] strncmp() will be broken if used in ld.so on x86-64

[Bug libc/11120] strncmp() will be broken if used in ld.so on x86-64

[dhatch at ilm dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=11120

Don Hatch <dhatch at ilm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dhatch at ilm dot com

--- Comment #2 from Don Hatch <dhatch at ilm dot com> 2013-03-21 06:42:25 UTC ---
For reference, the change was:

commit f69190e74a081f0a35906ff0b9a8dc24e42341e8
Author: Ulrich Drepper <drepper@redhat.com>
Date:   Thu Jan 14 08:09:32 2010 -0800

    Prevent silent errors should x86-64 strncmp be needed outside libc.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/11120] strncmp() will be broken if used in ld.so on x86-64

[ldv at altlinux dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=11120

Dmitry V. Levin <ldv at altlinux dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |ASSIGNED
                 CC|                            |ldv at altlinux dot org
         AssignedTo|unassigned at sourceware    |ldv at altlinux dot org
                   |dot org                     |

--- Comment #4 from Dmitry V. Levin <ldv at altlinux dot org> 2013-03-21 23:42:42 UTC ---
I'll submit a patch shortly.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/11120] strncmp() will be broken if used in ld.so on x86-64

[ldv at altlinux dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=11120

Dmitry V. Levin <ldv at altlinux dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |
         AssignedTo|drepper.fsp at gmail dot    |unassigned at sourceware
                   |com                         |dot org

--- Comment #3 from Dmitry V. Levin <ldv at altlinux dot org> 2013-03-21 23:42:04 UTC ---
Due to a typo repeated several times, this bug hasn't been fixed yet,
despite being closed for more than two years.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/11120] strncmp() will be broken if used in ld.so on x86-64

[dhatch at ilm dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=11120

--- Comment #5 from Don Hatch <dhatch at ilm dot com> 2013-03-22 02:48:03 UTC ---
Interesting!
So what was Ulrich's intent in that commit--
to fix the function,
or to make it abort if called from inside ld.so?
I couldn't tell from the commit message
or bug closure comment.

The reason I started looking at this bug
is I was noticing flakiness of getenv() when called inside ld.so;
then when I tried to implement my own my_getenv as a workaround,
I ran into the fact that strncmp() is flaky too
(which is why getenv is flaky-- it uses strncmp),
and I had to implement a my_strncmp as well.

This is really hard to nail down--
sometimes strncmp("abc", "ab", 2) returns 0, and sometimes it returns 1.
It seems consistent within a given compile,
but other than that I don't see much rhyme or reason to it.

I can reproduce the inconsistency by sticking the following
at the top of the _dl_sort_fini function body in elf/dl-fini.c:

    #define PRINT_STRNCMP(a,b,len) _dl_debug_printf("strncmp(\"%s\", \"%s\",
%u) returned %u\n", (a), (b), (int)(len), (int)strncmp((a), (b), (len)));
    PRINT_STRNCMP("abc", "ab", 2); // prints correct result 0
    char abc[] = {'a','b','c','\0'};
    char ab[] = {'a','b','\0'};
    PRINT_STRNCMP(abc, ab, 2);     // prints incorrect result 1

It prints something like:
     30747:     strncmp("abc", "ab", 2) = 0
     30747:     strncmp("abc", "ab", 2) = 1

I was about to open a new bug report,
but at this point I'm not sure whether that would be a dup of this one?

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/11120] strncmp() will be broken if used in ld.so on x86-64

[ldv at altlinux dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=11120

Dmitry V. Levin <ldv at altlinux dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #6 from Dmitry V. Levin <ldv at altlinux dot org> 2013-03-22 03:30:49 UTC ---
(In reply to comment #5)
> Interesting!
> So what was Ulrich's intent in that commit--
> to fix the function,
> or to make it abort if called from inside ld.so?
> I couldn't tell from the commit message
> or bug closure comment.

The commit message says:
"Prevent silent errors should x86-64 strncmp be needed outside libc"
The intent clearly was to make build fail.

The bug is fixed finally:
http://sourceware.org/git/?p=glibc.git;a=commitdiff;h=glibc-2.17-458-g2e0fb52

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/11120] strncmp() will be broken if used in ld.so on x86-64

[ldv at altlinux dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=11120

--- Comment #7 from Dmitry V. Levin <ldv at altlinux dot org> 2013-03-22 03:37:26 UTC ---
(In reply to comment #5)
> The reason I started looking at this bug
> is I was noticing flakiness of getenv() when called inside ld.so;

getenv() is not normally called inside rtld.
Are you talking about some patched rtld?

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/11120] strncmp() will be broken if used in ld.so on x86-64

[dhatch at ilm dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=11120

--- Comment #8 from Don Hatch <dhatch at ilm dot com> 2013-03-22 04:03:34 UTC ---
> The commit message says:
> "Prevent silent errors should x86-64 strncmp be needed outside libc"
> The intent clearly was to make build fail.

That commit message reads as completely ambiguous to me,
so thank you for the clarification.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/11120] strncmp() will be broken if used in ld.so on x86-64

[dhatch at ilm dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=11120

--- Comment #9 from Don Hatch <dhatch at ilm dot com> 2013-03-22 04:30:08 UTC ---
(In reply to comment #7)
> (In reply to comment #5)
> > The reason I started looking at this bug
> > is I was noticing flakiness of getenv() when called inside ld.so;
> 
> getenv() is not normally called inside rtld.
> Are you talking about some patched rtld?

I'm using it for debugging.
(There are also uses of getenv and strncmp throughout the elf subdirectory
which I mistakenly thought were part of rtld... apparently not).

Thanks for the fix.  It will prevent a lot of further wasted time.

Can you comment on the fact that, prior to your latest change
which now prevents calling it altogether,
strncmp was returning inconsistent results?
This is contrary to what I would expect from Mark Seaborn's original
description "If strncmp() is ever used in ld.so on x86-64 it will
silently be implemented as strcmp()".

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/11120] strncmp() will be broken if used in ld.so on x86-64

[ldv at altlinux dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=11120

--- Comment #10 from Dmitry V. Levin <ldv at altlinux dot org> 2013-03-22 12:00:54 UTC ---
(In reply to comment #9)
> Can you comment on the fact that, prior to your latest change
> which now prevents calling it altogether,
> strncmp was returning inconsistent results?
> This is contrary to what I would expect from Mark Seaborn's original
> description "If strncmp() is ever used in ld.so on x86-64 it will
> silently be implemented as strcmp()".

There is no code in rtld that directly or indirectly calls strncmp.
The change I committed is fixed safeguards that in case rtld is changed to use
strncmp, x86-64 build would fail until strncmp is implemented for rtld.

It's certainly better to let rtld build fail rather than allow strncmp to be
implemented as strcmp.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/11120] strncmp() will be broken if used in ld.so on x86-64

[dhatch at ilm dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=11120

--- Comment #11 from Don Hatch <dhatch at ilm dot com> 2013-03-22 22:21:35 UTC ---
(In reply to comment #10)
> (In reply to comment #9)
> > Can you comment on the fact that, prior to your latest change
> > which now prevents calling it altogether,
> > strncmp was returning inconsistent results?
> > This is contrary to what I would expect from Mark Seaborn's original
> > description "If strncmp() is ever used in ld.so on x86-64 it will
> > silently be implemented as strcmp()".
> 
> There is no code in rtld that directly or indirectly calls strncmp.
> The change I committed is fixed safeguards that in case rtld is changed to use
> strncmp, x86-64 build would fail until strncmp is implemented for rtld.
> 
> It's certainly better to let rtld build fail rather than allow strncmp to be
> implemented as strcmp.

Hi Dmitry,

I understand, and I completely agree with you,
as I've tried to say already.
There is absolutely no disagreement or question there.
After your latest change, I observe that any attempt to call strncmp directly,
or indirectly (e.g. by attempting to call getenv), fails at compile time.
That's a very good thing.  Thank you.

However, you have not answered my question.

My question is about the fact that,
contrary to what both you and Mark Seaborn have said,
prior to your change,
strncmp was *not* acting like strcmp, at least not consistently.
On one call it acted like strcmp, on the next call
(with exactly the same effective input) it didn't.
For a simple example of this,
please look carefully at my test case above (in my note of 2013-03-22
02:48:03).

Is this a surprise or not?
My concern is that your disabling of the function inside rtld
may be pushing more mistakes or rottenness under the rug.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/11120] strncmp() will be broken if used in ld.so on x86-64

[ldv at altlinux dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=11120

--- Comment #12 from Dmitry V. Levin <ldv at altlinux dot org> 2013-03-22 23:00:44 UTC ---
(In reply to comment #11)
> There is absolutely no disagreement or question there.
> After your latest change, I observe that any attempt to call strncmp directly,
> or indirectly (e.g. by attempting to call getenv), fails at compile time.
> That's a very good thing.  Thank you.
> 
> However, you have not answered my question.
> 
> My question is about the fact that,
> contrary to what both you and Mark Seaborn have said,
> prior to your change,
> strncmp was *not* acting like strcmp, at least not consistently.
> On one call it acted like strcmp, on the next call
> (with exactly the same effective input) it didn't.

If you have a look at the implementation, you'll see that it's all the same in
the non-libc case no matter by what name it is called.
The reason why you can see different results is that gcc is capable to optimize
out some strncmp calls.  For example, in your case

$ echo -e '#include <string.h>\nint foo(void){return strncmp("abc", "ab", 2);}'
|gcc -O2 -E - |tail -1
int foo(void){return (__extension__ (__builtin_constant_p (2) &&
((__builtin_constant_p ("abc") && strlen ("abc") < ((size_t) (2))) ||
(__builtin_constant_p ("ab") && strlen ("ab") < ((size_t) (2)))) ?
__extension__ ({ size_t __s1_len, __s2_len; (__builtin_constant_p ("abc") &&
__builtin_constant_p ("ab") && (__s1_len = strlen ("abc"), __s2_len = strlen
("ab"), (!((size_t)(const void *)(("abc") + 1) - (size_t)(const void *)("abc")
== 1) || __s1_len >= 4) && (!((size_t)(const void *)(("ab") + 1) -
(size_t)(const void *)("ab") == 1) || __s2_len >= 4)) ? __builtin_strcmp
("abc", "ab") : (__builtin_constant_p ("abc") && ((size_t)(const void
*)(("abc") + 1) - (size_t)(const void *)("abc") == 1) && (__s1_len = strlen
("abc"), __s1_len < 4) ? (__builtin_constant_p ("ab") && ((size_t)(const void
*)(("ab") + 1) - (size_t)(const void *)("ab") == 1) ? __builtin_strcmp ("abc",
"ab") : (__extension__ ({ const unsigned char *__s2 = (const unsigned char *)
(const char *) ("ab"); register int __result = (((const unsigned char *) (const
char *) ("abc"))[0] - __s2[0]); if (__s1_len > 0 && __result == 0) { __result =
(((const unsigned char *) (const char *) ("abc"))[1] - __s2[1]); if (__s1_len >
1 && __result == 0) { __result = (((const unsigned char *) (const char *)
("abc"))[2] - __s2[2]); if (__s1_len > 2 && __result == 0) __result = (((const
unsigned char *) (const char *) ("abc"))[3] - __s2[3]); } } __result; }))) :
(__builtin_constant_p ("ab") && ((size_t)(const void *)(("ab") + 1) -
(size_t)(const void *)("ab") == 1) && (__s2_len = strlen ("ab"), __s2_len < 4)
? (__builtin_constant_p ("abc") && ((size_t)(const void *)(("abc") + 1) -
(size_t)(const void *)("abc") == 1) ? __builtin_strcmp ("abc", "ab") :
(__extension__ ({ const unsigned char *__s1 = (const unsigned char *) (const
char *) ("abc"); register int __result = __s1[0] - ((const unsigned char *)
(const char *) ("ab"))[0]; if (__s2_len > 0 && __result == 0) { __result =
(__s1[1] - ((const unsigned char *) (const char *) ("ab"))[1]); if (__s2_len >
1 && __result == 0) { __result = (__s1[2] - ((const unsigned char *) (const
char *) ("ab"))[2]); if (__s2_len > 2 && __result == 0) __result = (__s1[3] -
((const unsigned char *) (const char *) ("ab"))[3]); } } __result; }))) :
__builtin_strcmp ("abc", "ab")))); }) : strncmp ("abc", "ab", 2)));}

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/11120] strncmp() will be broken if used in ld.so on x86-64

[dhatch at ilm dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=11120

--- Comment #13 from Don Hatch <dhatch at ilm dot com> 2013-03-26 05:43:37 UTC ---
(In reply to comment #12)
> (In reply to comment #11)
> > 
> > My question is about the fact that,
> > contrary to what both you and Mark Seaborn have said,
> > prior to your change,
> > strncmp was *not* acting like strcmp, at least not consistently.
> > On one call it acted like strcmp, on the next call
> > (with exactly the same effective input) it didn't.
> 
> If you have a look at the implementation, you'll see that it's all the same in
> the non-libc case no matter by what name it is called.
> The reason why you can see different results is that gcc is capable to optimize
> out some strncmp calls.  For example, in your case
> 
> $ echo -e '#include <string.h>\nint foo(void){return strncmp("abc", "ab", 2);}'
> |gcc -O2 -E - |tail -1

(in tcsh, this needs to say /bin/echo so it will understand the -e flag)

Ah!  Thank you, that makes perfect sense to me now.
It must be that __builtin_constant_p (abc) is returning false
but __builtin_constant_p("abc") is returning true,
resulting in two different execution paths taken, one of which actually
calls the function and the other doesn't.

Thanks very much for taking the time to examine and explain this.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug libc/11120] strncmp() will be broken if used in ld.so on x86-64

[federicosacerdoti at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=11120

Federico Sacerdoti <federicosacerdoti at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |federicosacerdoti at gmail dot com

--- Comment #14 from Federico Sacerdoti <federicosacerdoti at gmail dot com> ---
Glad to find this, saved quite a bit of head-scratching. Below is my
implementation of getenv that doesnt use strncmp. 

<pre>
static
char* getenv2(char **envp, const char* name)
{
  char **ep;
  size_t namelen = strlen(name);
  int i=0;
  for (ep = envp; *ep != NULL; ++ep)
  {
    for (i=0; (*ep)[i] == name[i] && i<namelen; i++)
      ;
    if (i == namelen && (*ep)[namelen] == '=')
      return &(*ep)[namelen+1];
  }
  return NULL;
}
</pre>

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/11120] strncmp() will be broken if used in ld.so on x86-64

[fweimer at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=11120

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |security-

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/11120] strncmp() will be broken if used in ld.so on x86-64

[drepper at redhat dot com]

------- Additional Comments From drepper at redhat dot com  2010-01-14 16:10 -------
Changed in git.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED


http://sourceware.org/bugzilla/show_bug.cgi?id=11120

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.