lkml.org 
[lkml]   [2009]   [Jul]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] information leak in sigaltstack

[ Cc'ing jakub, since that code generation looks crappy, and I think he
has worked on gcc memset(). I wonder if it's because we use -Os, and gcc
tries to avoid one REX prefix on the 'stosq'.

I also wonder why gcc doesn't just notice that it should really only
initialize a single 4-byte word (no rep, no prefix, no nothing, just a
single "movl $0,44(%ebp)") - so even with the -Os, that is just wrong,
and it would have been better to do as multiple stores and then noticing
that most of them end up dead ]

On Fri, 31 Jul 2009, Ulrich Drepper wrote:
>
> I was just composing a reply with basically this. So you'll apply this
> and don't wait for me to send a new version of the patch, right?

Grr. Gcc creates truly crap code for this trivial 24-byte memset. Why does
it do that?

gcc knows the alignment is 8 bytes, but it still uses 6 4-byte stores
instead of 3 8-byte ones. And it does it with this:

xorl %eax, %eax # tmp88
leaq -48(%rbp), %rsi #, tmp86
movl $6, %ecx #, tmp89
movq %rsi, %rdi # tmp86, tmp87
rep stosl

which is just incredibly lame in so many ways.

And it doesn't optimize anything away, even though the next lines will
then re-initialize 20 of the 24 bytes.

Now, maybe this isn't performance-critical, but it just makes me feel that
there has to be a better way to make gcc DTRT.

Here's the patch I used, just for posterity. I can't decide if I really
want to commit this crap. But at least on 32-bit architectures the
"alignof" testing should remove the horrid code. I do wonder why gcc
thinks that 32-bit writes are a good idea in this case, though.

Linus

---
kernel/signal.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index ccf1cee..b990dc8 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2455,6 +2455,9 @@ do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long s
int error;

if (uoss) {
+ /* Fill cracks around 'ss_flags' */
+ if (__alignof__(oss.ss_flags) != __alignof__(oss))
+ memset(&oss, 0, sizeof(oss));
oss.ss_sp = (void __user *) current->sas_ss_sp;
oss.ss_size = current->sas_ss_size;
oss.ss_flags = sas_ss_flags(sp);

\
 
 \ /
  Last update: 2009-10-18 23:28    [W:0.084 / U:0.356 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site