diff mbox series

aarch64: Respect p_flags when protecting code with PROT_BTI

Message ID 20200715154444.GA26693@arm.com
State Committed
Headers
Series aarch64: Respect p_flags when protecting code with PROT_BTI |

Commit Message

Szabolcs Nagy July 15, 2020, 3:44 p.m. UTC 
  i'd like to commit the attached patch for 2.32

Comments

Szabolcs Nagy July 17, 2020, 2:52 p.m. UTC | #1 
The 07/15/2020 16:44, Szabolcs Nagy wrote:
> i'd like to commit the attached patch for 2.32

ping.

or can i commit such a patch as a bug fix?


> From af3c11a811cfcc2b72f07efa0696c2200e928e12 Mon Sep 17 00:00:00 2001
> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
> Date: Mon, 13 Jul 2020 11:28:18 +0100
> Subject: [PATCH] aarch64: Respect p_flags when protecting code with PROT_BTI
> 
> Use PROT_READ and PROT_WRITE according to the load segment p_flags
> when adding PROT_BTI.
> 
> This is before processing relocations which may drop PROT_BTI in
> case of textrels.  Executable stacks are not protected via PROT_BTI
> either.  PROT_BTI is hardening in case memory corruption happened,
> it's value is reduced if there is writable and executable memory
> available so missing it on such memory is fine, but we should
> respect the p_flags and should not drop PROT_WRITE.
> ---
>  sysdeps/aarch64/dl-bti.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/sysdeps/aarch64/dl-bti.c b/sysdeps/aarch64/dl-bti.c
> index 965ddcc732..196e462520 100644
> --- a/sysdeps/aarch64/dl-bti.c
> +++ b/sysdeps/aarch64/dl-bti.c
> @@ -24,13 +24,20 @@ static int
>  enable_bti (struct link_map *map, const char *program)
>  {
>    const ElfW(Phdr) *phdr;
> -  unsigned prot = PROT_READ | PROT_EXEC | PROT_BTI;
> +  unsigned prot;
>  
>    for (phdr = map->l_phdr; phdr < &map->l_phdr[map->l_phnum]; ++phdr)
>      if (phdr->p_type == PT_LOAD && (phdr->p_flags & PF_X))
>        {
>  	void *start = (void *) (phdr->p_vaddr + map->l_addr);
>  	size_t len = phdr->p_memsz;
> +
> +	prot = PROT_EXEC | PROT_BTI;
> +	if (phdr->p_flags & PF_R)
> +	  prot |= PROT_READ;
> +	if (phdr->p_flags & PF_W)
> +	  prot |= PROT_WRITE;
> +
>  	if (__mprotect (start, len, prot) < 0)
>  	  {
>  	    if (program)
> -- 
> 2.17.1
> 


--
Szabolcs Nagy July 23, 2020, 12:17 p.m. UTC | #2 
The 07/17/2020 15:52, Szabolcs Nagy wrote:
> The 07/15/2020 16:44, Szabolcs Nagy wrote:
> > i'd like to commit the attached patch for 2.32
> 
> ping.
> 
> or can i commit such a patch as a bug fix?

Carlos, is there any particular reason this is not approved for 2.32?


> > From af3c11a811cfcc2b72f07efa0696c2200e928e12 Mon Sep 17 00:00:00 2001
> > From: Szabolcs Nagy <szabolcs.nagy@arm.com>
> > Date: Mon, 13 Jul 2020 11:28:18 +0100
> > Subject: [PATCH] aarch64: Respect p_flags when protecting code with PROT_BTI
> > 
> > Use PROT_READ and PROT_WRITE according to the load segment p_flags
> > when adding PROT_BTI.
> > 
> > This is before processing relocations which may drop PROT_BTI in
> > case of textrels.  Executable stacks are not protected via PROT_BTI
> > either.  PROT_BTI is hardening in case memory corruption happened,
> > it's value is reduced if there is writable and executable memory
> > available so missing it on such memory is fine, but we should
> > respect the p_flags and should not drop PROT_WRITE.
> > ---
> >  sysdeps/aarch64/dl-bti.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/sysdeps/aarch64/dl-bti.c b/sysdeps/aarch64/dl-bti.c
> > index 965ddcc732..196e462520 100644
> > --- a/sysdeps/aarch64/dl-bti.c
> > +++ b/sysdeps/aarch64/dl-bti.c
> > @@ -24,13 +24,20 @@ static int
> >  enable_bti (struct link_map *map, const char *program)
> >  {
> >    const ElfW(Phdr) *phdr;
> > -  unsigned prot = PROT_READ | PROT_EXEC | PROT_BTI;
> > +  unsigned prot;
> >  
> >    for (phdr = map->l_phdr; phdr < &map->l_phdr[map->l_phnum]; ++phdr)
> >      if (phdr->p_type == PT_LOAD && (phdr->p_flags & PF_X))
> >        {
> >  	void *start = (void *) (phdr->p_vaddr + map->l_addr);
> >  	size_t len = phdr->p_memsz;
> > +
> > +	prot = PROT_EXEC | PROT_BTI;
> > +	if (phdr->p_flags & PF_R)
> > +	  prot |= PROT_READ;
> > +	if (phdr->p_flags & PF_W)
> > +	  prot |= PROT_WRITE;
> > +
> >  	if (__mprotect (start, len, prot) < 0)
> >  	  {
> >  	    if (program)
> > -- 
> > 2.17.1
> > 
> 
> 
> -- 

--
Carlos O'Donell July 23, 2020, 9:25 p.m. UTC | #3 
On 7/23/20 8:17 AM, Szabolcs Nagy wrote:
> The 07/17/2020 15:52, Szabolcs Nagy wrote:
>> The 07/15/2020 16:44, Szabolcs Nagy wrote:
>>> i'd like to commit the attached patch for 2.32
>>
>> ping.
>>
>> or can i commit such a patch as a bug fix?
> 
> Carlos, is there any particular reason this is not approved for 2.32?
 
No, I missed it in my review.

OK for 2.32.
 
>>> From af3c11a811cfcc2b72f07efa0696c2200e928e12 Mon Sep 17 00:00:00 2001
>>> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
>>> Date: Mon, 13 Jul 2020 11:28:18 +0100
>>> Subject: [PATCH] aarch64: Respect p_flags when protecting code with PROT_BTI
>>>
>>> Use PROT_READ and PROT_WRITE according to the load segment p_flags
>>> when adding PROT_BTI.
>>>
>>> This is before processing relocations which may drop PROT_BTI in
>>> case of textrels.  Executable stacks are not protected via PROT_BTI
>>> either.  PROT_BTI is hardening in case memory corruption happened,
>>> it's value is reduced if there is writable and executable memory
>>> available so missing it on such memory is fine, but we should
>>> respect the p_flags and should not drop PROT_WRITE.
>>> ---
>>>  sysdeps/aarch64/dl-bti.c | 9 ++++++++-
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/sysdeps/aarch64/dl-bti.c b/sysdeps/aarch64/dl-bti.c
>>> index 965ddcc732..196e462520 100644
>>> --- a/sysdeps/aarch64/dl-bti.c
>>> +++ b/sysdeps/aarch64/dl-bti.c
>>> @@ -24,13 +24,20 @@ static int
>>>  enable_bti (struct link_map *map, const char *program)
>>>  {
>>>    const ElfW(Phdr) *phdr;
>>> -  unsigned prot = PROT_READ | PROT_EXEC | PROT_BTI;
>>> +  unsigned prot;

OK.

>>>  
>>>    for (phdr = map->l_phdr; phdr < &map->l_phdr[map->l_phnum]; ++phdr)
>>>      if (phdr->p_type == PT_LOAD && (phdr->p_flags & PF_X))
>>>        {
>>>  	void *start = (void *) (phdr->p_vaddr + map->l_addr);
>>>  	size_t len = phdr->p_memsz;
>>> +
>>> +	prot = PROT_EXEC | PROT_BTI;
>>> +	if (phdr->p_flags & PF_R)
>>> +	  prot |= PROT_READ;
>>> +	if (phdr->p_flags & PF_W)
>>> +	  prot |= PROT_WRITE;

OK.

>>> +
>>>  	if (__mprotect (start, len, prot) < 0)
>>>  	  {
>>>  	    if (program)
>>> -- 
>>> 2.17.1
>>>
>>
>>
>> -- 
>
diff mbox series

Patch

From af3c11a811cfcc2b72f07efa0696c2200e928e12 Mon Sep 17 00:00:00 2001
From: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date: Mon, 13 Jul 2020 11:28:18 +0100
Subject: [PATCH] aarch64: Respect p_flags when protecting code with PROT_BTI

Use PROT_READ and PROT_WRITE according to the load segment p_flags
when adding PROT_BTI.

This is before processing relocations which may drop PROT_BTI in
case of textrels.  Executable stacks are not protected via PROT_BTI
either.  PROT_BTI is hardening in case memory corruption happened,
it's value is reduced if there is writable and executable memory
available so missing it on such memory is fine, but we should
respect the p_flags and should not drop PROT_WRITE.
---
 sysdeps/aarch64/dl-bti.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/sysdeps/aarch64/dl-bti.c b/sysdeps/aarch64/dl-bti.c
index 965ddcc732..196e462520 100644
--- a/sysdeps/aarch64/dl-bti.c
+++ b/sysdeps/aarch64/dl-bti.c
@@ -24,13 +24,20 @@  static int
 enable_bti (struct link_map *map, const char *program)
 {
   const ElfW(Phdr) *phdr;
-  unsigned prot = PROT_READ | PROT_EXEC | PROT_BTI;
+  unsigned prot;
 
   for (phdr = map->l_phdr; phdr < &map->l_phdr[map->l_phnum]; ++phdr)
     if (phdr->p_type == PT_LOAD && (phdr->p_flags & PF_X))
       {
 	void *start = (void *) (phdr->p_vaddr + map->l_addr);
 	size_t len = phdr->p_memsz;
+
+	prot = PROT_EXEC | PROT_BTI;
+	if (phdr->p_flags & PF_R)
+	  prot |= PROT_READ;
+	if (phdr->p_flags & PF_W)
+	  prot |= PROT_WRITE;
+
 	if (__mprotect (start, len, prot) < 0)
 	  {
 	    if (program)
-- 
2.17.1