[PATCH] binfmt_elf.c SIGILL with large external static array on PPC64

[PATCH] binfmt_elf.c SIGILL with large external static array on PPC64

[srikrish]

When using the follow code compiled as an ELF32 binary on a PPC64 machine, application terminates with a SIGILL.
<source code of C Application>
int x[3000][3000][30];
int main() {
    return 0;
}

Platform: Linux-2.6.0-test11 on PPC64

Comments: The problem is that we don't reserve the bss region for the app (via a set_brk/do_brk call) until after we've loaded the
loader so they get mapped to overlapping memory locations. The fix is to move the set_brk/do_brk call to before the point we call
load_elf_interp().
Here's a patch. This gives a clearer error message and terminates with a SEGFAULT. Though I'm not sure if a large external array
could be the only trigger for this. I'd appreciate your comments/suggestions.
 In case of difficulty in applying the patch enclosed, you could find it at
http://www.geocities.com/ssrikrishnan/binfmtelf.htm

Thanks,
Srikrishnan

--- linux-2.6.0-test11/fs/binfmt_elf.c	2003-12-16 18:50:46.000000000 +0530
+++ linux-2.6.0-test11.patched/fs/binfmt_elf.c	2003-12-16 18:58:51.000000000 +0530
@@ -82,13 +82,15 @@

 #define BAD_ADDR(x)	((unsigned long)(x) > TASK_SIZE)

-static void set_brk(unsigned long start, unsigned long end)
+static unsigned long set_brk(unsigned long start, unsigned long end)
 {
+	unsigned long retval;
 	start = ELF_PAGEALIGN(start);
 	end = ELF_PAGEALIGN(end);
 	if (end > start)
-		do_brk(start, end - start);
+		retval = do_brk(start, end - start);
 	current->mm->start_brk = current->mm->brk = end;
+	return retval;
 }


@@ -737,6 +739,19 @@
 	start_data += load_bias;
 	end_data += load_bias;

+        /* Calling set_brk effectively mmaps the pages that we need
+         * for the bss and break sections. We must do this before
+         * before we load the interpreter, in case we have a
+         * huge bss region.
+         */
+	if(BAD_ADDR(set_brk(elf_bss, elf_brk)) {
+		printk(KERN_ERR "unable to map bss region \n");
+		send_sig(SIGSEGV, current, 0);
+		goto out_free_dentry;
+
+	}
+
+	padzero(elf_bss);
 	if (elf_interpreter) {
 		if (interpreter_type == INTERPRETER_AOUT)
 			elf_entry = load_aout_interp(&interp_ex,
@@ -782,13 +797,6 @@
 	current->mm->end_data = end_data;
 	current->mm->start_stack = bprm->p;

-	/* Calling set_brk effectively mmaps the pages that we need
-	 * for the bss and break sections
-	 */
-	set_brk(elf_bss, elf_brk);
-
-	padzero(elf_bss);
-
 	if (current->personality & MMAP_PAGE_ZERO) {
 		/* Why this, you ask???  Well SVr4 maps page 0 as read-only,
 		   and some applications "depend" upon this behavior.

Re: [PATCH] binfmt_elf.c SIGILL with large external static array on PPC64

[Andrew Morton]

"srikrish" <srikrish@in.ibm.com> wrote:
>
>  When using the follow code compiled as an ELF32 binary on a PPC64 machine, application terminates with a SIGILL.

Yes.  I have queued up a patch for this from Roland Turner.  It is at

	ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.0-test10/2.6.0-test10-mm1/broken-out/fix-ELF-exec-with-huge-bss.patch

Could you please review and test it?

Re: [PATCH] binfmt_elf.c SIGILL with large external static array on PPC64

[srikrish]

Andrew,
I found that Roland's patch addresses the issue in the same way that I had proposed. His patch looks fine. The return value of
set_brk function could be changed to unsigned long as do_brk returns unsigned long.

Srikrishnan

Andrew wrote:
>Yes.  I have queued up a patch for this from Roland Turner.  It is at

ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.0-test10/2.6.0-test10-mm1/broken-out/fix-ELF-exec-with-huge-bss.patch

>Could you please review and test it?