Re: [PATCH][RESEND] PIE randomization

2007-07-11 Thread Jiri Kosina 
On Tue, 10 Jul 2007, Jakub Jelinek wrote:

> Restore BAD_ADDR check strictness, use IS_ERR in the only place where
> the stricter BAD_ADDR can't work, as the value is a load bias rather
> than userland address.
> Signed-off-by: Jakub Jelinek <[EMAIL PROTECTED]>

Signed-off-by: Jiri Kosina <[EMAIL PROTECTED]>

Andrew, fold this into pie-randomization.patch if possible please. Thanks,

-- 
Jiri Kosina
SUSE Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-11 Thread Jiri Kosina 
On Tue, 10 Jul 2007, Jakub Jelinek wrote:

 Restore BAD_ADDR check strictness, use IS_ERR in the only place where
 the stricter BAD_ADDR can't work, as the value is a load bias rather
 than userland address.
 Signed-off-by: Jakub Jelinek [EMAIL PROTECTED]

Signed-off-by: Jiri Kosina [EMAIL PROTECTED]

Andrew, fold this into pie-randomization.patch if possible please. Thanks,

-- 
Jiri Kosina
SUSE Labs
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-10 Thread Jakub Jelinek 
On Mon, Jul 09, 2007 at 11:58:07PM +0200, Jiri Kosina wrote:
> On Mon, 9 Jul 2007, Jiri Kosina wrote:
> > [ ... ]
> > > - if (!BAD_ADDR(elf_entry)) {
> > > + if (!IS_ERR((void *)elf_entry)) {
> > I agree that this is better solution. Andrew, this Jakub's patch should 
> > replace the pie-randomization-fix-bad_addr-macro.patch if possible. You 
> > can add 
> 
> as this raced :) with Andrew who already folded the 
> pie-randomization-fix-bad_addr-macro.patch into pie-randomization.patch, 
> do you think you could rebase this change against the current state of -mm 
> and resend it? Thanks,

Here it is:

Restore BAD_ADDR check strictness, use IS_ERR in the only place where
the stricter BAD_ADDR can't work, as the value is a load bias rather
than userland address.

Signed-off-by: Jakub Jelinek <[EMAIL PROTECTED]>

--- linux/fs/binfmt_elf.c   2007-07-10 11:39:29.0 +0200
+++ linux/fs/binfmt_elf.c   2007-07-10 11:41:03.0 +0200
@@ -80,7 +80,7 @@ static struct linux_binfmt elf_format = 
.hasvdso= 1
 };
 
