Hello folks,
I recently pick up the implementation of arch_align_stack() from x86
architectures to make it available for mips.
But now I just realised that this function seems useless because of
the way it's used.
Currently, this function seems to be only used to randomize the stack
pointer inside a page during an execve() syscall. Only i386, x86_64
and recently mips do that. Here is the code taken from exec.c which
calls it:
int setup_arg_pages(struct linux_binprm *bprm,
unsigned long stack_top,
int executable_stack)
{
[...]
stack_top = arch_align_stack(stack_top);
stack_top = PAGE_ALIGN(stack_top);
[...]
}
Since PAGE_ALIGN() is called right after arch_align_stack(), it seems
to me that the call to the latter function is useless...
Am I missing something ?
thanks
Franck
On Mon, 27 Aug 2007 16:08:31 +0200
Franck Bui-Huu <[email protected]> wrote:
> and recently mips do that. Here is the code taken from exec.c which
> calls it:
>
> int setup_arg_pages(struct linux_binprm *bprm,
> unsigned long stack_top,
> int executable_stack)
> {
> [...]
>
> stack_top = arch_align_stack(stack_top);
> stack_top = PAGE_ALIGN(stack_top);
>
> [...]
> }
>
> Since PAGE_ALIGN() is called right after arch_align_stack(), it seems
> to me that the call to the latter function is useless...
>
> Am I missing something ?
arch_align_stack aligns, on x86, within a 2 page range (this is for
cache coloring). The other thing you missed is that arch_align_stack()
is called in 2 locations, binfmt_elf.c is the primary location for
inside-the-page randomization.
Hello Arjan,
Arjan van de Ven wrote:
> arch_align_stack aligns, on x86, within a 2 page range (this is for
> cache coloring).
OK, but for elf case this seems useless since the top of the stack is
already randomized.
It seems that the randomization stuff (top of the stack + stack
pointer inside a page) belongs to the elf binary format whereas it
could have been part of exec.c. Are there any reasons ?
> The other thing you missed is that arch_align_stack()
> is called in 2 locations, binfmt_elf.c is the primary location for
> inside-the-page randomization.
>
Well not really because for mips case, we have:
$ git grep ELF_PLATFORM include/asm-mips
include/asm-mips/elf.h:#define ELF_PLATFORM (NULL)
So on mips, the stack pointer won't get the inside the page
randomization. Is that correct ?
If so, I'm wondering why this randomization must depend on that string
to be defined. I must admit that I'm not sure how it's used. I guess
it's used by ld.so and it could be set to "mips" for now...
thanks
Franck
Arjan,
Franck Bui-Huu wrote:
> Arjan van de Ven wrote:
>> arch_align_stack aligns, on x86, within a 2 page range (this is for
>> cache coloring).
>
> OK, but for elf case this seems useless since the top of the stack is
> already randomized.
>
> It seems that the randomization stuff (top of the stack + stack
> pointer inside a page) belongs to the elf binary format whereas it
> could have been part of exec.c. Are there any reasons ?
>
>> The other thing you missed is that arch_align_stack()
>> is called in 2 locations, binfmt_elf.c is the primary location for
>> inside-the-page randomization.
>>
>
> Well not really because for mips case, we have:
>
> $ git grep ELF_PLATFORM include/asm-mips
> include/asm-mips/elf.h:#define ELF_PLATFORM (NULL)
>
> So on mips, the stack pointer won't get the inside the page
> randomization. Is that correct ?
>
> If so, I'm wondering why this randomization must depend on that string
> to be defined. I must admit that I'm not sure how it's used. I guess
> it's used by ld.so and it could be set to "mips" for now...
>
Do you think that the diff below would be correct ? I tested it on
mips and it seems to work fine but I can't really say if it's
valid enough to submit it as a patch...
thanks
Franck
---8<---
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 4482a06..024006e 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -151,6 +151,13 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
struct vm_area_struct *vma;
/*
+ * In some cases (e.g. Hyper-Threading), we want to avoid L1
+ * evictions by the processes running on the same package. One
+ * thing we can do is to shuffle the initial stack for them.
+ */
+ p = arch_align_stack(p);
+
+ /*
* If this architecture has a platform capability string, copy it
* to userspace. In some cases (Sparc), this info is impossible
* for userspace to get any other way, in others (i386) it is
@@ -160,14 +167,6 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
if (k_platform) {
size_t len = strlen(k_platform) + 1;
- /*
- * In some cases (e.g. Hyper-Threading), we want to avoid L1
- * evictions by the processes running on the same package. One
- * thing we can do is to shuffle the initial stack for them.
- */
-
- p = arch_align_stack(p);
-
u_platform = (elf_addr_t __user *)STACK_ALLOC(p, len);
if (__copy_to_user(u_platform, k_platform, len))
return -EFAULT;