Re: [PATCH 02/12] Make __stack_chk_fail() not use other glibc facilities.

[Nix <nix at esperi dot org dot uk>]

On 22 Feb 2016, Mike Frysinger told this:

> On 21 Feb 2016 12:44, Nix wrote:
>> On 20 Feb 2016, Mike Frysinger outgrape:
>> > On 20 Feb 2016 12:06, Nix wrote:
>> >> Magnus Granberg has now pointed me at a much more recent version of this
>> >> change, which replaces __chk_fail() to the same end and reimplement
>> >> __stack_chk_fail() in terms of it. I'll incorporate that in the next
>> >> version. (The only concern is that this has received changes from lots
>> >> of people. Maybe I'll have to reimplement it entirely to avoid
>> >> copyright-assignment concerns. What a drag...)
>> >
>> > yes, you will.  you (and Oracle) have CLA papers already signed for glibc
>> > right ?  if not, there isn't much point in following up on this series if
>> > you don't plan to.
>> 
>> Oracle has a disclaimer signed, of couse, but I'll need to start the
>> copyright papers dance for myself (I have signed papers for other GNU
>> projects, but not glibc yet).
>
> a disclaimer is not the same thing as a CLA.  what exactly are you
> referring to here ?

Well, when I had to do this last (years ago), there was an employer's
disclaimer of rights and there were assignment-and-grantback papers for
the individual to sign for the project in question. I still have the
latter to do for glibc.

>> > it needs a rewrite anyways to not use INLINE_SYSCALL but INTERNAL_SYSCALL. 
>> > i had done this locally, stashed it, but then lost it when cleaning up the
>> > repo later :(.
>> 
>> Frankly, I was rather hoping that someone could figure out what I
>> hadn't yet -- the right way to prevent the intermediate link failing
>> when the original __stack_chk_fail() is used.
>
> i haven't looked closely at the loop.  i assumed the fail relied on more
> printf machinery.

Yeah, but the question is just why that incremental link is trying to
pull the stuff in twice rather than depending on the canonical copy in
libc -- presumably because everything is still statically linked at that
point. If that gets solved, the only reason for this patch being in this
series goes away.

>> > all that said, i don't think we want to import the Gentoo one anyways.
>> > it changes a lot of behavior that we shouldn't conflate here -- the
>> > crash/syslog handling is contentious.  maybe something like (untested):
>> >
>> > void
>> > __attribute__ ((noreturn)) internal_function
>> > __fortify_fail (const char *msg)
>> > {
>> > #ifdef __SSP__
>> >   int fd = -1;
>> >
>> >   const char *on_2 = __libc_secure_getenv ("LIBC_FATAL_STDERR_");
>> >   if (on_2 == NULL || *on_2 == '\0')
>> >     fd = open_not_cancel_2 (_PATH_TTY, O_RDWR | O_NOCTTY | O_NDELAY);
>> 
>> I fear this use of open_not_cancel_2() may bring back the same behaviour
>> we're trying to avoid :(
>
> if this triggers the loop, then you'll need INTERNAL_SYSCALL.  if that
> fixes the loop, then we can probably not make it depend on __SSP__ at
> all.

True enough. I'll give it a try, and thanks for the help!

-- 
NULL && (void)