* [PATCH v1] Arm64: ASLR: fix text randomization
@ 2014-10-07 12:40 Arun Chandran
2014-10-07 13:43 ` Mark Rutland
0 siblings, 1 reply; 6+ messages in thread
From: Arun Chandran @ 2014-10-07 12:40 UTC (permalink / raw)
To: linux-arm-kernel
This is due to incorrect definition of ELF_ET_DYN_BASE. It
introduces randomization for text even if user does a "echo 0 >
/proc/sys/kernel/randomize_va_space"
Signed-off-by: Arun Chandran <achandran@mvista.com>
---
This can be tested using the code below
#include <stdio.h>
int main(int argc, char *argv)
{
printf("main = %p\n", main);
return 0;
}
* compile it possition independently
aarch64-linux-gnu-gcc -fPIE -pie aslr.c -o aslr
* run it on the target
# ./aslr
main = 0x7f87138950
# ./aslr
main = 0x7f94a10950
# ./aslr
main = 0x7f94fee950
# ./aslr text
main = 0x7f8cb72950
# echo 0 > /proc/sys/kernel/randomize_va_space
# ./aslr text
main = 0x5555555950
# ./aslr
main = 0x5555555950
# ./aslr
main = 0x5555555950
# ./aslr
main = 0x5555555950
---
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/elf.h | 4 ++--
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index fd4e81a..a2eefc9 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1,5 +1,6 @@
config ARM64
def_bool y
+ select ARCH_BINFMT_ELF_RANDOMIZE_PIE
select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
select ARCH_HAS_SG_CHAIN
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 01d3aab..1f65be3 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -126,7 +126,7 @@ typedef struct user_fpsimd_state elf_fpregset_t;
* that it will "exec", and that there is sufficient room for the brk.
*/
extern unsigned long randomize_et_dyn(unsigned long base);
-#define ELF_ET_DYN_BASE (randomize_et_dyn(2 * TASK_SIZE_64 / 3))
+#define ELF_ET_DYN_BASE (2 * TASK_SIZE_64 / 3)
/*
* When the program starts, a1 contains a pointer to a function to be
@@ -169,7 +169,7 @@ extern unsigned long arch_randomize_brk(struct mm_struct *mm);
#define COMPAT_ELF_PLATFORM ("v8l")
#endif
-#define COMPAT_ELF_ET_DYN_BASE (randomize_et_dyn(2 * TASK_SIZE_32 / 3))
+#define COMPAT_ELF_ET_DYN_BASE (2 * TASK_SIZE_32 / 3)
/* AArch32 registers. */
#define COMPAT_ELF_NGREG 18
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v1] Arm64: ASLR: fix text randomization
2014-10-07 12:40 [PATCH v1] Arm64: ASLR: fix text randomization Arun Chandran
@ 2014-10-07 13:43 ` Mark Rutland
2014-10-08 6:51 ` Arun Chandran
0 siblings, 1 reply; 6+ messages in thread
From: Mark Rutland @ 2014-10-07 13:43 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Oct 07, 2014 at 01:40:28PM +0100, Arun Chandran wrote:
> This is due to incorrect definition of ELF_ET_DYN_BASE. It
> introduces randomization for text even if user does a "echo 0 >
> /proc/sys/kernel/randomize_va_space"
Interesting.
It looks like this was a copy of what powerpc and s390 do (authors
Cc'd), and the generic support came later. powerpc gained support in
501cb16d3cfdcca9 (powerpc: Randomise PIEs), but the generic support was
enabled later in e39f560239984c30 (fs: binfmt_elf: create Kconfig
variable for PIE randomization).
The policy of disabling PIE randomization was added in a3defbe5c337dbc6
(binfmt_elf: fix PIE execution with randomization disabled), after the
powerpc implementation, but before the x86 implementation was made
generic.
I wasn't able to spot where the randomness came from in the
ARCH_BINFMT_ELF_RANDOMIZE_PIE case, so it's not clear to me if the
generic implementation behaves identically other than disabling
randomization when told to via proc.
Assuming it behaves similarly enough, it looks like arm64, powerpc, and
s390 should all be moved over.
>
> Signed-off-by: Arun Chandran <achandran@mvista.com>
> ---
> This can be tested using the code below
>
> #include <stdio.h>
>
> int main(int argc, char *argv)
> {
> printf("main = %p\n", main);
> return 0;
> }
>
> * compile it possition independently
> aarch64-linux-gnu-gcc -fPIE -pie aslr.c -o aslr
>
> * run it on the target
>
> # ./aslr
> main = 0x7f87138950
> # ./aslr
> main = 0x7f94a10950
> # ./aslr
> main = 0x7f94fee950
> # ./aslr text
> main = 0x7f8cb72950
>
> # echo 0 > /proc/sys/kernel/randomize_va_space
> # ./aslr text
> main = 0x5555555950
> # ./aslr
> main = 0x5555555950
> # ./aslr
> main = 0x5555555950
> # ./aslr
> main = 0x5555555950
It would be worth pointing out that this is after your patch is applied.
Before your patch I get randomized VAs even after writing 0 to
randomize_va_spave.
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/elf.h | 4 ++--
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index fd4e81a..a2eefc9 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1,5 +1,6 @@
> config ARM64
> def_bool y
> + select ARCH_BINFMT_ELF_RANDOMIZE_PIE
> select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
> select ARCH_HAS_SG_CHAIN
> select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
> index 01d3aab..1f65be3 100644
> --- a/arch/arm64/include/asm/elf.h
> +++ b/arch/arm64/include/asm/elf.h
> @@ -126,7 +126,7 @@ typedef struct user_fpsimd_state elf_fpregset_t;
> * that it will "exec", and that there is sufficient room for the brk.
> */
> extern unsigned long randomize_et_dyn(unsigned long base);
> -#define ELF_ET_DYN_BASE (randomize_et_dyn(2 * TASK_SIZE_64 / 3))
> +#define ELF_ET_DYN_BASE (2 * TASK_SIZE_64 / 3)
>
> /*
> * When the program starts, a1 contains a pointer to a function to be
> @@ -169,7 +169,7 @@ extern unsigned long arch_randomize_brk(struct mm_struct *mm);
> #define COMPAT_ELF_PLATFORM ("v8l")
> #endif
>
> -#define COMPAT_ELF_ET_DYN_BASE (randomize_et_dyn(2 * TASK_SIZE_32 / 3))
> +#define COMPAT_ELF_ET_DYN_BASE (2 * TASK_SIZE_32 / 3)
>
> /* AArch32 registers. */
> #define COMPAT_ELF_NGREG 18
> --
> 1.7.9.5
Given randomize_et_dyn is no longer used after this patch, I think it
should be dropped from arch/arm64/kernel/process.c.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v1] Arm64: ASLR: fix text randomization
2014-10-07 13:43 ` Mark Rutland
@ 2014-10-08 6:51 ` Arun Chandran
2014-10-08 11:21 ` Will Deacon
0 siblings, 1 reply; 6+ messages in thread
From: Arun Chandran @ 2014-10-08 6:51 UTC (permalink / raw)
To: linux-arm-kernel
Hi Mark,
On Tue, Oct 7, 2014 at 7:13 PM, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Tue, Oct 07, 2014 at 01:40:28PM +0100, Arun Chandran wrote:
> > This is due to incorrect definition of ELF_ET_DYN_BASE. It
> > introduces randomization for text even if user does a "echo 0 >
> > /proc/sys/kernel/randomize_va_space"
>
> Interesting.
>
> It looks like this was a copy of what powerpc and s390 do (authors
> Cc'd), and the generic support came later. powerpc gained support in
> 501cb16d3cfdcca9 (powerpc: Randomise PIEs), but the generic support was
> enabled later in e39f560239984c30 (fs: binfmt_elf: create Kconfig
> variable for PIE randomization).
>
I did not understand why they need a special architecture randomize_et_dyn()
function to handle the situation.
I have tested PIE on arm and x86 (which don't have a randomize_et_dyn()) and
it works as expected.
>
> The policy of disabling PIE randomization was added in a3defbe5c337dbc6
> (binfmt_elf: fix PIE execution with randomization disabled), after the
> powerpc implementation, but before the x86 implementation was made
> generic.
Thought about extending the policy(a3defbe5c337dbc6) to arm64 by doing
#############
diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 01d3aab..401b1e8 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -127,6 +127,7 @@ typedef struct user_fpsimd_state elf_fpregset_t;
*/
extern unsigned long randomize_et_dyn(unsigned long base);
#define ELF_ET_DYN_BASE (randomize_et_dyn(2 * TASK_SIZE_64 / 3))
+#define ARM64_ELF_ET_CONST_BASE (2 * TASK_SIZE_64 / 3)
/*
* When the program starts, a1 contains a pointer to a function to be
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 29d4869..5115f80 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -406,5 +406,8 @@ unsigned long arch_randomize_brk(struct mm_struct *mm)
unsigned long randomize_et_dyn(unsigned long base)
{
- return randomize_base(base);
+ if (current->flags & PF_RANDOMIZE)
+ return randomize_base(base);
+ else
+ return ARM64_ELF_ET_CONST_BASE;
}
##############
then discarded it after seeing the same thing works on x86 and arm.
In arm64(and in ppc and s390) why we need a special randomize_et_dyn()?
>
>
> I wasn't able to spot where the randomness came from in the
> ARCH_BINFMT_ELF_RANDOMIZE_PIE case, so it's not clear to me if the
> generic implementation behaves identically other than disabling
> randomization when told to via proc.
I also don't know from where it is coming; but it works on arm and x86 :)
>
>
> Assuming it behaves similarly enough, it looks like arm64, powerpc, and
> s390 should all be moved over.
>
> >
> > Signed-off-by: Arun Chandran <achandran@mvista.com>
> > ---
> > This can be tested using the code below
> >
> > #include <stdio.h>
> >
> > int main(int argc, char *argv)
> > {
> > printf("main = %p\n", main);
> > return 0;
> > }
> >
> > * compile it possition independently
> > aarch64-linux-gnu-gcc -fPIE -pie aslr.c -o aslr
> >
> > * run it on the target
> >
> > # ./aslr
> > main = 0x7f87138950
> > # ./aslr
> > main = 0x7f94a10950
> > # ./aslr
> > main = 0x7f94fee950
> > # ./aslr text
> > main = 0x7f8cb72950
> >
> > # echo 0 > /proc/sys/kernel/randomize_va_space
> > # ./aslr text
> > main = 0x5555555950
> > # ./aslr
> > main = 0x5555555950
> > # ./aslr
> > main = 0x5555555950
> > # ./aslr
> > main = 0x5555555950
>
> It would be worth pointing out that this is after your patch is applied.
> Before your patch I get randomized VAs even after writing 0 to
> randomize_va_spave.
Ok.
--Arun
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v1] Arm64: ASLR: fix text randomization
2014-10-08 6:51 ` Arun Chandran
@ 2014-10-08 11:21 ` Will Deacon
2014-10-09 14:44 ` Mark Rutland
0 siblings, 1 reply; 6+ messages in thread
From: Will Deacon @ 2014-10-08 11:21 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Oct 08, 2014 at 07:51:54AM +0100, Arun Chandran wrote:
> On Tue, Oct 7, 2014 at 7:13 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > I wasn't able to spot where the randomness came from in the
> > ARCH_BINFMT_ELF_RANDOMIZE_PIE case, so it's not clear to me if the
> > generic implementation behaves identically other than disabling
> > randomization when told to via proc.
>
> I also don't know from where it is coming; but it works on arm and x86 :)
The randomness will come from elf_map, since that will descend into the mmap
code and end up with a randomised base address (see mmap_base and
arch_pick_mmap_layout).
Will
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v1] Arm64: ASLR: fix text randomization
2014-10-08 11:21 ` Will Deacon
@ 2014-10-09 14:44 ` Mark Rutland
2014-10-10 11:31 ` [PATCH v2] Arm64: ASLR: Don't randomise text when randomise_va_space == 0 Arun Chandran
0 siblings, 1 reply; 6+ messages in thread
From: Mark Rutland @ 2014-10-09 14:44 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Oct 08, 2014 at 12:21:55PM +0100, Will Deacon wrote:
> On Wed, Oct 08, 2014 at 07:51:54AM +0100, Arun Chandran wrote:
> > On Tue, Oct 7, 2014 at 7:13 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> > > I wasn't able to spot where the randomness came from in the
> > > ARCH_BINFMT_ELF_RANDOMIZE_PIE case, so it's not clear to me if the
> > > generic implementation behaves identically other than disabling
> > > randomization when told to via proc.
> >
> > I also don't know from where it is coming; but it works on arm and x86 :)
>
> The randomness will come from elf_map, since that will descend into the mmap
> code and end up with a randomised base address (see mmap_base and
> arch_pick_mmap_layout).
Given that, it sounds like the existing randomize_et_dyn is redundant?
If so, the patch looks sane to me.
Mark.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] Arm64: ASLR: Don't randomise text when randomise_va_space == 0
2014-10-09 14:44 ` Mark Rutland
@ 2014-10-10 11:31 ` Arun Chandran
0 siblings, 0 replies; 6+ messages in thread
From: Arun Chandran @ 2014-10-10 11:31 UTC (permalink / raw)
To: linux-arm-kernel
When user asks to turn off ASLR by writing "0" to
/proc/sys/kernel/randomize_va_space there should not be
any randomization to mmap base, stack, VDSO, libs, text and heap
Currently arm64 violates this behavior by randomising text.
Fix this.
Signed-off-by: Arun Chandran <achandran@mvista.com>
---
Changes since v1:
removed randomize_et_dyn()
---
This can be tested using the code below
#include <stdio.h>
int main(int argc, char *argv)
{
printf("main = %p\n", main);
return 0;
}
* compile it possition independently
aarch64-linux-gnu-gcc -fPIE -pie aslr.c -o aslr
* run it on the target
* Behavior before the patch
# for i in 1 2 3 4 5 ; do ./aslr ; done
main = 0x557020a950
main = 0x5561e55950
main = 0x5563e3a950
main = 0x555af30950
main = 0x5592859950
#
# echo 0 > /proc/sys/kernel/randomize_va_space
# for i in 1 2 3 4 5 ; do ./aslr ; done
main = 0x555927f950
main = 0x556829b950
main = 0x5566625950
main = 0x556b533950
main = 0x555c84d950
* Behavior after the patch
# for i in 1 2 3 4 5 ; do ./aslr ; done
main = 0x7f91988950
main = 0x7fb17b6950
main = 0x7f7805a950
main = 0x7fa372e950
main = 0x7f9cc1e950
#
# echo 0 > /proc/sys/kernel/randomize_va_space
# for i in 1 2 3 4 5 ; do ./aslr ; done
main = 0x5555555950
main = 0x5555555950
main = 0x5555555950
main = 0x5555555950
main = 0x5555555950
---
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/elf.h | 4 ++--
arch/arm64/kernel/process.c | 5 -----
3 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index fd4e81a..a2eefc9 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1,5 +1,6 @@
config ARM64
def_bool y
+ select ARCH_BINFMT_ELF_RANDOMIZE_PIE
select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
select ARCH_HAS_SG_CHAIN
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 01d3aab..1f65be3 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -126,7 +126,7 @@ typedef struct user_fpsimd_state elf_fpregset_t;
* that it will "exec", and that there is sufficient room for the brk.
*/
extern unsigned long randomize_et_dyn(unsigned long base);
-#define ELF_ET_DYN_BASE (randomize_et_dyn(2 * TASK_SIZE_64 / 3))
+#define ELF_ET_DYN_BASE (2 * TASK_SIZE_64 / 3)
/*
* When the program starts, a1 contains a pointer to a function to be
@@ -169,7 +169,7 @@ extern unsigned long arch_randomize_brk(struct mm_struct *mm);
#define COMPAT_ELF_PLATFORM ("v8l")
#endif
-#define COMPAT_ELF_ET_DYN_BASE (randomize_et_dyn(2 * TASK_SIZE_32 / 3))
+#define COMPAT_ELF_ET_DYN_BASE (2 * TASK_SIZE_32 / 3)
/* AArch32 registers. */
#define COMPAT_ELF_NGREG 18
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 29d4869..d2edb12 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -403,8 +403,3 @@ unsigned long arch_randomize_brk(struct mm_struct *mm)
{
return randomize_base(mm->brk);
}
-
-unsigned long randomize_et_dyn(unsigned long base)
-{
- return randomize_base(base);
-}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-10-10 11:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-07 12:40 [PATCH v1] Arm64: ASLR: fix text randomization Arun Chandran
2014-10-07 13:43 ` Mark Rutland
2014-10-08 6:51 ` Arun Chandran
2014-10-08 11:21 ` Will Deacon
2014-10-09 14:44 ` Mark Rutland
2014-10-10 11:31 ` [PATCH v2] Arm64: ASLR: Don't randomise text when randomise_va_space == 0 Arun Chandran
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).