[PATCH] aarch64: update NEWS about branch protection

[PATCH] aarch64: update NEWS about branch protection

[Szabolcs Nagy]

After some discussions it seems the original news was not clear
and that it is valid to manually pass the branch protection flags
iff GCC target libs are built with them too. The main difference
between manually passing the flags and using the configure
option is that the latter also makes branch protection the
default in GCC which may not be desirable in some cases.
---
 NEWS | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/NEWS b/NEWS
index 1ef4a0a7a4..0e6ad5edc4 100644
--- a/NEWS
+++ b/NEWS
@@ -70,7 +70,9 @@ Major new features:
 
 * AArch64 now supports standard branch protection security hardening
   in glibc when it is built with a GCC that is configured with
-  --enable-standard-branch-protection.  This includes branch target
+  --enable-standard-branch-protection (or if -mbranch-protection=standard
+  flag is passed when building both GCC target libraries and glibc,
+  in either case a custom GCC is needed).  This includes branch target
   identification (BTI) and pointer authentication for return addresses
   (PAC-RET).  They require armv8.5-a and armv8.3-a architecture
   extensions respectively for the protection to be effective,
-- 
2.17.1

Re: [PATCH] aarch64: update NEWS about branch protection

[Florian Weimer via Libc-alpha]

* Szabolcs Nagy:

> diff --git a/NEWS b/NEWS
> index 1ef4a0a7a4..0e6ad5edc4 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -70,7 +70,9 @@ Major new features:
>  
>  * AArch64 now supports standard branch protection security hardening
>    in glibc when it is built with a GCC that is configured with
> -  --enable-standard-branch-protection.  This includes branch target
> +  --enable-standard-branch-protection (or if -mbranch-protection=standard
> +  flag is passed when building both GCC target libraries and glibc,
> +  in either case a custom GCC is needed).  This includes branch target
>    identification (BTI) and pointer authentication for return addresses
>    (PAC-RET).  They require armv8.5-a and armv8.3-a architecture
>    extensions respectively for the protection to be effective,

Please clarify if you need to pass the flags in CFLAGS or CC for glibc.
Thanks.

Florian

Re: [PATCH] aarch64: update NEWS about branch protection

[Szabolcs Nagy]

The 07/29/2020 10:11, Florian Weimer wrote:
> * Szabolcs Nagy:
> 
> > diff --git a/NEWS b/NEWS
> > index 1ef4a0a7a4..0e6ad5edc4 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -70,7 +70,9 @@ Major new features:
> >  
> >  * AArch64 now supports standard branch protection security hardening
> >    in glibc when it is built with a GCC that is configured with
> > -  --enable-standard-branch-protection.  This includes branch target
> > +  --enable-standard-branch-protection (or if -mbranch-protection=standard
> > +  flag is passed when building both GCC target libraries and glibc,
> > +  in either case a custom GCC is needed).  This includes branch target
> >    identification (BTI) and pointer authentication for return addresses
> >    (PAC-RET).  They require armv8.5-a and armv8.3-a architecture
> >    extensions respectively for the protection to be effective,
> 
> Please clarify if you need to pass the flags in CFLAGS or CC for glibc.
> Thanks.

cflags is enough, but it is hard to tell what
the glibc build system does with the various
cflags.

if i simply override CFLAGS i get
# error "glibc cannot be compiled without optimization"

Re: [PATCH] aarch64: update NEWS about branch protection

[Florian Weimer via Libc-alpha]

* Szabolcs Nagy:

