Re: arch_align_stack() seems useless

[Franck Bui-Huu <vagabon.xyz@gmail.com>]

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;