Subject: [PATCH] Break ELF_PLATFORM and stack pointer randomization dependency

Currently arch_align_stack() is used by fs/binfmt_elf.c to randomize
stack pointer inside a page. But this happens only if ELF_PLATFORM
symbol is defined.

ELF_PLATFORM is normally set if the architecture wants ld.so to load
implementation specific libraries for optimization. And currently a
lot of architectures just yield this symbol to NULL.

This is the case for MIPS architecture where ELF_PLATFORM is NULL but
arch_align_stack() has been redefined to do stack inside page
randomization. So in this case no randomization is actually done.

This patch breaks this dependency which seems to be useless and allows
platforms such MIPS to do the randomization.

Signed-off-by: Franck Bui-Huu <[email protected]>
---

Andrew,

I tried several times to poke people on the list to understand why
this dependency exists at all, but unfortunately got no answers.

So I'm submitting this patch to at least have some feedbacks. An
easier solution would be to define ELF_PLATFORM to a dummy string
for MIPS but it sounds very hackish.

Thanks,
Franck

fs/binfmt_elf.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 4482a06..760d53d 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -151,6 +151,14 @@ 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 +168,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;
--
1.5.3.1


2007-09-28 14:53:03

by Ralf Baechle

[permalink] [raw]
Subject: Re: [PATCH] Break ELF_PLATFORM and stack pointer randomization dependency

On Fri, Sep 28, 2007 at 12:59:47PM +0200, Franck Bui-Huu wrote:

> Currently arch_align_stack() is used by fs/binfmt_elf.c to randomize
> stack pointer inside a page. But this happens only if ELF_PLATFORM
> symbol is defined.
>
> ELF_PLATFORM is normally set if the architecture wants ld.so to load
> implementation specific libraries for optimization. And currently a
> lot of architectures just yield this symbol to NULL.
>
> This is the case for MIPS architecture where ELF_PLATFORM is NULL but
> arch_align_stack() has been redefined to do stack inside page
> randomization. So in this case no randomization is actually done.
>
> This patch breaks this dependency which seems to be useless and allows
> platforms such MIPS to do the randomization.
>
> Signed-off-by: Franck Bui-Huu <[email protected]>
> ---
>
> Andrew,
>
> I tried several times to poke people on the list to understand why
> this dependency exists at all, but unfortunately got no answers.
>
> So I'm submitting this patch to at least have some feedbacks. An
> easier solution would be to define ELF_PLATFORM to a dummy string
> for MIPS but it sounds very hackish.

Probably this was introduced a long time ago, so it's only recorded in
tglx's history git tree:

commit ccc875c1d2fe18b50020d501f1005ef46fc55fed
Author: Arjan van de Ven <[email protected]>
Date: Fri Mar 4 17:25:13 2005 -0800

[PATCH] Randomisation: stack randomisation

The patch below replaces the existing 8Kb randomisation of the userspace sta
pointer (which is currently only done for Hyperthreaded P-IVs) with a more
general randomisation over a 64Kb range. 64Kb is not a lot, but it's a star
and once the dust settles we can increase this value to a more agressive
value.

Signed-off-by: Arjan van de Ven <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>

http://git.kernel.org/?p=linux/kernel/git/tglx/history.git;a=commitdiff;h=ccc875c1d2fe18b50020d501f1005ef46fc55fed

Ralf