> The 07/29/2020 10:11, Florian Weimer wrote:
>> * Szabolcs Nagy:
>> 
>> > diff --git a/NEWS b/NEWS
>> > index 1ef4a0a7a4..0e6ad5edc4 100644
>> > --- a/NEWS
>> > +++ b/NEWS
>> > @@ -70,7 +70,9 @@ Major new features:
>> >  
>> >  * AArch64 now supports standard branch protection security hardening
>> >    in glibc when it is built with a GCC that is configured with
>> > -  --enable-standard-branch-protection.  This includes branch target
>> > +  --enable-standard-branch-protection (or if -mbranch-protection=standard
>> > +  flag is passed when building both GCC target libraries and glibc,
>> > +  in either case a custom GCC is needed).  This includes branch target
>> >    identification (BTI) and pointer authentication for return addresses
>> >    (PAC-RET).  They require armv8.5-a and armv8.3-a architecture
>> >    extensions respectively for the protection to be effective,
>> 
>> Please clarify if you need to pass the flags in CFLAGS or CC for glibc.
>> Thanks.
>
> cflags is enough, but it is hard to tell what
> the glibc build system does with the various
> cflags.
>
> if i simply override CFLAGS i get
> # error "glibc cannot be compiled without optimization"

Okay, I trust you that CFLAGS is enough.

Are there any ELF notes I should watch out for?

My RM delegation has already expired, so I cannot approve your patch.

Thanks,
Florian

Re: [PATCH] aarch64: update NEWS about branch protection

[Szabolcs Nagy]

The 07/29/2020 11:01, Florian Weimer wrote:
> * Szabolcs Nagy:
> 
> > The 07/29/2020 10:11, Florian Weimer wrote:
> >> * Szabolcs Nagy:
> >> 
> >> > diff --git a/NEWS b/NEWS
> >> > index 1ef4a0a7a4..0e6ad5edc4 100644
> >> > --- a/NEWS
> >> > +++ b/NEWS
> >> > @@ -70,7 +70,9 @@ Major new features:
> >> >  
> >> >  * AArch64 now supports standard branch protection security hardening
> >> >    in glibc when it is built with a GCC that is configured with
> >> > -  --enable-standard-branch-protection.  This includes branch target
> >> > +  --enable-standard-branch-protection (or if -mbranch-protection=standard
> >> > +  flag is passed when building both GCC target libraries and glibc,
> >> > +  in either case a custom GCC is needed).  This includes branch target
> >> >    identification (BTI) and pointer authentication for return addresses
> >> >    (PAC-RET).  They require armv8.5-a and armv8.3-a architecture
> >> >    extensions respectively for the protection to be effective,
> >> 
> >> Please clarify if you need to pass the flags in CFLAGS or CC for glibc.
> >> Thanks.
> >
> > cflags is enough, but it is hard to tell what
> > the glibc build system does with the various
> > cflags.
> >
> > if i simply override CFLAGS i get
> > # error "glibc cannot be compiled without optimization"
> 
> Okay, I trust you that CFLAGS is enough.
> 
> Are there any ELF notes I should watch out for?

readelf should show

  GNU                  0x00000010       NT_GNU_PROPERTY_TYPE_0        Properties: AArch64 feature: BTI