-#define BAD_ADDR(x) IS_ERR_VALUE(x)
+#define BAD_ADDR(x) ((unsigned long)(x) >= TASK_SIZE)
 
 static int set_brk(unsigned long start, unsigned long end)
 {
@@ -1005,7 +1005,7 @@ static int load_elf_binary(struct linux_
interpreter,
_map_addr,
load_bias);
-   if (!BAD_ADDR(elf_entry)) {
+   if (!IS_ERR((void *)elf_entry)) {
/*
 * load_elf_interp() returns relocation
 * adjustment


Jakub
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-10 Thread Jakub Jelinek 
On Mon, Jul 09, 2007 at 11:58:07PM +0200, Jiri Kosina wrote:
 On Mon, 9 Jul 2007, Jiri Kosina wrote:
  [ ... ]
   - if (!BAD_ADDR(elf_entry)) {
   + if (!IS_ERR((void *)elf_entry)) {
  I agree that this is better solution. Andrew, this Jakub's patch should 
  replace the pie-randomization-fix-bad_addr-macro.patch if possible. You 
  can add 
 
 as this raced :) with Andrew who already folded the 
 pie-randomization-fix-bad_addr-macro.patch into pie-randomization.patch, 
 do you think you could rebase this change against the current state of -mm 
 and resend it? Thanks,

Here it is:

Restore BAD_ADDR check strictness, use IS_ERR in the only place where
the stricter BAD_ADDR can't work, as the value is a load bias rather
than userland address.

Signed-off-by: Jakub Jelinek [EMAIL PROTECTED]

--- linux/fs/binfmt_elf.c   2007-07-10 11:39:29.0 +0200
+++ linux/fs/binfmt_elf.c   2007-07-10 11:41:03.0 +0200
@@ -80,7 +80,7 @@ static struct linux_binfmt elf_format = 
.hasvdso= 1
 };
 
-#define BAD_ADDR(x) IS_ERR_VALUE(x)
+#define BAD_ADDR(x) ((unsigned long)(x) = TASK_SIZE)
 
 static int set_brk(unsigned long start, unsigned long end)
 {
@@ -1005,7 +1005,7 @@ static int load_elf_binary(struct linux_
interpreter,
interp_map_addr,
load_bias);
-   if (!BAD_ADDR(elf_entry)) {
+   if (!IS_ERR((void *)elf_entry)) {
/*
 * load_elf_interp() returns relocation
 * adjustment


Jakub
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-09 Thread Jiri Kosina 
On Mon, 9 Jul 2007, Jiri Kosina wrote:

> > Signed-off-by: Jakub Jelinek <[EMAIL PROTECTED]>
> [ ... ]
> > -   if (!BAD_ADDR(elf_entry)) {
> > +   if (!IS_ERR((void *)elf_entry)) {
> I agree that this is better solution. Andrew, this Jakub's patch should 
> replace the pie-randomization-fix-bad_addr-macro.patch if possible. You 
> can add 

Hi Jakub,

as this raced :) with Andrew who already folded the 
pie-randomization-fix-bad_addr-macro.patch into pie-randomization.patch, 
do you think you could rebase this change against the current state of -mm 
and resend it? Thanks,

-- 
Jiri Kosina
SUSE Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-09 Thread Jiri Kosina 
On Sat, 7 Jul 2007, Jakub Jelinek wrote:

> I believe BAD_ADDR macro was changes from ((unsigned long)(x) >= 
> TASK_SIZE) (which is the right test for invalid user addresses, stronger 
> check than >= PAGE_MASK) to >= PAGE_MASK only because of the one check 
> of the return value of load_elf_interp.  

Yes, this is correct.

> All other uses of BAD_ADDR macro are either on userland addresses (what 
> do_mmap, elf_map, do_brk etc. return; where TASK_SIZE or more is 
> certainly wrong) 

True. But these functions are supposed to perform all the necessary checks 
and return ERR_PTR if anything fails, so everything is fine and nothing 
gets broken.

> or in one case still on unbiased ELF p_vaddr: if (BAD_ADDR(k) || 
> elf_ppnt->p_filesz > elf_ppnt->p_memsz || in load_elf_binary (where >= 
> TASK_SIZE check is ok too).
> So perhaps doing this instead of changing BAD_ADDR to IS_ERR_VAL
> might be better:
> Signed-off-by: Jakub Jelinek <[EMAIL PROTECTED]>
[ ... ]
> - if (!BAD_ADDR(elf_entry)) {
> + if (!IS_ERR((void *)elf_entry)) {

I agree that this is better solution. Andrew, this Jakub's patch should 
replace the pie-randomization-fix-bad_addr-macro.patch if possible. You 
can add 

Signed-off-by: Jiri Kosina <[EMAIL PROTECTED]>

if that makes any difference.

Thanks,

-- 
Jiri Kosina
SUSE Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-09 Thread Jiri Kosina 
On Sat, 7 Jul 2007, Jakub Jelinek wrote:

 I believe BAD_ADDR macro was changes from ((unsigned long)(x) = 
 TASK_SIZE) (which is the right test for invalid user addresses, stronger 
 check than = PAGE_MASK) to = PAGE_MASK only because of the one check 
 of the return value of load_elf_interp.  

Yes, this is correct.

 All other uses of BAD_ADDR macro are either on userland addresses (what 
 do_mmap, elf_map, do_brk etc. return; where TASK_SIZE or more is 
 certainly wrong) 

True. But these functions are supposed to perform all the necessary checks 
and return ERR_PTR if anything fails, so everything is fine and nothing 
gets broken.

 or in one case still on unbiased ELF p_vaddr: if (BAD_ADDR(k) || 
 elf_ppnt-p_filesz  elf_ppnt-p_memsz || in load_elf_binary (where = 
 TASK_SIZE check is ok too).
 So perhaps doing this instead of changing BAD_ADDR to IS_ERR_VAL
 might be better:
 Signed-off-by: Jakub Jelinek [EMAIL PROTECTED]
[ ... ]
 - if (!BAD_ADDR(elf_entry)) {
 + if (!IS_ERR((void *)elf_entry)) {

I agree that this is better solution. Andrew, this Jakub's patch should 
replace the pie-randomization-fix-bad_addr-macro.patch if possible. You 
can add 

Signed-off-by: Jiri Kosina [EMAIL PROTECTED]

if that makes any difference.

Thanks,

-- 
Jiri Kosina
SUSE Labs
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-09 Thread Jiri Kosina 
On Mon, 9 Jul 2007, Jiri Kosina wrote:

  Signed-off-by: Jakub Jelinek [EMAIL PROTECTED]
 [ ... ]
  -   if (!BAD_ADDR(elf_entry)) {
  +   if (!IS_ERR((void *)elf_entry)) {
 I agree that this is better solution. Andrew, this Jakub's patch should 
 replace the pie-randomization-fix-bad_addr-macro.patch if possible. You 
 can add 

Hi Jakub,

as this raced :) with Andrew who already folded the 
pie-randomization-fix-bad_addr-macro.patch into pie-randomization.patch, 
do you think you could rebase this change against the current state of -mm 
and resend it? Thanks,

-- 
Jiri Kosina
SUSE Labs
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-07 Thread Jakub Jelinek 
On Sat, Jul 07, 2007 at 02:13:01AM +0200, Jiri Kosina wrote:
> On Thu, 5 Jul 2007, Rik van Riel wrote:
> 
> > So the original patch has:
> > #define BAD_ADDR(x) ((unsigned long)(x) >= TASK_SIZE)
> > For some reason(?) it got changed to the clearly buggy:
> > #define BAD_ADDR(x) ((unsigned long)(x) >= PAGE_MASK)
> > Jiri's patch undoes that second buggy define, which is very
> > different from the original that was sent in by you and Ernie.
> 
> This is a part of execshield patch, fthe pie-compiled binary executable 
> memory layout randomization was extracted from - see 
> http://people.redhat.com/~mingo/exec-shield/exec-shield-nx-2.6.19.patch
> 
> Note that load_elf_interp() in vanilla kernel differs from the 
> execshield's (and pie-randomization.patch) version.
> 
> The fix makes the BAD_ADDR check whether the address belongs to the 
> ERR_PTR range, which seems valid for all uses of BAD_ADDR in the patched 
> binfmt_elf.c (do_brk(), elf_map(), do_mmap() etc return valid address or 
> err ptr) ... am I missing something obvious here?

I believe BAD_ADDR macro was changes from ((unsigned long)(x) >= TASK_SIZE)
(which is the right test for invalid user addresses, stronger check than
>= PAGE_MASK) to >= PAGE_MASK only because of the one check of the return
value of load_elf_interp.  All other uses of BAD_ADDR macro are either on
userland addresses (what do_mmap, elf_map, do_brk etc. return;
where TASK_SIZE or more is certainly wrong) or in one case still on unbiased
ELF p_vaddr:
if (BAD_ADDR(k) || elf_ppnt->p_filesz > elf_ppnt->p_memsz ||
in load_elf_binary (where >= TASK_SIZE check is ok too).

So perhaps doing this instead of changing BAD_ADDR to IS_ERR_VAL
might be better:

Signed-off-by: Jakub Jelinek <[EMAIL PROTECTED]>

--- linux/fs/binfmt_elf.c   2007-06-08 21:53:45.0 +0200
+++ linux/fs/binfmt_elf.c   2007-07-07 14:19:14.0 +0200
@@ -80,7 +80,7 @@ static struct linux_binfmt elf_format = 
.hasvdso= 1
 };
 
-#define BAD_ADDR(x) ((unsigned long)(x) >= PAGE_MASK)
+#define BAD_ADDR(x) ((unsigned long)(x) >= TASK_SIZE)
 
 static int set_brk(unsigned long start, unsigned long end)
 {
@@ -1015,7 +1015,7 @@ static int load_elf_binary(struct linux_
interpreter,
_map_addr,
load_bias);
-   if (!BAD_ADDR(elf_entry)) {
+   if (!IS_ERR((void *)elf_entry)) {
/* load_elf_interp() returns relocation 
adjustment */
interp_load_addr = elf_entry;
elf_entry += loc->interp_elf_ex.e_entry;


Jakub
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-07 Thread Jakub Jelinek 
On Sat, Jul 07, 2007 at 02:13:01AM +0200, Jiri Kosina wrote:
 On Thu, 5 Jul 2007, Rik van Riel wrote:
 
  So the original patch has:
  #define BAD_ADDR(x) ((unsigned long)(x) = TASK_SIZE)
  For some reason(?) it got changed to the clearly buggy:
  #define BAD_ADDR(x) ((unsigned long)(x) = PAGE_MASK)
  Jiri's patch undoes that second buggy define, which is very
  different from the original that was sent in by you and Ernie.
 
 This is a part of execshield patch, fthe pie-compiled binary executable 
 memory layout randomization was extracted from - see 
 http://people.redhat.com/~mingo/exec-shield/exec-shield-nx-2.6.19.patch
 
 Note that load_elf_interp() in vanilla kernel differs from the 
 execshield's (and pie-randomization.patch) version.
 
 The fix makes the BAD_ADDR check whether the address belongs to the 
 ERR_PTR range, which seems valid for all uses of BAD_ADDR in the patched 
 binfmt_elf.c (do_brk(), elf_map(), do_mmap() etc return valid address or 
 err ptr) ... am I missing something obvious here?

I believe BAD_ADDR macro was changes from ((unsigned long)(x) = TASK_SIZE)
(which is the right test for invalid user addresses, stronger check than
= PAGE_MASK) to = PAGE_MASK only because of the one check of the return
value of load_elf_interp.  All other uses of BAD_ADDR macro are either on
userland addresses (what do_mmap, elf_map, do_brk etc. return;
where TASK_SIZE or more is certainly wrong) or in one case still on unbiased
ELF p_vaddr:
if (BAD_ADDR(k) || elf_ppnt-p_filesz  elf_ppnt-p_memsz ||
in load_elf_binary (where = TASK_SIZE check is ok too).

So perhaps doing this instead of changing BAD_ADDR to IS_ERR_VAL
might be better:

Signed-off-by: Jakub Jelinek [EMAIL PROTECTED]

--- linux/fs/binfmt_elf.c   2007-06-08 21:53:45.0 +0200
+++ linux/fs/binfmt_elf.c   2007-07-07 14:19:14.0 +0200
@@ -80,7 +80,7 @@ static struct linux_binfmt elf_format = 
.hasvdso= 1
 };
 
-#define BAD_ADDR(x) ((unsigned long)(x) = PAGE_MASK)
+#define BAD_ADDR(x) ((unsigned long)(x) = TASK_SIZE)
 
 static int set_brk(unsigned long start, unsigned long end)
 {
@@ -1015,7 +1015,7 @@ static int load_elf_binary(struct linux_
interpreter,
interp_map_addr,
load_bias);
-   if (!BAD_ADDR(elf_entry)) {
+   if (!IS_ERR((void *)elf_entry)) {
/* load_elf_interp() returns relocation 
adjustment */
interp_load_addr = elf_entry;
elf_entry += loc-interp_elf_ex.e_entry;


Jakub
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-06 Thread Jiri Kosina 
On Thu, 5 Jul 2007, Rik van Riel wrote:

> So the original patch has:
> #define BAD_ADDR(x) ((unsigned long)(x) >= TASK_SIZE)
> For some reason(?) it got changed to the clearly buggy:
> #define BAD_ADDR(x) ((unsigned long)(x) >= PAGE_MASK)
> Jiri's patch undoes that second buggy define, which is very
> different from the original that was sent in by you and Ernie.

This is a part of execshield patch, fthe pie-compiled binary executable 
memory layout randomization was extracted from - see 
http://people.redhat.com/~mingo/exec-shield/exec-shield-nx-2.6.19.patch

Note that load_elf_interp() in vanilla kernel differs from the 
execshield's (and pie-randomization.patch) version.

The fix makes the BAD_ADDR check whether the address belongs to the 
ERR_PTR range, which seems valid for all uses of BAD_ADDR in the patched 
binfmt_elf.c (do_brk(), elf_map(), do_mmap() etc return valid address or 
err ptr) ... am I missing something obvious here?

-- 
Jiri Kosina
SUSE Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-06 Thread Jiri Kosina 
On Thu, 5 Jul 2007, Rik van Riel wrote:

 So the original patch has:
 #define BAD_ADDR(x) ((unsigned long)(x) = TASK_SIZE)
 For some reason(?) it got changed to the clearly buggy:
 #define BAD_ADDR(x) ((unsigned long)(x) = PAGE_MASK)
 Jiri's patch undoes that second buggy define, which is very
 different from the original that was sent in by you and Ernie.

This is a part of execshield patch, fthe pie-compiled binary executable 
memory layout randomization was extracted from - see 
http://people.redhat.com/~mingo/exec-shield/exec-shield-nx-2.6.19.patch

Note that load_elf_interp() in vanilla kernel differs from the 
execshield's (and pie-randomization.patch) version.

The fix makes the BAD_ADDR check whether the address belongs to the 
ERR_PTR range, which seems valid for all uses of BAD_ADDR in the patched 
binfmt_elf.c (do_brk(), elf_map(), do_mmap() etc return valid address or 
err ptr) ... am I missing something obvious here?

-- 
Jiri Kosina
SUSE Labs
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-05 Thread Rik van Riel 

Chuck Ebbert wrote:

On 07/04/2007 01:35 PM, Jiri Kosina wrote:

On Wed, 4 Jul 2007, Jakub Jelinek wrote:

The above highlighted changes are the cause of random segfaults of PIE 
binaries.  See 
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=246623 
Thanks a lot for pointing this out. Andrew, could this be folded into 
pie-randomization.patch please?



From: Jiri Kosina <[EMAIL PROTECTED]>

pie randomization: fix BAD_ADDR macro

pie-randomization.patch makes the load_addr in load_elf_interp() the load 
bias of ld.so (difference between the actual load base address and first 
PT_LOAD segment's p_vaddr). If the difference equals (on x86) to 
0xf000 (which is valid [1]), SIGSEGV is incorrectly sent.


This patch changes the BAD_ADDR so that it catches the mappings to the 
error-area properly.


But what about this patch that made the opposite change:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=ce51059be56f63762089412b3ece348067afda85


Ohhh, interesting.

So the original patch has:

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

For some reason(?) it got changed to the clearly buggy:

#define BAD_ADDR(x) ((unsigned long)(x) >= PAGE_MASK)

Jiri's patch undoes that second buggy define, which is very
different from the original that was sent in by you and Ernie.

--
Politics is the struggle between those who want to make their country
the best in the world, and those who believe it already is.  Each group
calls the other unpatriotic.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-05 Thread Chuck Ebbert 
On 07/04/2007 01:35 PM, Jiri Kosina wrote:
> On Wed, 4 Jul 2007, Jakub Jelinek wrote:
> 
>> The above highlighted changes are the cause of random segfaults of PIE 
>> binaries.  See 
>> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=246623 
> 
> Thanks a lot for pointing this out. Andrew, could this be folded into 
> pie-randomization.patch please?
> 
> 
> From: Jiri Kosina <[EMAIL PROTECTED]>
> 
> pie randomization: fix BAD_ADDR macro
> 
> pie-randomization.patch makes the load_addr in load_elf_interp() the load 
> bias of ld.so (difference between the actual load base address and first 
> PT_LOAD segment's p_vaddr). If the difference equals (on x86) to 
> 0xf000 (which is valid [1]), SIGSEGV is incorrectly sent.
> 
> This patch changes the BAD_ADDR so that it catches the mappings to the 
> error-area properly.

But what about this patch that made the opposite change:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=ce51059be56f63762089412b3ece348067afda85

There was a reason for that change...

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-05 Thread Chuck Ebbert 
On 07/04/2007 01:35 PM, Jiri Kosina wrote:
 On Wed, 4 Jul 2007, Jakub Jelinek wrote:
 
 The above highlighted changes are the cause of random segfaults of PIE 
 binaries.  See 
 https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=246623 
 
 Thanks a lot for pointing this out. Andrew, could this be folded into 
 pie-randomization.patch please?
 
 
 From: Jiri Kosina [EMAIL PROTECTED]
 
 pie randomization: fix BAD_ADDR macro
 
 pie-randomization.patch makes the load_addr in load_elf_interp() the load 
 bias of ld.so (difference between the actual load base address and first 
 PT_LOAD segment's p_vaddr). If the difference equals (on x86) to 
 0xf000 (which is valid [1]), SIGSEGV is incorrectly sent.
 
 This patch changes the BAD_ADDR so that it catches the mappings to the 
 error-area properly.

But what about this patch that made the opposite change:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=ce51059be56f63762089412b3ece348067afda85

There was a reason for that change...

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-05 Thread Rik van Riel 

Chuck Ebbert wrote:

On 07/04/2007 01:35 PM, Jiri Kosina wrote:

On Wed, 4 Jul 2007, Jakub Jelinek wrote:

The above highlighted changes are the cause of random segfaults of PIE 
binaries.  See 
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=246623 
Thanks a lot for pointing this out. Andrew, could this be folded into 
pie-randomization.patch please?



From: Jiri Kosina [EMAIL PROTECTED]

pie randomization: fix BAD_ADDR macro

pie-randomization.patch makes the load_addr in load_elf_interp() the load 
bias of ld.so (difference between the actual load base address and first 
PT_LOAD segment's p_vaddr). If the difference equals (on x86) to 
0xf000 (which is valid [1]), SIGSEGV is incorrectly sent.


This patch changes the BAD_ADDR so that it catches the mappings to the 
error-area properly.


But what about this patch that made the opposite change:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=ce51059be56f63762089412b3ece348067afda85


Ohhh, interesting.

So the original patch has:

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

For some reason(?) it got changed to the clearly buggy:

#define BAD_ADDR(x) ((unsigned long)(x) = PAGE_MASK)

Jiri's patch undoes that second buggy define, which is very
different from the original that was sent in by you and Ernie.

--
Politics is the struggle between those who want to make their country
the best in the world, and those who believe it already is.  Each group
calls the other unpatriotic.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-04 Thread Jiri Kosina 
On Wed, 4 Jul 2007, Jakub Jelinek wrote:

> The above highlighted changes are the cause of random segfaults of PIE 
> binaries.  See 
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=246623 

Thanks a lot for pointing this out. Andrew, could this be folded into 
pie-randomization.patch please?


From: Jiri Kosina <[EMAIL PROTECTED]>

pie randomization: fix BAD_ADDR macro

pie-randomization.patch makes the load_addr in load_elf_interp() the load 
bias of ld.so (difference between the actual load base address and first 
PT_LOAD segment's p_vaddr). If the difference equals (on x86) to 
0xf000 (which is valid [1]), SIGSEGV is incorrectly sent.

This patch changes the BAD_ADDR so that it catches the mappings to the 
error-area properly.

[1] https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=246623

Reported-by: Jakub Jelinek <[EMAIL PROTECTED]>
Signed-off-by: Jiri Kosina <[EMAIL PROTECTED]>

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index da270d1..466477d 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -80,7 +80,7 @@ static struct linux_binfmt elf_format = {
.hasvdso= 1
 };
 
-#define BAD_ADDR(x) ((unsigned long)(x) >= PAGE_MASK)
+#define BAD_ADDR(x) IS_ERR_VALUE(x)
 
 static int set_brk(unsigned long start, unsigned long end)
 {
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-04 Thread Jakub Jelinek 
On Wed, May 23, 2007 at 10:50:24AM +0200, Jiri Kosina wrote:
> From: Jan Kratochvil <[EMAIL PROTECTED]>
> 
> This patch is using mmap()'s randomization functionality in such a way 
> that it maps the main executable of (specially compiled/linked -pie/-fpie) 
> ET_DYN binaries onto a random address (in cases in which mmap() is allowed 
> to perform a randomization).
> 
> Origin of this patch is in exec-shield
> (http://people.redhat.com/mingo/exec-shield/)
> 
> Signed-off-by: Jan Kratochvil <[EMAIL PROTECTED]>
> Signed-off-by: Jiri Kosina <[EMAIL PROTECTED]>
> Cc: Ingo Molnar <[EMAIL PROTECTED]>
> Cc: Roland McGrath <[EMAIL PROTECTED]>
> Cc: Jakub Jelinek <[EMAIL PROTECTED]>

> -#define BAD_ADDR(x) ((unsigned long)(x) >= TASK_SIZE)
> +#define BAD_ADDR(x) ((unsigned long)(x) >= PAGE_MASK)
...
> @@ -442,8 +491,7 @@ static unsigned long load_elf_interp(str
>   goto out_close;
>   }
>  
> - *interp_load_addr = load_addr;
> - error = ((unsigned long)interp_elf_ex->e_entry) + load_addr;
> + error = load_addr;
...
>   if (elf_interpreter) {
> - if (interpreter_type == INTERPRETER_AOUT)
> + if (interpreter_type == INTERPRETER_AOUT) {
>   elf_entry = load_aout_interp(>interp_ex,
>interpreter);
> - else
> + } else {
> + unsigned long interp_map_addr;  /* unused */
> +
>   elf_entry = load_elf_interp(>interp_elf_ex,
>   interpreter,
> - _load_addr);
> + _map_addr,
> + load_bias);
> + if (!BAD_ADDR(elf_entry)) {
> + /*
> +  * load_elf_interp() returns relocation
> +  * adjustment
> +  */
> + interp_load_addr = elf_entry;
> + elf_entry += loc->interp_elf_ex.e_entry;
> + }
> + }
>   if (BAD_ADDR(elf_entry)) {
>   force_sig(SIGSEGV, current);
>   retval = IS_ERR((void *)elf_entry) ?

The above highlighted changes are the cause of random segfaults of PIE
binaries.  See
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=246623
The problem is if ld.so is prelinked to some address in the area where
the kernel actually maps it, particularly if elf_map in load_elf_interp
returns an address one page below its first PT_LOAD segments vaddr.
Then load_addr (it is a load bias actually) returned from load_elf_interp
is 0xf000 (on 32-bit kernels) and BAD_ADDR are all
addresses >= 0xf000 (on i?86).
The fix should be either changing the definition of BAD_ADDR to
e.g. IS_ERR_VALUE(x), or at least changing the if (!BAD_ADDR(elf_entry)) {
above to if (!IS_ERR_VALUE(elf_entry)) {, the second BAD_ADDR can already
stay, because at that place elf_entry is no longer a bias (difference
between actual and preferred load address), but an actual address, where
very high addresses are of course invalid.

Jakub
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-04 Thread Jakub Jelinek 
On Wed, May 23, 2007 at 10:50:24AM +0200, Jiri Kosina wrote:
 From: Jan Kratochvil [EMAIL PROTECTED]
 
 This patch is using mmap()'s randomization functionality in such a way 
 that it maps the main executable of (specially compiled/linked -pie/-fpie) 
 ET_DYN binaries onto a random address (in cases in which mmap() is allowed 
 to perform a randomization).
 
 Origin of this patch is in exec-shield
 (http://people.redhat.com/mingo/exec-shield/)
 
 Signed-off-by: Jan Kratochvil [EMAIL PROTECTED]
 Signed-off-by: Jiri Kosina [EMAIL PROTECTED]
 Cc: Ingo Molnar [EMAIL PROTECTED]
 Cc: Roland McGrath [EMAIL PROTECTED]
 Cc: Jakub Jelinek [EMAIL PROTECTED]

 -#define BAD_ADDR(x) ((unsigned long)(x) = TASK_SIZE)
 +#define BAD_ADDR(x) ((unsigned long)(x) = PAGE_MASK)
...
 @@ -442,8 +491,7 @@ static unsigned long load_elf_interp(str
   goto out_close;
   }
  
 - *interp_load_addr = load_addr;
 - error = ((unsigned long)interp_elf_ex-e_entry) + load_addr;
 + error = load_addr;
...
   if (elf_interpreter) {
 - if (interpreter_type == INTERPRETER_AOUT)
 + if (interpreter_type == INTERPRETER_AOUT) {
   elf_entry = load_aout_interp(loc-interp_ex,
interpreter);
 - else
 + } else {
 + unsigned long interp_map_addr;  /* unused */
 +
   elf_entry = load_elf_interp(loc-interp_elf_ex,
   interpreter,
 - interp_load_addr);
 + interp_map_addr,
 + load_bias);
 + if (!BAD_ADDR(elf_entry)) {
 + /*
 +  * load_elf_interp() returns relocation
 +  * adjustment
 +  */
 + interp_load_addr = elf_entry;
 + elf_entry += loc-interp_elf_ex.e_entry;
 + }
 + }
   if (BAD_ADDR(elf_entry)) {
   force_sig(SIGSEGV, current);
   retval = IS_ERR((void *)elf_entry) ?

The above highlighted changes are the cause of random segfaults of PIE
binaries.  See
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=246623
The problem is if ld.so is prelinked to some address in the area where
the kernel actually maps it, particularly if elf_map in load_elf_interp
returns an address one page below its first PT_LOAD segments vaddr.
Then load_addr (it is a load bias actually) returned from load_elf_interp
is 0xf000 (on 32-bit kernels) and BAD_ADDR are all
addresses = 0xf000 (on i?86).
The fix should be either changing the definition of BAD_ADDR to
e.g. IS_ERR_VALUE(x), or at least changing the if (!BAD_ADDR(elf_entry)) {
above to if (!IS_ERR_VALUE(elf_entry)) {, the second BAD_ADDR can already
stay, because at that place elf_entry is no longer a bias (difference
between actual and preferred load address), but an actual address, where
very high addresses are of course invalid.

Jakub
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-07-04 Thread Jiri Kosina 
On Wed, 4 Jul 2007, Jakub Jelinek wrote:

 The above highlighted changes are the cause of random segfaults of PIE 
 binaries.  See 
 https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=246623 

Thanks a lot for pointing this out. Andrew, could this be folded into 
pie-randomization.patch please?


From: Jiri Kosina [EMAIL PROTECTED]

pie randomization: fix BAD_ADDR macro

pie-randomization.patch makes the load_addr in load_elf_interp() the load 
bias of ld.so (difference between the actual load base address and first 
PT_LOAD segment's p_vaddr). If the difference equals (on x86) to 
0xf000 (which is valid [1]), SIGSEGV is incorrectly sent.

This patch changes the BAD_ADDR so that it catches the mappings to the 
error-area properly.

[1] https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=246623

Reported-by: Jakub Jelinek [EMAIL PROTECTED]
Signed-off-by: Jiri Kosina [EMAIL PROTECTED]

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index da270d1..466477d 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -80,7 +80,7 @@ static struct linux_binfmt elf_format = {
.hasvdso= 1
 };
 
-#define BAD_ADDR(x) ((unsigned long)(x) = PAGE_MASK)
+#define BAD_ADDR(x) IS_ERR_VALUE(x)
 
 static int set_brk(unsigned long start, unsigned long end)
 {
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-23 Thread Jiri Kosina 
On Tue, 22 May 2007, Andrew Morton wrote:

> > This patch is using mmap()'s randomization functionality in such a way 
> > that it maps the main executable of (specially compiled/linked 
> > -pie/-fpie) ET_DYN binaries onto a random address (in cases in which 
> > mmap() is allowed to perform a randomization).
> ia64:
> arch/ia64/ia32/binfmt_elf32.c:265: error: conflicting types for 'elf32_map'
> arch/ia64/ia32/../../../fs/binfmt_elf.c:48: error: previous declaration of 
> 'elf32_map' was here
> arch/ia64/ia32/binfmt_elf32.c:265: error: conflicting types for 'elf32_map'
> arch/ia64/ia32/../../../fs/binfmt_elf.c:48: error: previous declaration of 
> 'elf32_map' was here
> arch/ia64/ia32/../../../fs/binfmt_elf.c:48: warning: 'elf32_map' declared 
> `static' but never defined
> arch/ia64/ia32/binfmt_elf32.c:265: warning: 'elf32_map' defined but not used

The fix to this is trivial - just fix the prototype in ia32 compat version 
of ia64 code. Updated patch is below.



From: Jan Kratochvil <[EMAIL PROTECTED]>

This patch is using mmap()'s randomization functionality in such a way 
that it maps the main executable of (specially compiled/linked -pie/-fpie) 
ET_DYN binaries onto a random address (in cases in which mmap() is allowed 
to perform a randomization).

Origin of this patch is in exec-shield
(http://people.redhat.com/mingo/exec-shield/)

Signed-off-by: Jan Kratochvil <[EMAIL PROTECTED]>
Signed-off-by: Jiri Kosina <[EMAIL PROTECTED]>
Cc: Ingo Molnar <[EMAIL PROTECTED]>
Cc: Roland McGrath <[EMAIL PROTECTED]>
Cc: Jakub Jelinek <[EMAIL PROTECTED]>

--- 

 arch/ia64/ia32/binfmt_elf32.c |2 +-
 fs/binfmt_elf.c   |  109 -
 2 files changed, 87 insertions(+), 24 deletions(-)

diff --git a/arch/ia64/ia32/binfmt_elf32.c b/arch/ia64/ia32/binfmt_elf32.c
index c05bda6..6f4d3d0 100644
--- a/arch/ia64/ia32/binfmt_elf32.c
+++ b/arch/ia64/ia32/binfmt_elf32.c
@@ -261,7 +261,7 @@ elf32_set_personality (void)
 }
 
 static unsigned long
-elf32_map (struct file *filep, unsigned long addr, struct elf_phdr *eppnt, int 
prot, int type)
+elf32_map (struct file *filep, unsigned long addr, struct elf_phdr *eppnt, int 
prot, int type, unsigned long unused)
 {
unsigned long pgoff = (eppnt->p_vaddr) & ~IA32_PAGE_MASK;
 
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index fa8ea33..abcac6a 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -45,7 +45,7 @@
 
 static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs);
 static int load_elf_library(struct file *);
-static unsigned long elf_map (struct file *, unsigned long, struct elf_phdr *, 
int, int);
+static unsigned long elf_map (struct file *, unsigned long, struct elf_phdr *, 
int, int, unsigned long);
 
 /*
  * If we don't support core dumping, then supply a NULL so we
@@ -80,7 +80,7 @@ static struct linux_binfmt elf_format =
.hasvdso= 1
 };
 
-#define BAD_ADDR(x) ((unsigned long)(x) >= TASK_SIZE)
+#define BAD_ADDR(x) ((unsigned long)(x) >= PAGE_MASK)
 
 static int set_brk(unsigned long start, unsigned long end)
 {
@@ -285,33 +285,70 @@ create_elf_tables(struct linux_binprm *b
 #ifndef elf_map
 
 static unsigned long elf_map(struct file *filep, unsigned long addr,
-   struct elf_phdr *eppnt, int prot, int type)
+   struct elf_phdr *eppnt, int prot, int type,
+   unsigned long total_size)
 {
unsigned long map_addr;
-   unsigned long pageoffset = ELF_PAGEOFFSET(eppnt->p_vaddr);
+   unsigned long size = eppnt->p_filesz + ELF_PAGEOFFSET(eppnt->p_vaddr);
+   unsigned long off = eppnt->p_offset - ELF_PAGEOFFSET(eppnt->p_vaddr);
+   addr = ELF_PAGESTART(addr);
+   size = ELF_PAGEALIGN(size);
 
-   down_write(>mm->mmap_sem);
/* mmap() will return -EINVAL if given a zero size, but a
 * segment with zero filesize is perfectly valid */
-   if (eppnt->p_filesz + pageoffset)
-   map_addr = do_mmap(filep, ELF_PAGESTART(addr),
-  eppnt->p_filesz + pageoffset, prot, type,
-  eppnt->p_offset - pageoffset);
-   else
-   map_addr = ELF_PAGESTART(addr);
+   if (!size)
+   return addr;
+
+   down_write(>mm->mmap_sem);
+   /*
+   * total_size is the size of the ELF (interpreter) image.
+   * The _first_ mmap needs to know the full size, otherwise
+   * randomization might put this image into an overlapping
+   * position with the ELF binary image. (since size < total_size)
+   * So we first map the 'big' image - and unmap the remainder at
+   * the end. (which unmap is needed for ELF images with holes.)
+   */
+   if (total_size) {
+   total_size = ELF_PAGEALIGN(total_size);
+   map_addr = do_mmap(filep, addr, total_size, prot, type, off);
+   if (!BAD_ADDR(map_addr))
+   do_munmap(current->mm, map_addr+size,

Re: [PATCH][RESEND] PIE randomization

2007-05-23 Thread Jiri Kosina 
On Tue, 22 May 2007, Andrew Morton wrote:

  This patch is using mmap()'s randomization functionality in such a way 
  that it maps the main executable of (specially compiled/linked 
  -pie/-fpie) ET_DYN binaries onto a random address (in cases in which 
  mmap() is allowed to perform a randomization).
 ia64:
 arch/ia64/ia32/binfmt_elf32.c:265: error: conflicting types for 'elf32_map'
 arch/ia64/ia32/../../../fs/binfmt_elf.c:48: error: previous declaration of 
 'elf32_map' was here
 arch/ia64/ia32/binfmt_elf32.c:265: error: conflicting types for 'elf32_map'
 arch/ia64/ia32/../../../fs/binfmt_elf.c:48: error: previous declaration of 
 'elf32_map' was here
 arch/ia64/ia32/../../../fs/binfmt_elf.c:48: warning: 'elf32_map' declared 
 `static' but never defined
 arch/ia64/ia32/binfmt_elf32.c:265: warning: 'elf32_map' defined but not used

The fix to this is trivial - just fix the prototype in ia32 compat version 
of ia64 code. Updated patch is below.



From: Jan Kratochvil [EMAIL PROTECTED]

This patch is using mmap()'s randomization functionality in such a way 
that it maps the main executable of (specially compiled/linked -pie/-fpie) 
ET_DYN binaries onto a random address (in cases in which mmap() is allowed 
to perform a randomization).

Origin of this patch is in exec-shield
(http://people.redhat.com/mingo/exec-shield/)

Signed-off-by: Jan Kratochvil [EMAIL PROTECTED]
Signed-off-by: Jiri Kosina [EMAIL PROTECTED]
Cc: Ingo Molnar [EMAIL PROTECTED]
Cc: Roland McGrath [EMAIL PROTECTED]
Cc: Jakub Jelinek [EMAIL PROTECTED]

--- 

 arch/ia64/ia32/binfmt_elf32.c |2 +-
 fs/binfmt_elf.c   |  109 -
 2 files changed, 87 insertions(+), 24 deletions(-)

diff --git a/arch/ia64/ia32/binfmt_elf32.c b/arch/ia64/ia32/binfmt_elf32.c
index c05bda6..6f4d3d0 100644
--- a/arch/ia64/ia32/binfmt_elf32.c
+++ b/arch/ia64/ia32/binfmt_elf32.c
@@ -261,7 +261,7 @@ elf32_set_personality (void)
 }
 
 static unsigned long
-elf32_map (struct file *filep, unsigned long addr, struct elf_phdr *eppnt, int 
prot, int type)
+elf32_map (struct file *filep, unsigned long addr, struct elf_phdr *eppnt, int 
prot, int type, unsigned long unused)
 {
unsigned long pgoff = (eppnt-p_vaddr)  ~IA32_PAGE_MASK;
 
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index fa8ea33..abcac6a 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -45,7 +45,7 @@
 
 static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs);
 static int load_elf_library(struct file *);
-static unsigned long elf_map (struct file *, unsigned long, struct elf_phdr *, 
int, int);
+static unsigned long elf_map (struct file *, unsigned long, struct elf_phdr *, 
int, int, unsigned long);
 
 /*
  * If we don't support core dumping, then supply a NULL so we
@@ -80,7 +80,7 @@ static struct linux_binfmt elf_format =
.hasvdso= 1
 };
 
-#define BAD_ADDR(x) ((unsigned long)(x) = TASK_SIZE)
+#define BAD_ADDR(x) ((unsigned long)(x) = PAGE_MASK)
 
 static int set_brk(unsigned long start, unsigned long end)
 {
@@ -285,33 +285,70 @@ create_elf_tables(struct linux_binprm *b
 #ifndef elf_map
 
 static unsigned long elf_map(struct file *filep, unsigned long addr,
-   struct elf_phdr *eppnt, int prot, int type)
+   struct elf_phdr *eppnt, int prot, int type,
+   unsigned long total_size)
 {
unsigned long map_addr;
-   unsigned long pageoffset = ELF_PAGEOFFSET(eppnt-p_vaddr);
+   unsigned long size = eppnt-p_filesz + ELF_PAGEOFFSET(eppnt-p_vaddr);
+   unsigned long off = eppnt-p_offset - ELF_PAGEOFFSET(eppnt-p_vaddr);
+   addr = ELF_PAGESTART(addr);
+   size = ELF_PAGEALIGN(size);
 
-   down_write(current-mm-mmap_sem);
/* mmap() will return -EINVAL if given a zero size, but a
 * segment with zero filesize is perfectly valid */
-   if (eppnt-p_filesz + pageoffset)
-   map_addr = do_mmap(filep, ELF_PAGESTART(addr),
-  eppnt-p_filesz + pageoffset, prot, type,
-  eppnt-p_offset - pageoffset);
-   else
-   map_addr = ELF_PAGESTART(addr);
+   if (!size)
+   return addr;
+
+   down_write(current-mm-mmap_sem);
+   /*
+   * total_size is the size of the ELF (interpreter) image.
+   * The _first_ mmap needs to know the full size, otherwise
+   * randomization might put this image into an overlapping
+   * position with the ELF binary image. (since size  total_size)
+   * So we first map the 'big' image - and unmap the remainder at
+   * the end. (which unmap is needed for ELF images with holes.)
+   */
+   if (total_size) {
+   total_size = ELF_PAGEALIGN(total_size);
+   map_addr = do_mmap(filep, addr, total_size, prot, type, off);
+   if (!BAD_ADDR(map_addr))
+   do_munmap(current-mm, map_addr+size, total_size-size);
+   } else

Re: [PATCH][RESEND] PIE randomization

2007-05-22 Thread Andrew Morton 
On Thu, 17 May 2007 22:24:11 +0200 (CEST)
Jan Kratochvil <[EMAIL PROTECTED]> wrote:

> This patch is using mmap()'s randomization functionality in such a way 
> that it maps the main executable of (specially compiled/linked -pie/-fpie) 
> ET_DYN binaries onto a random address (in cases in which mmap() is allowed 
> to perform a randomization).

ia64:

arch/ia64/ia32/binfmt_elf32.c:265: error: conflicting types for 'elf32_map'
arch/ia64/ia32/../../../fs/binfmt_elf.c:48: error: previous declaration of 
'elf32_map' was here
arch/ia64/ia32/binfmt_elf32.c:265: error: conflicting types for 'elf32_map'
arch/ia64/ia32/../../../fs/binfmt_elf.c:48: error: previous declaration of 
'elf32_map' was here
arch/ia64/ia32/../../../fs/binfmt_elf.c:48: warning: 'elf32_map' declared 
`static' but never defined
arch/ia64/ia32/binfmt_elf32.c:265: warning: 'elf32_map' defined but not used
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-22 Thread Andrew Morton 
On Thu, 17 May 2007 22:24:11 +0200 (CEST)
Jan Kratochvil [EMAIL PROTECTED] wrote:

 This patch is using mmap()'s randomization functionality in such a way 
 that it maps the main executable of (specially compiled/linked -pie/-fpie) 
 ET_DYN binaries onto a random address (in cases in which mmap() is allowed 
 to perform a randomization).

ia64:

arch/ia64/ia32/binfmt_elf32.c:265: error: conflicting types for 'elf32_map'
arch/ia64/ia32/../../../fs/binfmt_elf.c:48: error: previous declaration of 
'elf32_map' was here
arch/ia64/ia32/binfmt_elf32.c:265: error: conflicting types for 'elf32_map'
arch/ia64/ia32/../../../fs/binfmt_elf.c:48: error: previous declaration of 
'elf32_map' was here
arch/ia64/ia32/../../../fs/binfmt_elf.c:48: warning: 'elf32_map' declared 
`static' but never defined
arch/ia64/ia32/binfmt_elf32.c:265: warning: 'elf32_map' defined but not used
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-21 Thread Hugh Dickins 
On Thu, 17 May 2007, Jan Kratochvil wrote:

> Hi,
>  sorry for insufficient description in my earlier post. I hope it is better
> this time.
> 
> Jiri: Thanks for help, I applied your change on my previous patch.
> 
> This patch is using mmap()'s randomization functionality in such a way that it
> maps the main executable of (specially compiled/linked -pie/-fpie) ET_DYN
> binaries onto a random address (in cases in which mmap() is allowed to perform
> a randomization).
> 
> Origin of this patch is in exec-shield
> (http://people.redhat.com/mingo/exec-shield/)
> 
> Signed-off-by: Jan Kratochvil <[EMAIL PROTECTED]>

I haven't reviewed this patch, but I have given it the same testing
as Marcus' earlier PIE randomization patch, which had been heading
into 2.6.20 until I reported strange build failures from it on i386,
which went away when it got reverted.  This PIE randomization patch
from Jan gives me no problems (beyond s/^  / / to get it to apply).

Hugh

> 
> ---
>  fs/binfmt_elf.c |   99 ++
>  1 files changed, 77 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index fa8ea33..8406f9a 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -45,7 +45,7 @@
> 
>  static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs);
>  static int load_elf_library(struct file *);
> -static unsigned long elf_map (struct file *, unsigned long, struct elf_phdr
> *, int, int);
> +static unsigned long elf_map (struct file *, unsigned long, struct elf_phdr
> *, int, int, unsigned long);
> 
>  /*
>   * If we don't support core dumping, then supply a NULL so we
> @@ -80,7 +80,7 @@ static struct linux_binfmt elf_format =
>   .hasvdso= 1
>  };
> 
> -#define BAD_ADDR(x) ((unsigned long)(x) >= TASK_SIZE)
> +#define BAD_ADDR(x) ((unsigned long)(x) >= PAGE_MASK)
> 
>  static int set_brk(unsigned long start, unsigned long end)
>  {
> @@ -285,33 +285,70 @@ create_elf_tables(struct linux_binprm *b
>  #ifndef elf_map
> 
>  static unsigned long elf_map(struct file *filep, unsigned long addr,
> - struct elf_phdr *eppnt, int prot, int type)
> + struct elf_phdr *eppnt, int prot, int type,
> + unsigned long total_size)
>  {
>   unsigned long map_addr;
> - unsigned long pageoffset = ELF_PAGEOFFSET(eppnt->p_vaddr);
> + unsigned long size = eppnt->p_filesz + ELF_PAGEOFFSET(eppnt->p_vaddr);
> + unsigned long off = eppnt->p_offset - ELF_PAGEOFFSET(eppnt->p_vaddr);
> + addr = ELF_PAGESTART(addr);
> + size = ELF_PAGEALIGN(size);
> 
> - down_write(>mm->mmap_sem);
>   /* mmap() will return -EINVAL if given a zero size, but a
>* segment with zero filesize is perfectly valid */
> - if (eppnt->p_filesz + pageoffset)
> - map_addr = do_mmap(filep, ELF_PAGESTART(addr),
> -eppnt->p_filesz + pageoffset, prot, type,
> -eppnt->p_offset - pageoffset);
> - else
> - map_addr = ELF_PAGESTART(addr);
> + if (!size)
> + return addr;
> +
> + down_write(>mm->mmap_sem);
> + /*
> + * total_size is the size of the ELF (interpreter) image.
> + * The _first_ mmap needs to know the full size, otherwise
> + * randomization might put this image into an overlapping
> + * position with the ELF binary image. (since size < total_size)
> + * So we first map the 'big' image - and unmap the remainder at
> + * the end. (which unmap is needed for ELF images with holes.)
> + */
> + if (total_size) {
> + total_size = ELF_PAGEALIGN(total_size);
> + map_addr = do_mmap(filep, addr, total_size, prot, type, off);
> + if (!BAD_ADDR(map_addr))
> + do_munmap(current->mm, map_addr+size,
> total_size-size);
> + } else
> + map_addr = do_mmap(filep, addr, size, prot, type, off);
> +
>   up_write(>mm->mmap_sem);
>   return(map_addr);
>  }
> 
>  #endif /* !elf_map */
> 
> +static inline unsigned long total_mapping_size(struct elf_phdr *cmds, int nr)
> +{
> + int i, first_idx = -1, last_idx = -1;
> +
> + for (i = 0; i < nr; i++)
> + if (cmds[i].p_type == PT_LOAD) {
> + last_idx = i;
> + if (first_idx == -1)
> + first_idx = i;
> + }
> +
> + if (first_idx == -1)
> + return 0;
> +
> + return cmds[last_idx].p_vaddr + cmds[last_idx].p_memsz -
> + ELF_PAGESTART(cmds[first_idx].p_vaddr);
> +}
> +
> +
>  /* This is much more generalized than the library routine read function,
> so we keep this separate.  Technically the library read function
> is only provided so that we can read a.out libraries that have
> an ELF header */
> 
>  static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
> - struct file

Re: [PATCH][RESEND] PIE randomization

2007-05-21 Thread Hugh Dickins 
On Thu, 17 May 2007, Jan Kratochvil wrote:

 Hi,
  sorry for insufficient description in my earlier post. I hope it is better
 this time.
 
 Jiri: Thanks for help, I applied your change on my previous patch.
 
 This patch is using mmap()'s randomization functionality in such a way that it
 maps the main executable of (specially compiled/linked -pie/-fpie) ET_DYN
 binaries onto a random address (in cases in which mmap() is allowed to perform
 a randomization).
 
 Origin of this patch is in exec-shield
 (http://people.redhat.com/mingo/exec-shield/)
 
 Signed-off-by: Jan Kratochvil [EMAIL PROTECTED]

I haven't reviewed this patch, but I have given it the same testing
as Marcus' earlier PIE randomization patch, which had been heading
into 2.6.20 until I reported strange build failures from it on i386,
which went away when it got reverted.  This PIE randomization patch
from Jan gives me no problems (beyond s/^  / / to get it to apply).

Hugh

 
 ---
  fs/binfmt_elf.c |   99 ++
  1 files changed, 77 insertions(+), 22 deletions(-)
 
 diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
 index fa8ea33..8406f9a 100644
 --- a/fs/binfmt_elf.c
 +++ b/fs/binfmt_elf.c
 @@ -45,7 +45,7 @@
 
  static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs);
  static int load_elf_library(struct file *);
 -static unsigned long elf_map (struct file *, unsigned long, struct elf_phdr
 *, int, int);
 +static unsigned long elf_map (struct file *, unsigned long, struct elf_phdr
 *, int, int, unsigned long);
 
  /*
   * If we don't support core dumping, then supply a NULL so we
 @@ -80,7 +80,7 @@ static struct linux_binfmt elf_format =
   .hasvdso= 1
  };
 
 -#define BAD_ADDR(x) ((unsigned long)(x) = TASK_SIZE)
 +#define BAD_ADDR(x) ((unsigned long)(x) = PAGE_MASK)
 
  static int set_brk(unsigned long start, unsigned long end)
  {
 @@ -285,33 +285,70 @@ create_elf_tables(struct linux_binprm *b
  #ifndef elf_map
 
  static unsigned long elf_map(struct file *filep, unsigned long addr,
 - struct elf_phdr *eppnt, int prot, int type)
 + struct elf_phdr *eppnt, int prot, int type,
 + unsigned long total_size)
  {
   unsigned long map_addr;
 - unsigned long pageoffset = ELF_PAGEOFFSET(eppnt-p_vaddr);
 + unsigned long size = eppnt-p_filesz + ELF_PAGEOFFSET(eppnt-p_vaddr);
 + unsigned long off = eppnt-p_offset - ELF_PAGEOFFSET(eppnt-p_vaddr);
 + addr = ELF_PAGESTART(addr);
 + size = ELF_PAGEALIGN(size);
 
 - down_write(current-mm-mmap_sem);
   /* mmap() will return -EINVAL if given a zero size, but a
* segment with zero filesize is perfectly valid */
 - if (eppnt-p_filesz + pageoffset)
 - map_addr = do_mmap(filep, ELF_PAGESTART(addr),
 -eppnt-p_filesz + pageoffset, prot, type,
 -eppnt-p_offset - pageoffset);
 - else
 - map_addr = ELF_PAGESTART(addr);
 + if (!size)
 + return addr;
 +
 + down_write(current-mm-mmap_sem);
 + /*
 + * total_size is the size of the ELF (interpreter) image.
 + * The _first_ mmap needs to know the full size, otherwise
 + * randomization might put this image into an overlapping
 + * position with the ELF binary image. (since size  total_size)
 + * So we first map the 'big' image - and unmap the remainder at
 + * the end. (which unmap is needed for ELF images with holes.)
 + */
 + if (total_size) {
 + total_size = ELF_PAGEALIGN(total_size);
 + map_addr = do_mmap(filep, addr, total_size, prot, type, off);
 + if (!BAD_ADDR(map_addr))
 + do_munmap(current-mm, map_addr+size,
 total_size-size);
 + } else
 + map_addr = do_mmap(filep, addr, size, prot, type, off);
 +
   up_write(current-mm-mmap_sem);
   return(map_addr);
  }
 
  #endif /* !elf_map */
 
 +static inline unsigned long total_mapping_size(struct elf_phdr *cmds, int nr)
 +{
 + int i, first_idx = -1, last_idx = -1;
 +
 + for (i = 0; i  nr; i++)
 + if (cmds[i].p_type == PT_LOAD) {
 + last_idx = i;
 + if (first_idx == -1)
 + first_idx = i;
 + }
 +
 + if (first_idx == -1)
 + return 0;
 +
 + return cmds[last_idx].p_vaddr + cmds[last_idx].p_memsz -
 + ELF_PAGESTART(cmds[first_idx].p_vaddr);
 +}
 +
 +
  /* This is much more generalized than the library routine read function,
 so we keep this separate.  Technically the library read function
 is only provided so that we can read a.out libraries that have
 an ELF header */
 
  static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
 - struct file *interpreter, unsigned long *interp_load_addr)
 + struct file *interpreter, unsigned long *interp_map_addr,

Re: [PATCH][RESEND] PIE randomization

2007-05-18 Thread Andrew Morton 
On Thu, 17 May 2007 22:24:11 +0200 (CEST)
Jan Kratochvil <[EMAIL PROTECTED]> wrote:

> This patch is using mmap()'s randomization functionality in such a way 
> that it maps the main executable of (specially compiled/linked -pie/-fpie) 
> ET_DYN binaries onto a random address (in cases in which mmap() is allowed 
> to perform a randomization).
> 
> Origin of this patch is in exec-shield 
> (http://people.redhat.com/mingo/exec-shield/)


From: Andrew Morton <[EMAIL PROTECTED]>

- the compiler knows how to inline things

- return -EINVAL on zero-size, not -EIO

- reduce scope of local `interp_map_addr', remove unneeded initialisation,
  add needed comment.

- coding-style repairs

Cc: Jan Kratochvil <[EMAIL PROTECTED]>
Cc: Jiri Kosina <[EMAIL PROTECTED]>
Cc: Ingo Molnar <[EMAIL PROTECTED]>
Cc: Roland McGrath <[EMAIL PROTECTED]>
Cc: Jakub Jelinek <[EMAIL PROTECTED]>
Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
---

 fs/binfmt_elf.c |   26 +-
 1 files changed, 17 insertions(+), 9 deletions(-)

diff -puN fs/binfmt_elf.c~pie-randomization-fix fs/binfmt_elf.c
--- a/fs/binfmt_elf.c~pie-randomization-fix
+++ a/fs/binfmt_elf.c
@@ -322,17 +322,17 @@ static unsigned long elf_map(struct file
 
 #endif /* !elf_map */
 
-static inline unsigned long total_mapping_size(struct elf_phdr *cmds, int nr)
+static unsigned long total_mapping_size(struct elf_phdr *cmds, int nr)
 {
int i, first_idx = -1, last_idx = -1;
 
-   for (i = 0; i < nr; i++)
+   for (i = 0; i < nr; i++) {
if (cmds[i].p_type == PT_LOAD) {
last_idx = i;
if (first_idx == -1)
first_idx = i;
}
-
+   }
if (first_idx == -1)
return 0;
 
@@ -396,8 +396,10 @@ static unsigned long load_elf_interp(str
}
 
total_size = total_mapping_size(elf_phdata, interp_elf_ex->e_phnum);
-   if (!total_size)
+   if (!total_size) {
+   error = -EINVAL;
goto out_close;
+   }
 
eppnt = elf_phdata;
for (i = 0; i < interp_elf_ex->e_phnum; i++, eppnt++) {
@@ -586,7 +588,8 @@ static int load_elf_binary(struct linux_
int elf_exec_fileno;
int retval, i;
unsigned int size;
-   unsigned long elf_entry, interp_load_addr = 0, interp_map_addr = 0;
+   unsigned long elf_entry;
+   unsigned long interp_load_addr = 0;
unsigned long start_code, end_code, start_data, end_data;
unsigned long reloc_func_desc = 0;
char passed_fileno[6];
@@ -908,7 +911,7 @@ static int load_elf_binary(struct linux_
 * default mmap base, as well as whatever program they
 * might try to exec.  This is because the brk will
 * follow the loader, and is not movable.  */
-#if defined(__i386__) || defined(__x86_64__)
+#ifdef CONFIG_X86
load_bias = 0;
 #else
load_bias = ELF_PAGESTART(ELF_ET_DYN_BASE - vaddr);
@@ -992,16 +995,21 @@ static int load_elf_binary(struct linux_
}
 
if (elf_interpreter) {
-   if (interpreter_type == INTERPRETER_AOUT)
+   if (interpreter_type == INTERPRETER_AOUT) {
elf_entry = load_aout_interp(>interp_ex,
 interpreter);
-   else {
+   } else {
+   unsigned long interp_map_addr;  /* unused */
+
elf_entry = load_elf_interp(>interp_elf_ex,
interpreter,
_map_addr,
load_bias);
if (!BAD_ADDR(elf_entry)) {
-   /* load_elf_interp() returns relocation 
adjustment */
+   /*
+* load_elf_interp() returns relocation
+* adjustment
+*/
interp_load_addr = elf_entry;
elf_entry += loc->interp_elf_ex.e_entry;
}
_

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-18 Thread Andrew Morton 
On Thu, 17 May 2007 22:24:11 +0200 (CEST)
Jan Kratochvil [EMAIL PROTECTED] wrote:

 This patch is using mmap()'s randomization functionality in such a way 
 that it maps the main executable of (specially compiled/linked -pie/-fpie) 
 ET_DYN binaries onto a random address (in cases in which mmap() is allowed 
 to perform a randomization).
 
 Origin of this patch is in exec-shield 
 (http://people.redhat.com/mingo/exec-shield/)


From: Andrew Morton [EMAIL PROTECTED]

- the compiler knows how to inline things

- return -EINVAL on zero-size, not -EIO

- reduce scope of local `interp_map_addr', remove unneeded initialisation,
  add needed comment.

- coding-style repairs

Cc: Jan Kratochvil [EMAIL PROTECTED]
Cc: Jiri Kosina [EMAIL PROTECTED]
Cc: Ingo Molnar [EMAIL PROTECTED]
Cc: Roland McGrath [EMAIL PROTECTED]
Cc: Jakub Jelinek [EMAIL PROTECTED]
Signed-off-by: Andrew Morton [EMAIL PROTECTED]
---

 fs/binfmt_elf.c |   26 +-
 1 files changed, 17 insertions(+), 9 deletions(-)

diff -puN fs/binfmt_elf.c~pie-randomization-fix fs/binfmt_elf.c
--- a/fs/binfmt_elf.c~pie-randomization-fix
+++ a/fs/binfmt_elf.c
@@ -322,17 +322,17 @@ static unsigned long elf_map(struct file
 
 #endif /* !elf_map */
 
-static inline unsigned long total_mapping_size(struct elf_phdr *cmds, int nr)
+static unsigned long total_mapping_size(struct elf_phdr *cmds, int nr)
 {
int i, first_idx = -1, last_idx = -1;
 
-   for (i = 0; i  nr; i++)
+   for (i = 0; i  nr; i++) {
if (cmds[i].p_type == PT_LOAD) {
last_idx = i;
if (first_idx == -1)
first_idx = i;
}
-
+   }
if (first_idx == -1)
return 0;
 
@@ -396,8 +396,10 @@ static unsigned long load_elf_interp(str
}
 
total_size = total_mapping_size(elf_phdata, interp_elf_ex-e_phnum);
-   if (!total_size)
+   if (!total_size) {
+   error = -EINVAL;
goto out_close;
+   }
 
eppnt = elf_phdata;
for (i = 0; i  interp_elf_ex-e_phnum; i++, eppnt++) {
@@ -586,7 +588,8 @@ static int load_elf_binary(struct linux_
int elf_exec_fileno;
int retval, i;
unsigned int size;
-   unsigned long elf_entry, interp_load_addr = 0, interp_map_addr = 0;
+   unsigned long elf_entry;
+   unsigned long interp_load_addr = 0;
unsigned long start_code, end_code, start_data, end_data;
unsigned long reloc_func_desc = 0;
char passed_fileno[6];
@@ -908,7 +911,7 @@ static int load_elf_binary(struct linux_
 * default mmap base, as well as whatever program they
 * might try to exec.  This is because the brk will
 * follow the loader, and is not movable.  */
-#if defined(__i386__) || defined(__x86_64__)
+#ifdef CONFIG_X86
load_bias = 0;
 #else
load_bias = ELF_PAGESTART(ELF_ET_DYN_BASE - vaddr);
@@ -992,16 +995,21 @@ static int load_elf_binary(struct linux_
}
 
if (elf_interpreter) {
-   if (interpreter_type == INTERPRETER_AOUT)
+   if (interpreter_type == INTERPRETER_AOUT) {
elf_entry = load_aout_interp(loc-interp_ex,
 interpreter);
-   else {
+   } else {
+   unsigned long interp_map_addr;  /* unused */
+
elf_entry = load_elf_interp(loc-interp_elf_ex,
interpreter,
interp_map_addr,
load_bias);
if (!BAD_ADDR(elf_entry)) {
-   /* load_elf_interp() returns relocation 
adjustment */
+   /*
+* load_elf_interp() returns relocation
+* adjustment
+*/
interp_load_addr = elf_entry;
elf_entry += loc-interp_elf_ex.e_entry;
}
_

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-17 Thread Jiri Kosina 
On Thu, 17 May 2007, Jan Kratochvil wrote:

> This patch is using mmap()'s randomization functionality in such a way 
> that it maps the main executable of (specially compiled/linked 
> -pie/-fpie) ET_DYN binaries onto a random address (in cases in which 
> mmap() is allowed to perform a randomization). Origin of this patch is 
> in exec-shield (http://people.redhat.com/mingo/exec-shield/)
> Signed-off-by: Jan Kratochvil <[EMAIL PROTECTED]>

Andrew,

if you are going to merge this, you could add

Signed-off-by: Jiri Kosina <[EMAIL PROTECTED]>

to this patch, if it makes any difference.

Ingo, do you have any opinion on merging this upstream please?

Thanks,

-- 
Jiri Kosina
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-17 Thread Jan Kratochvil 

Hi,
 sorry for insufficient description in my earlier post. I hope it is better
this time.

Jiri: Thanks for help, I applied your change on my previous patch.

This patch is using mmap()'s randomization functionality in such a way 
that it maps the main executable of (specially compiled/linked -pie/-fpie) 
ET_DYN binaries onto a random address (in cases in which mmap() is allowed 
to perform a randomization).


Origin of this patch is in exec-shield 
(http://people.redhat.com/mingo/exec-shield/)

Signed-off-by: Jan Kratochvil <[EMAIL PROTECTED]>

---
 fs/binfmt_elf.c |   99 ++
 1 files changed, 77 insertions(+), 22 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index fa8ea33..8406f9a 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -45,7 +45,7 @@

 static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs);
 static int load_elf_library(struct file *);
-static unsigned long elf_map (struct file *, unsigned long, struct elf_phdr *, 
int, int);
+static unsigned long elf_map (struct file *, unsigned long, struct elf_phdr *, 
int, int, unsigned long);

 /*
  * If we don't support core dumping, then supply a NULL so we
@@ -80,7 +80,7 @@ static struct linux_binfmt elf_format =
.hasvdso= 1
 };

-#define BAD_ADDR(x) ((unsigned long)(x) >= TASK_SIZE)
+#define BAD_ADDR(x) ((unsigned long)(x) >= PAGE_MASK)

 static int set_brk(unsigned long start, unsigned long end)
 {
@@ -285,33 +285,70 @@ create_elf_tables(struct linux_binprm *b
 #ifndef elf_map

 static unsigned long elf_map(struct file *filep, unsigned long addr,
-   struct elf_phdr *eppnt, int prot, int type)
+   struct elf_phdr *eppnt, int prot, int type,
+   unsigned long total_size)
 {
unsigned long map_addr;
-   unsigned long pageoffset = ELF_PAGEOFFSET(eppnt->p_vaddr);
+   unsigned long size = eppnt->p_filesz + ELF_PAGEOFFSET(eppnt->p_vaddr);
+   unsigned long off = eppnt->p_offset - ELF_PAGEOFFSET(eppnt->p_vaddr);
+   addr = ELF_PAGESTART(addr);
+   size = ELF_PAGEALIGN(size);

-   down_write(>mm->mmap_sem);
/* mmap() will return -EINVAL if given a zero size, but a
 * segment with zero filesize is perfectly valid */
-   if (eppnt->p_filesz + pageoffset)
-   map_addr = do_mmap(filep, ELF_PAGESTART(addr),
-  eppnt->p_filesz + pageoffset, prot, type,
-  eppnt->p_offset - pageoffset);
-   else
-   map_addr = ELF_PAGESTART(addr);
+   if (!size)
+   return addr;
+
+   down_write(>mm->mmap_sem);
+   /*
+   * total_size is the size of the ELF (interpreter) image.
+   * The _first_ mmap needs to know the full size, otherwise
+   * randomization might put this image into an overlapping
+   * position with the ELF binary image. (since size < total_size)
+   * So we first map the 'big' image - and unmap the remainder at
+   * the end. (which unmap is needed for ELF images with holes.)
+   */
+   if (total_size) {
+   total_size = ELF_PAGEALIGN(total_size);
+   map_addr = do_mmap(filep, addr, total_size, prot, type, off);
+   if (!BAD_ADDR(map_addr))
+   do_munmap(current->mm, map_addr+size, total_size-size);
+   } else
+   map_addr = do_mmap(filep, addr, size, prot, type, off);
+
up_write(>mm->mmap_sem);
return(map_addr);
 }

 #endif /* !elf_map */

+static inline unsigned long total_mapping_size(struct elf_phdr *cmds, int nr)
+{
+   int i, first_idx = -1, last_idx = -1;
+
+   for (i = 0; i < nr; i++)
+   if (cmds[i].p_type == PT_LOAD) {
+   last_idx = i;
+   if (first_idx == -1)
+   first_idx = i;
+   }
+
+   if (first_idx == -1)
+   return 0;
+
+   return cmds[last_idx].p_vaddr + cmds[last_idx].p_memsz -
+   ELF_PAGESTART(cmds[first_idx].p_vaddr);
+}
+
+
 /* This is much more generalized than the library routine read function,
so we keep this separate.  Technically the library read function
is only provided so that we can read a.out libraries that have
an ELF header */

 static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
-   struct file *interpreter, unsigned long *interp_load_addr)
+   struct file *interpreter, unsigned long *interp_map_addr,
+   unsigned long no_base)
 {
struct elf_phdr *elf_phdata;
struct elf_phdr *eppnt;
@@ -319,6 +356,7 @@ static unsigned long load_elf_interp(str
int load_addr_set = 0;
unsigned long last_bss = 0, elf_bss = 0;
unsigned long error = ~0UL;
+   unsigned long total_size;
int retval, i, size;

/* First of all, some simple consistency

Re: [PATCH][RESEND] PIE randomization

2007-05-17 Thread Jan Kratochvil 

Hi,
 sorry for insufficient description in my earlier post. I hope it is better
this time.

Jiri: Thanks for help, I applied your change on my previous patch.

This patch is using mmap()'s randomization functionality in such a way 
that it maps the main executable of (specially compiled/linked -pie/-fpie) 
ET_DYN binaries onto a random address (in cases in which mmap() is allowed 
to perform a randomization).


Origin of this patch is in exec-shield 
(http://people.redhat.com/mingo/exec-shield/)

Signed-off-by: Jan Kratochvil [EMAIL PROTECTED]

---
 fs/binfmt_elf.c |   99 ++
 1 files changed, 77 insertions(+), 22 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index fa8ea33..8406f9a 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -45,7 +45,7 @@

 static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs);
 static int load_elf_library(struct file *);
-static unsigned long elf_map (struct file *, unsigned long, struct elf_phdr *, 
int, int);
+static unsigned long elf_map (struct file *, unsigned long, struct elf_phdr *, 
int, int, unsigned long);

 /*
  * If we don't support core dumping, then supply a NULL so we
@@ -80,7 +80,7 @@ static struct linux_binfmt elf_format =
.hasvdso= 1
 };

-#define BAD_ADDR(x) ((unsigned long)(x) = TASK_SIZE)
+#define BAD_ADDR(x) ((unsigned long)(x) = PAGE_MASK)

 static int set_brk(unsigned long start, unsigned long end)
 {
@@ -285,33 +285,70 @@ create_elf_tables(struct linux_binprm *b
 #ifndef elf_map

 static unsigned long elf_map(struct file *filep, unsigned long addr,
-   struct elf_phdr *eppnt, int prot, int type)
+   struct elf_phdr *eppnt, int prot, int type,
+   unsigned long total_size)
 {
unsigned long map_addr;
-   unsigned long pageoffset = ELF_PAGEOFFSET(eppnt-p_vaddr);
+   unsigned long size = eppnt-p_filesz + ELF_PAGEOFFSET(eppnt-p_vaddr);
+   unsigned long off = eppnt-p_offset - ELF_PAGEOFFSET(eppnt-p_vaddr);
+   addr = ELF_PAGESTART(addr);
+   size = ELF_PAGEALIGN(size);

-   down_write(current-mm-mmap_sem);
/* mmap() will return -EINVAL if given a zero size, but a
 * segment with zero filesize is perfectly valid */
-   if (eppnt-p_filesz + pageoffset)
-   map_addr = do_mmap(filep, ELF_PAGESTART(addr),
-  eppnt-p_filesz + pageoffset, prot, type,
-  eppnt-p_offset - pageoffset);
-   else
-   map_addr = ELF_PAGESTART(addr);
+   if (!size)
+   return addr;
+
+   down_write(current-mm-mmap_sem);
+   /*
+   * total_size is the size of the ELF (interpreter) image.
+   * The _first_ mmap needs to know the full size, otherwise
+   * randomization might put this image into an overlapping
+   * position with the ELF binary image. (since size  total_size)
+   * So we first map the 'big' image - and unmap the remainder at
+   * the end. (which unmap is needed for ELF images with holes.)
+   */
+   if (total_size) {
+   total_size = ELF_PAGEALIGN(total_size);
+   map_addr = do_mmap(filep, addr, total_size, prot, type, off);
+   if (!BAD_ADDR(map_addr))
+   do_munmap(current-mm, map_addr+size, total_size-size);
+   } else
+   map_addr = do_mmap(filep, addr, size, prot, type, off);
+
up_write(current-mm-mmap_sem);
return(map_addr);
 }

 #endif /* !elf_map */

+static inline unsigned long total_mapping_size(struct elf_phdr *cmds, int nr)
+{
+   int i, first_idx = -1, last_idx = -1;
+
+   for (i = 0; i  nr; i++)
+   if (cmds[i].p_type == PT_LOAD) {
+   last_idx = i;
+   if (first_idx == -1)
+   first_idx = i;
+   }
+
+   if (first_idx == -1)
+   return 0;
+
+   return cmds[last_idx].p_vaddr + cmds[last_idx].p_memsz -
+   ELF_PAGESTART(cmds[first_idx].p_vaddr);
+}
+
+
 /* This is much more generalized than the library routine read function,
so we keep this separate.  Technically the library read function
is only provided so that we can read a.out libraries that have
an ELF header */

 static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
-   struct file *interpreter, unsigned long *interp_load_addr)
+   struct file *interpreter, unsigned long *interp_map_addr,
+   unsigned long no_base)
 {
struct elf_phdr *elf_phdata;
struct elf_phdr *eppnt;
@@ -319,6 +356,7 @@ static unsigned long load_elf_interp(str
int load_addr_set = 0;
unsigned long last_bss = 0, elf_bss = 0;
unsigned long error = ~0UL;
+   unsigned long total_size;
int retval, i, size;

/* First of all, some simple consistency

Re: [PATCH][RESEND] PIE randomization

2007-05-17 Thread Jiri Kosina 
On Thu, 17 May 2007, Jan Kratochvil wrote:

 This patch is using mmap()'s randomization functionality in such a way 
 that it maps the main executable of (specially compiled/linked 
 -pie/-fpie) ET_DYN binaries onto a random address (in cases in which 
 mmap() is allowed to perform a randomization). Origin of this patch is 
 in exec-shield (http://people.redhat.com/mingo/exec-shield/)
 Signed-off-by: Jan Kratochvil [EMAIL PROTECTED]

Andrew,

if you are going to merge this, you could add

Signed-off-by: Jiri Kosina [EMAIL PROTECTED]

to this patch, if it makes any difference.

Ingo, do you have any opinion on merging this upstream please?

Thanks,

-- 
Jiri Kosina
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-16 Thread Jiri Kosina 
On Sat, 12 May 2007, Jiri Kosina wrote:

> However, I seem to get "soft" hang on boot with this patch, 
> approximately at the time the init should be executed. The system is not 
> completely stuck - interrupts are delivered, keyboard is working, 
> alt-sysrq-t dumps proper output, but userspace doesn't seem to get 
> started. This happens on i386, didn't try on other archs.

Hi Jan,

I finally had time to look at it a little bit - I think you omitted 
porting of proper handling of *interp_load_addr == 0, which made my box 
hang. The patch below, when applied on top of what you have sent, makes it 
work again and also the randomization for ET_DYN executables seems to work 
OK. 

Could you please refresh your patch, update the Changelog in a proper way 
and resubmit?

Thanks.


diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index be6671e..8406f9a 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -347,7 +347,7 @@ static inline unsigned long total_mappin
an ELF header */
 
 static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
-   struct file *interpreter, unsigned long *interp_load_addr,
+   struct file *interpreter, unsigned long *interp_map_addr,
unsigned long no_base)
 {
struct elf_phdr *elf_phdata;
@@ -421,6 +421,9 @@ static unsigned long load_elf_interp(str
 
map_addr = elf_map(interpreter, load_addr + vaddr,
   eppnt, elf_prot, elf_type, 
total_size);
+   total_size = 0;
+   if (!*interp_map_addr)
+   *interp_map_addr = map_addr;
error = map_addr;
if (BAD_ADDR(map_addr))
goto out_close;


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-16 Thread Jiri Kosina 
On Sat, 12 May 2007, Jiri Kosina wrote:

 However, I seem to get soft hang on boot with this patch, 
 approximately at the time the init should be executed. The system is not 
 completely stuck - interrupts are delivered, keyboard is working, 
 alt-sysrq-t dumps proper output, but userspace doesn't seem to get 
 started. This happens on i386, didn't try on other archs.

Hi Jan,

I finally had time to look at it a little bit - I think you omitted 
porting of proper handling of *interp_load_addr == 0, which made my box 
hang. The patch below, when applied on top of what you have sent, makes it 
work again and also the randomization for ET_DYN executables seems to work 
OK. 

Could you please refresh your patch, update the Changelog in a proper way 
and resubmit?

Thanks.


diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index be6671e..8406f9a 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -347,7 +347,7 @@ static inline unsigned long total_mappin
an ELF header */
 
 static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
-   struct file *interpreter, unsigned long *interp_load_addr,
+   struct file *interpreter, unsigned long *interp_map_addr,
unsigned long no_base)
 {
struct elf_phdr *elf_phdata;
@@ -421,6 +421,9 @@ static unsigned long load_elf_interp(str
 
map_addr = elf_map(interpreter, load_addr + vaddr,
   eppnt, elf_prot, elf_type, 
total_size);
+   total_size = 0;
+   if (!*interp_map_addr)
+   *interp_map_addr = map_addr;
error = map_addr;
if (BAD_ADDR(map_addr))
goto out_close;


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-11 Thread Jiri Kosina 
On Fri, 11 May 2007, Andrew Morton wrote:

> I could reverse-engineer that info from the patch, I guess, but I'd 
> prefer to go in the opposite direction: you tell us what the patch is 
> trying to do, then we look at it and see if we agree that it is in fact 
> doing that.

I've just quickly looked at the patch and it seems fine - it's using 
mmap()'s randomization functionality in such a way that it maps the the 
main executable of (specially compiled/linked) ET_DYN binaries onto a 
random address (in cases in which mmap() is allowed to perform a 
randomization). Which is what we want, I'd guess.

Jan, would you care to update the patch with proper Changelog entry?


However, I seem to get "soft" hang on boot with this patch, approximately 
at the time the init should be executed. The system is not completely 
stuck - interrupts are delivered, keyboard is working, alt-sysrq-t dumps 
proper output, but userspace doesn't seem to get started. This happens on 
i386, didn't try on other archs.

-- 
Jiri Kosina
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-11 Thread Ulrich Drepper 

On 5/11/07, Andrew Morton <[EMAIL PROTECTED]> wrote:

erm, I was being funny.  If you randomize a binary it won't run any more.
cp /dev/random /bin/login.  Oh well.

My point is, we're not being told what is being randomized here.  Is it the
virtual starting address of the main executable mmap?  Of the shared
libraries also?  Is it the stack location?  What?


PIE = Position Independent Executable, that's how I named them.

These are not regular executables, they are basically DSOs but usually
compiled with -fpie/-fPIE instead of -fpic/-fPIC and linked with -pie
instead of -shared to allow the compiled and linker perform more
optimizations.

See section 5 in

 http://people.redhat.com/drepper/nonselsec.pdf

Jan unfortunately Ingo's document which doesn't really explain it.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-11 Thread Andrew Morton 
On Fri, 11 May 2007 22:18:16 +0200 (CEST)
Jiri Kosina <[EMAIL PROTECTED]> wrote:

> On Fri, 11 May 2007, Andrew Morton wrote:
> 
> > >I sent this patch 5 days ago, nobody replied. So I am giving it 
> > > second attempt. Andrew, is it possible to test this in -mm branch? 
> > > Original mail follows:
> > > this is something like reaction to this thread: 
> > > http://lkml.org/lkml/2007/1/6/124. I hope I was able to separate the 
> > > PIE randomization part correctly.
> > I don't know what to do with this.  The changelog doesn't tell me what PIE
> > randomization _is_, nor why the kernel would want to do it. "Randomizing 
> > -pie compiled binaries" sounds fairly undesirable, actually ;)
> 
> I think it's precisely what we want to do in case the randomize_va_space 
> is set to 1, don't we? (I haven't yet gone throught the patch though, so I 
> am not sure whether this is the case).

erm, I was being funny.  If you randomize a binary it won't run any more. 
cp /dev/random /bin/login.  Oh well.

My point is, we're not being told what is being randomized here.  Is it the
virtual starting address of the main executable mmap?  Of the shared
libraries also?  Is it the stack location?  What?

I could reverse-engineer that info from the patch, I guess, but I'd prefer
to go in the opposite direction: you tell us what the patch is trying to
do, then we look at it and see if we agree that it is in fact doing that.

> We already have stack randomization and mmap() base randomization but 
> executable base randomization (which is of course only feasible for -pie 
> executables) and brk() randomization still seem to be missing to make it 
> complete.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-11 Thread Jiri Kosina 
On Fri, 11 May 2007, Andrew Morton wrote:

> >I sent this patch 5 days ago, nobody replied. So I am giving it 
> > second attempt. Andrew, is it possible to test this in -mm branch? 
> > Original mail follows:
> > this is something like reaction to this thread: 
> > http://lkml.org/lkml/2007/1/6/124. I hope I was able to separate the 
> > PIE randomization part correctly.
> I don't know what to do with this.  The changelog doesn't tell me what PIE
> randomization _is_, nor why the kernel would want to do it. "Randomizing 
> -pie compiled binaries" sounds fairly undesirable, actually ;)

I think it's precisely what we want to do in case the randomize_va_space 
is set to 1, don't we? (I haven't yet gone throught the patch though, so I 
am not sure whether this is the case).

We already have stack randomization and mmap() base randomization but 
executable base randomization (which is of course only feasible for -pie 
executables) and brk() randomization still seem to be missing to make it 
complete.

-- 
Jiri Kosina
SUSE Labs
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-11 Thread Andrew Morton 
On Fri, 11 May 2007 14:33:46 +0200 (CEST)
Jan Kratochvil <[EMAIL PROTECTED]> wrote:

> Hello,
>I sent this patch 5 days ago, nobody replied. So I am giving it second 
> attempt.
> Andrew, is it possible to test this in -mm branch?
> Original mail follows:
> 
> Hi,
> this is something like reaction to this thread:
> http://lkml.org/lkml/2007/1/6/124. I hope I was able to separate the PIE 
> randomization part correctly.
> 
> There is platform specific (__i386__ only) part in exec shield, I am not 
> pretty sure why is it there, but wasn't brave enough to touch it. Can 
> someone comment the #ifdef out and test it on some other platform?
> 
> It will be nice to follow with brk randomization, but in exec-shield it is 
> afaik i386 only. Right?
> 
> Randomizes -pie compiled binaries. The implementation is part of Redhat's 
> exec-shield (http://people.redhat.com/mingo/exec-shield/).
> 

I don't know what to do with this.  The changelog doesn't tell me what PIE
randomization _is_, nor why the kernel would want to do it.

"Randomizing -pie compiled binaries" sounds fairly undesirable, actually ;)

> 
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 9cc4f0a..1156f41 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -45,7 +45,7 @@
> 
>static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs 
> *regs);
>static int load_elf_library(struct file *);
> -static unsigned long elf_map (struct file *, unsigned long, struct elf_phdr 
> *, int, int);
> +static unsigned long elf_map (struct file *, unsigned long, struct elf_phdr 
> *, int, int, unsigned long);

Your email client is space-stuffing the patches.  That's easy to fix at the
receiving end, but it'd be better to fix it at the sending end, please.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-11 Thread Andrew Morton 
On Fri, 11 May 2007 14:33:46 +0200 (CEST)
Jan Kratochvil [EMAIL PROTECTED] wrote:

 Hello,
I sent this patch 5 days ago, nobody replied. So I am giving it second 
 attempt.
 Andrew, is it possible to test this in -mm branch?
 Original mail follows:
 
 Hi,
 this is something like reaction to this thread:
 http://lkml.org/lkml/2007/1/6/124. I hope I was able to separate the PIE 
 randomization part correctly.
 
 There is platform specific (__i386__ only) part in exec shield, I am not 
 pretty sure why is it there, but wasn't brave enough to touch it. Can 
 someone comment the #ifdef out and test it on some other platform?
 
 It will be nice to follow with brk randomization, but in exec-shield it is 
 afaik i386 only. Right?
 
 Randomizes -pie compiled binaries. The implementation is part of Redhat's 
 exec-shield (http://people.redhat.com/mingo/exec-shield/).
 

I don't know what to do with this.  The changelog doesn't tell me what PIE
randomization _is_, nor why the kernel would want to do it.

Randomizing -pie compiled binaries sounds fairly undesirable, actually ;)

 
 diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
 index 9cc4f0a..1156f41 100644
 --- a/fs/binfmt_elf.c
 +++ b/fs/binfmt_elf.c
 @@ -45,7 +45,7 @@
 
static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs 
 *regs);
static int load_elf_library(struct file *);
 -static unsigned long elf_map (struct file *, unsigned long, struct elf_phdr 
 *, int, int);
 +static unsigned long elf_map (struct file *, unsigned long, struct elf_phdr 
 *, int, int, unsigned long);

Your email client is space-stuffing the patches.  That's easy to fix at the
receiving end, but it'd be better to fix it at the sending end, please.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-11 Thread Jiri Kosina 
On Fri, 11 May 2007, Andrew Morton wrote:

 I sent this patch 5 days ago, nobody replied. So I am giving it 
  second attempt. Andrew, is it possible to test this in -mm branch? 
  Original mail follows:
  this is something like reaction to this thread: 
  http://lkml.org/lkml/2007/1/6/124. I hope I was able to separate the 
  PIE randomization part correctly.
 I don't know what to do with this.  The changelog doesn't tell me what PIE
 randomization _is_, nor why the kernel would want to do it. Randomizing 
 -pie compiled binaries sounds fairly undesirable, actually ;)

I think it's precisely what we want to do in case the randomize_va_space 
is set to 1, don't we? (I haven't yet gone throught the patch though, so I 
am not sure whether this is the case).

We already have stack randomization and mmap() base randomization but 
executable base randomization (which is of course only feasible for -pie 
executables) and brk() randomization still seem to be missing to make it 
complete.

-- 
Jiri Kosina
SUSE Labs
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-11 Thread Andrew Morton 
On Fri, 11 May 2007 22:18:16 +0200 (CEST)
Jiri Kosina [EMAIL PROTECTED] wrote:

 On Fri, 11 May 2007, Andrew Morton wrote:
 
  I sent this patch 5 days ago, nobody replied. So I am giving it 
   second attempt. Andrew, is it possible to test this in -mm branch? 
   Original mail follows:
   this is something like reaction to this thread: 
   http://lkml.org/lkml/2007/1/6/124. I hope I was able to separate the 
   PIE randomization part correctly.
  I don't know what to do with this.  The changelog doesn't tell me what PIE
  randomization _is_, nor why the kernel would want to do it. Randomizing 
  -pie compiled binaries sounds fairly undesirable, actually ;)
 
 I think it's precisely what we want to do in case the randomize_va_space 
 is set to 1, don't we? (I haven't yet gone throught the patch though, so I 
 am not sure whether this is the case).

erm, I was being funny.  If you randomize a binary it won't run any more. 
cp /dev/random /bin/login.  Oh well.

My point is, we're not being told what is being randomized here.  Is it the
virtual starting address of the main executable mmap?  Of the shared
libraries also?  Is it the stack location?  What?

I could reverse-engineer that info from the patch, I guess, but I'd prefer
to go in the opposite direction: you tell us what the patch is trying to
do, then we look at it and see if we agree that it is in fact doing that.

 We already have stack randomization and mmap() base randomization but 
 executable base randomization (which is of course only feasible for -pie 
 executables) and brk() randomization still seem to be missing to make it 
 complete.


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-11 Thread Ulrich Drepper 

On 5/11/07, Andrew Morton [EMAIL PROTECTED] wrote:

erm, I was being funny.  If you randomize a binary it won't run any more.
cp /dev/random /bin/login.  Oh well.

My point is, we're not being told what is being randomized here.  Is it the
virtual starting address of the main executable mmap?  Of the shared
libraries also?  Is it the stack location?  What?


PIE = Position Independent Executable, that's how I named them.

These are not regular executables, they are basically DSOs but usually
compiled with -fpie/-fPIE instead of -fpic/-fPIC and linked with -pie
instead of -shared to allow the compiled and linker perform more
optimizations.

See section 5 in

 http://people.redhat.com/drepper/nonselsec.pdf

Jan unfortunately Ingo's document which doesn't really explain it.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH][RESEND] PIE randomization

2007-05-11 Thread Jiri Kosina 
On Fri, 11 May 2007, Andrew Morton wrote:

 I could reverse-engineer that info from the patch, I guess, but I'd 
 prefer to go in the opposite direction: you tell us what the patch is 
 trying to do, then we look at it and see if we agree that it is in fact 
 doing that.

I've just quickly looked at the patch and it seems fine - it's using 
mmap()'s randomization functionality in such a way that it maps the the 
main executable of (specially compiled/linked) ET_DYN binaries onto a 
random address (in cases in which mmap() is allowed to perform a 
randomization). Which is what we want, I'd guess.

Jan, would you care to update the patch with proper Changelog entry?


However, I seem to get soft hang on boot with this patch, approximately 
at the time the init should be executed. The system is not completely 
stuck - interrupts are delivered, keyboard is working, alt-sysrq-t dumps 
proper output, but userspace doesn't seem to get started. This happens on 
i386, didn't try on other archs.

-- 
Jiri Kosina
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/