On Thu, Sep 5, 2019 at 11:20 AM Nick Desaulniers
wrote:
>
> On Wed, Sep 4, 2019 at 10:34 PM Andreas Smas wrote:
> >
> > On Wed, Sep 4, 2019 at 3:19 PM Nick Desaulniers
> > wrote:
> > > Thanks for confirming the fix. While it sounds like -mcmodel=large is
> > > the only necessary change, I don't object to -ffreestanding of
> > > -fno-zero-initialized-in-bss being readded, especially since I think
> > > what you've done with PURGATORY_CFLAGS_REMOVE is more concise.
> >
> > Without -ffreestanding this results in undefined symbols (as before this
> > patch)
>
> Thanks for the report and sorry for the breakage. Can you test
> Steve's patch and send your tested by tag? Steve will likely respin
> the final patch today with Boris' feedback, so now is the time to get
> on the train.
>
> >
> > $ readelf -a arch/x86/purgatory/purgatory.ro|grep UND
> > 0: 0 NOTYPE LOCAL DEFAULT UND
>
> ^ what's that? A horse symbol with no name?
No idea TBH. Not enough of an ELF-expert to explain that. It's also there with
the -ffreestanding -patch (when kexec() works for me again)
so it doesn't seem to cause any harm.
>
> > 51: 0 NOTYPE GLOBAL DEFAULT UND __stack_chk_fail
>
> ^ so I would have expected the stackprotector changes in my and Steve
> commits to prevent compiler emission of that runtime-implemented
> symbol. ie. that `-ffreestanding` affects that and not removing the
> stackprotector flags begs another question. Without `-ffreestanding`
> and `-fstack-protector` (or `-fstack-protector-strong`), why would the
> compiler emit references to __stack_chk_fail? Which .o file that
> composes the .ro file did we fail to remove the `-fstack-protector*`
> flag from? `-ffreestanding` seems to be covering that up.
So, I'm using
$ gcc --version
gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0
I think the problem is that stock ubuntu gcc defaults to -fstack-protector.
I haven't figured out where to check how/where ubuntu configures gcc except
an ancient discussion here: https://wiki.ubuntu.com/GccSsp.
Both -fno-stack-protector or -ffreestanding fixes the issue. I'm not sure
which would be preferred? -ffreestanding sounds a bit better to me though,
as that's really what we are dealing with here.
So,
Tested-by: Andreas Smas
FWIW, one of the offending functions is sha256_transform() where the u32 W[64];
triggers insert of a stack guard variable. (since -fstack-protector is
default on)
End of sha256_transform()
/* clear any sensitive info... */
a = b = c = d = e = f = g = h = t1 = t2 = 0;
memset(W, 0, 64 * sizeof(u32));
}
1aab: 48 8b 84 24 00 01 00mov0x100(%rsp),%rax
1ab2: 00
1ab3: 65 48 33 04 25 28 00xor%gs:0x28,%rax
1aba: 00 00
state[0] += a; state[1] += b; state[2] += c; state[3] += d;
1abc: 44 89 37mov%r14d,(%rdi)
1abf: 44 89 47 0c mov%r8d,0xc(%rdi)
state[4] += e; state[5] += f; state[6] += g; state[7] += h;
1ac3: 44 89 6f 10 mov%r13d,0x10(%rdi)
1ac7: 89 4f 14mov%ecx,0x14(%rdi)
1aca: 89 5f 18mov%ebx,0x18(%rdi)
}
1acd: 75 12 jne1ae1
1acf: 48 81 c4 08 01 00 00add$0x108,%rsp
1ad6: 5b pop%rbx
1ad7: 5d pop%rbp
1ad8: 41 5c pop%r12
1ada: 41 5d pop%r13
1adc: 41 5e pop%r14
1ade: 41 5f pop%r15
1ae0: c3 retq
1ae1: e8 00 00 00 00 callq 1ae6
.rela.text:
1ae2 00110002 R_X86_64_PC32 __stack_chk_fail - 4
Same thing with this latest patch (ie, -ffreestanding)
/* clear any sensitive info... */
a = b = c = d = e = f = g = h = t1 = t2 = 0;
memset(W, 0, 64 * sizeof(u32));
1aa2: ba 00 01 00 00 mov$0x100,%edx
state[4] += e; state[5] += f; state[6] += g; state[7] += h;
1aa7: 89 47 1cmov%eax,0x1c(%rdi)
state[0] += a; state[1] += b; state[2] += c; state[3] += d;
1aaa: 44 89 47 0c mov%r8d,0xc(%rdi)
memset(W, 0, 64 * sizeof(u32));
1aae: 31 f6 xor%esi,%esi
state[4] += e; state[5] += f; state[6] += g; state[7] += h;
1ab0: 89 4f 14mov%ecx,0x14(%rdi)
memset(W, 0, 64 * sizeof(u32));
1ab3: 48 b8 00 00 00 00 00movabs $0x0,%rax<- ()
1aba: 00 00 00
1abd: 48 89 e7mov%rsp,%rdi
1ac0: ff d0 callq *%rax
}