(PAC may be missing in some libgcc asm, that's
fixed up in gcc-trunk, but it's harmless.)

e.g. this should not print any file in the install dir:

find . -type f |while read i
do
  # skip non-elf files
  aarch64-none-linux-gnu-readelf -h $i >/dev/null 2>&1 || continue
  # print if missing BTI note
  aarch64-none-linux-gnu-readelf -nW $i |grep -q BTI || echo $i
done

> 
> My RM delegation has already expired, so I cannot approve your patch.

ok.

Re: [PATCH] aarch64: update NEWS about branch protection

[Florian Weimer via Libc-alpha]

* Szabolcs Nagy:

>> Are there any ELF notes I should watch out for?
>
> readelf should show
>
>   GNU                  0x00000010       NT_GNU_PROPERTY_TYPE_0        Properties: AArch64 feature: BTI
>
> (PAC may be missing in some libgcc asm, that's
> fixed up in gcc-trunk, but it's harmless.)

Thanks.  It looks like with the flags in CFLAGS, the notes are indeed
there in some cases, because RPM debugedit chokes on them:

explicitly decompress any DWARF compressed ELF sections in /builddir/build/BUILDROOT/glibc-2.31.9000-23.fc33.aarch64/usr/bin/gencat
extracting debug info from /builddir/build/BUILDROOT/glibc-2.31.9000-23.fc33.aarch64/usr/bin/gencat
Failed to update file: invalid section entry size

I'll try to figure out what is going on there.  It's a bit suspicious
that this is the first dynamically linked binary, so maybe the notes are
missing from the shared objects and statically linked binaries still.

Thanks,
Florian

Re: [PATCH] aarch64: update NEWS about branch protection

[Szabolcs Nagy]

The 07/29/2020 12:04, Florian Weimer wrote:
> * Szabolcs Nagy:
> 
> >> Are there any ELF notes I should watch out for?
> >
> > readelf should show
> >
> >   GNU                  0x00000010       NT_GNU_PROPERTY_TYPE_0        Properties: AArch64 feature: BTI
> >
> > (PAC may be missing in some libgcc asm, that's
> > fixed up in gcc-trunk, but it's harmless.)
> 
> Thanks.  It looks like with the flags in CFLAGS, the notes are indeed
> there in some cases, because RPM debugedit chokes on them:
> 
> explicitly decompress any DWARF compressed ELF sections in /builddir/build/BUILDROOT/glibc-2.31.9000-23.fc33.aarch64/usr/bin/gencat
> extracting debug info from /builddir/build/BUILDROOT/glibc-2.31.9000-23.fc33.aarch64/usr/bin/gencat
> Failed to update file: invalid section entry size

if it tries to parse the dwarf debug info then
it may choke on the new dwarf op code that is
used for PAC: DW_CFA_GNU_window_save

> 
> I'll try to figure out what is going on there.  It's a bit suspicious
> that this is the first dynamically linked binary, so maybe the notes are
> missing from the shared objects and statically linked binaries still.
> 
> Thanks,
> Florian
> 

-- 

Re: [PATCH] aarch64: update NEWS about branch protection

[Florian Weimer via Libc-alpha]

* Szabolcs Nagy:

>> Okay, I trust you that CFLAGS is enough.
>> 
>> Are there any ELF notes I should watch out for?
>
> readelf should show
>
>   GNU                  0x00000010       NT_GNU_PROPERTY_TYPE_0        Properties: AArch64 feature: BTI
>
> (PAC may be missing in some libgcc asm, that's
> fixed up in gcc-trunk, but it's harmless.)

It looks like we're hitting a binutils bug:

  <https://sourceware.org/bugzilla/show_bug.cgi?id=26312>

Thanks,
Florian

Re: [PATCH] aarch64: update NEWS about branch protection

[Szabolcs Nagy]

The 07/29/2020 10:17, Szabolcs Nagy wrote:
> The 07/29/2020 11:01, Florian Weimer wrote:
> > * Szabolcs Nagy:
> > > The 07/29/2020 10:11, Florian Weimer wrote:
> > >> * Szabolcs Nagy:
> > >> > diff --git a/NEWS b/NEWS
> > >> > index 1ef4a0a7a4..0e6ad5edc4 100644
> > >> > --- a/NEWS
> > >> > +++ b/NEWS
> > >> > @@ -70,7 +70,9 @@ Major new features:
> > >> >  
> > >> >  * AArch64 now supports standard branch protection security hardening
> > >> >    in glibc when it is built with a GCC that is configured with
> > >> > -  --enable-standard-branch-protection.  This includes branch target
> > >> > +  --enable-standard-branch-protection (or if -mbranch-protection=standard
> > >> > +  flag is passed when building both GCC target libraries and glibc,
> > >> > +  in either case a custom GCC is needed).  This includes branch target
> > >> >    identification (BTI) and pointer authentication for return addresses
> > >> >    (PAC-RET).  They require armv8.5-a and armv8.3-a architecture
> > >> >    extensions respectively for the protection to be effective,
> > >> 
> > >> Please clarify if you need to pass the flags in CFLAGS or CC for glibc.
> > >> Thanks.
> > >
> > > cflags is enough, but it is hard to tell what
> > > the glibc build system does with the various
> > > cflags.
> > >
> > > if i simply override CFLAGS i get
> > > # error "glibc cannot be compiled without optimization"
> > 
> > Okay, I trust you that CFLAGS is enough.
...
> > My RM delegation has already expired, so I cannot approve your patch.
> 
> ok.

ping.

Re: [PATCH] aarch64: update NEWS about branch protection

[Szabolcs Nagy]

The 07/31/2020 07:58, Szabolcs Nagy wrote:
> The 07/29/2020 10:17, Szabolcs Nagy wrote:
> > The 07/29/2020 11:01, Florian Weimer wrote:
> > > * Szabolcs Nagy:
> > > > The 07/29/2020 10:11, Florian Weimer wrote:
> > > >> * Szabolcs Nagy:
> > > >> > diff --git a/NEWS b/NEWS
> > > >> > index 1ef4a0a7a4..0e6ad5edc4 100644
> > > >> > --- a/NEWS
> > > >> > +++ b/NEWS
> > > >> > @@ -70,7 +70,9 @@ Major new features:
> > > >> >  
> > > >> >  * AArch64 now supports standard branch protection security hardening
> > > >> >    in glibc when it is built with a GCC that is configured with
> > > >> > -  --enable-standard-branch-protection.  This includes branch target
> > > >> > +  --enable-standard-branch-protection (or if -mbranch-protection=standard
> > > >> > +  flag is passed when building both GCC target libraries and glibc,
> > > >> > +  in either case a custom GCC is needed).  This includes branch target
> > > >> >    identification (BTI) and pointer authentication for return addresses
> > > >> >    (PAC-RET).  They require armv8.5-a and armv8.3-a architecture
> > > >> >    extensions respectively for the protection to be effective,

Carlos, i think this NEWS update would be a useful
clarification, is there some concern about it?


> > > >> 
> > > >> Please clarify if you need to pass the flags in CFLAGS or CC for glibc.
> > > >> Thanks.
> > > >
> > > > cflags is enough, but it is hard to tell what
> > > > the glibc build system does with the various
> > > > cflags.
> > > >
> > > > if i simply override CFLAGS i get
> > > > # error "glibc cannot be compiled without optimization"
> > > 
> > > Okay, I trust you that CFLAGS is enough.
> ...
> > > My RM delegation has already expired, so I cannot approve your patch.
> > 
> > ok.
> 
> ping.
> 

-- 

Re: [PATCH] aarch64: update NEWS about branch protection

[Carlos O'Donell via Libc-alpha]

On 7/29/20 4:08 AM, Szabolcs Nagy wrote:
> After some discussions it seems the original news was not clear
> and that it is valid to manually pass the branch protection flags
> iff GCC target libs are built with them too. The main difference
> between manually passing the flags and using the configure
> option is that the latter also makes branch protection the
> default in GCC which may not be desirable in some cases.
> ---
>  NEWS | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/NEWS b/NEWS
> index 1ef4a0a7a4..0e6ad5edc4 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -70,7 +70,9 @@ Major new features:
>  
>  * AArch64 now supports standard branch protection security hardening
>    in glibc when it is built with a GCC that is configured with
> -  --enable-standard-branch-protection.  This includes branch target
> +  --enable-standard-branch-protection (or if -mbranch-protection=standard
> +  flag is passed when building both GCC target libraries and glibc,
> +  in either case a custom GCC is needed).  This includes branch target
>    identification (BTI) and pointer authentication for return addresses
>    (PAC-RET).  They require armv8.5-a and armv8.3-a architecture
>    extensions respectively for the protection to be effective,
> 

OK for glibc 2.32. My apologies for the delay.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

-- 
Cheers,
Carlos.