Re: Enabling PAC/BTI support on arm64

2023-12-05 Thread Emanuele Rocca
Hey Aurelien,

On 2023-12-05 08:32, Aurelien Jarno wrote:
> What about the patch below, it basically just change the compiler to
> always enable -mbranch-protection=standard, and it's also used for the
> configure script.

> --- glibc-2.37/debian/sysdeps/arm64.mk
> +++ glibc-2.37/debian/sysdeps/arm64.mk
> @@ -1,2 +1,5 @@
>  # configuration options for all flavours
>  extra_config_options = --enable-multi-arch --enable-memory-tagging
> +
> +CC = $(DEB_HOST_GNU_TYPE)-$(BASE_CC)$(DEB_GCC_VERSION) 
> -mbranch-protection=standard
> +CXX = $(DEB_HOST_GNU_TYPE)-$(BASE_CXX)$(DEB_GCC_VERSION) 
> -mbranch-protection=standard

Your patch works. Highlights:

 cd build-tree/arm64-libc && \
 CC="aarch64-linux-gnu-gcc-12 -mbranch-protection=standard" \
 CXX="aarch64-linux-gnu-g++-12 -mbranch-protection=standard" \
 [...]
 checking for BTI support... yes
 checking if pac-ret is enabled... yes
 [...]
 aarch64-linux-gnu-gcc-12 -mbranch-protection=standard -nostdlib -nostartfiles 
-r -o /<>/build-tree/arm64-libc/csu/Scrt1.o 
/<>/build-tree/arm64-libc/csu/start.os 
/<>/build-tree/arm64-libc/csu/abi-note.o 
/<>/build-tree/arm64-libc/csu/init.o

Full logs at https://people.debian.org/~ema/glibc_2.37-13.1_arm64.build

At some later point when both gcc and glibc with BTI enabled are in the
archive we could think of adding an autopkgtest checking for something
like:

 readelf -n /usr/lib/aarch64-linux-gnu/Scrt1.o | grep 'BTI, PAC'

But for now: do you want me to file a bug against glibc to keep track of
this in the BTS?

Thank you very much!
  Emanuele



Re: Enabling PAC/BTI support on arm64

2023-12-05 Thread Aurelien Jarno
Hi Emanuele,

On 2023-12-04 16:13, Emanuele Rocca wrote:
> Hello Aurelien,
> 
> On 2023-12-03 01:08, Aurelien Jarno wrote:
> > On 2023-11-29 09:56, Emanuele Rocca wrote:
> > > To add BTI to the NOTE section of the above, we would need to build both
> > > GCC and glibc with -mbranch-protection=standard. For gcc-13 I have
> > > proposed https://bugs.debian.org/1055711. Given that glibc in Debian is
> > > built with gcc-12, we will also need to build gcc-12 with
> > > -mbranch-protection=standard.
> > 
> > The plan is to switch to gcc-13 once we have glibc 2.38 in testing. I
> > hope to be able to push glibc 2.38 to unstable in the next weeks.
> 
> Nice! In any case, ideally we can get BTI enabled in both gcc-12 and
> gcc-13, so that users of both versions can take advantage of the
> feature.
> 
> > > When it comes to glibc itself, there is a configure check to enable BTI
> > > support [2], and it uses CFLAGS as passed to ./configure. I have done
> > > the following here on my arm64 machine:
> > > 
> > > - built gcc-12 with PAC/BTI (see #1055711)
> > > - built glibc with the PAC/BTI enabled gcc-12 and the attached patch
> 
> > Is it necessary to wait for gcc-12 to be fixed before doing the change
> > on the glibc side?
> 
> Yes, it is. Without gcc support, glibc would fail building due to the
> use of -z force-bti when not all inputs have BTI in the NOTE section.
> See: https://people.debian.org/~ema/glibc-no-gcc-bti_2.37-12.1_arm64.build

Ok, thanks for the confirmation. I give a try to provide a better patch
and indeed ending up with this issue.

> > > diff -Nru glibc-2.37/debian/rules.d/build.mk 
> > > glibc-2.37/debian/rules.d/build.mk
> > > --- glibc-2.37/debian/rules.d/build.mk2023-10-02 22:29:12.0 
> > > +0200
> > > +++ glibc-2.37/debian/rules.d/build.mk2023-11-28 10:35:40.0 
> > > +0100
> > > @@ -97,6 +97,7 @@
> > >   echo -n "Build started: " ; date --rfc-2822; \
> > >   echo "---"; \
> > >   cd $(DEB_BUILDDIR) && \
> > > + $(if $(filter -mbranch-protection=standard,$(shell 
> > > dpkg-buildflags --get CFLAGS)),CFLAGS=-mbranch-protection=standard) \
> > 
> > I don't get why this is necessary with the changes in debian/rules.
> 
> glibc's configure script is using CFLAGS when running the check
> "checking for BTI support", and I couldn't find any other way to get
> that check to say "yes" other than passing CFLAGS to ./configure. Happy
> to do it otherwise, if you can find working alternatives. :-)

Understood, it looks like to me a bug in the configure script. Anyway
please see the patch below as an alternative workaround.

> > Also I am concerned with the use of dpkg-buildflags in the cross build
> > context, given that feature is arm64 specific. Is there any drawback in
> > enabling branch protection unconditionally?
> 
> No, we just need to avoid passing the flag to non-aarch64 compilers when
> cross-building. I understand that this is already taken care of if we
> move the code to d/sysdeps/arm64.mk? For example, this would fail:
> 
>  x86_64-linux-gnu-gcc-13 -mbranch-protection=standard
> 
> The type of error is the same as passing -fcf-protection to an aarch64
> gcc:
> 
>  aarch64-linux-gnu-gcc-13 -fcf-protection

Ok, so i think it's better to enable BTI unconditionally for the arm64
glibc.

> For reference, I've now tried building glibc with a bti-enabled gcc-12
> and the following patch. The resulting build has functioning BTI.

What about the patch below, it basically just change the compiler to
always enable -mbranch-protection=standard, and it's also used for the
configure script.

Regards
Aurelien

--- glibc-2.37/debian/sysdeps/arm64.mk
+++ glibc-2.37/debian/sysdeps/arm64.mk
@@ -1,2 +1,5 @@
 # configuration options for all flavours
 extra_config_options = --enable-multi-arch --enable-memory-tagging
+
+CC = $(DEB_HOST_GNU_TYPE)-$(BASE_CC)$(DEB_GCC_VERSION) 
-mbranch-protection=standard
+CXX = $(DEB_HOST_GNU_TYPE)-$(BASE_CXX)$(DEB_GCC_VERSION) 
-mbranch-protection=standard

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://aurel32.net



Re: Enabling PAC/BTI support on arm64

2023-12-04 Thread Emanuele Rocca
Hello Aurelien,

On 2023-12-03 01:08, Aurelien Jarno wrote:
> On 2023-11-29 09:56, Emanuele Rocca wrote:
> > To add BTI to the NOTE section of the above, we would need to build both
> > GCC and glibc with -mbranch-protection=standard. For gcc-13 I have
> > proposed https://bugs.debian.org/1055711. Given that glibc in Debian is
> > built with gcc-12, we will also need to build gcc-12 with
> > -mbranch-protection=standard.
> 
> The plan is to switch to gcc-13 once we have glibc 2.38 in testing. I
> hope to be able to push glibc 2.38 to unstable in the next weeks.

Nice! In any case, ideally we can get BTI enabled in both gcc-12 and
gcc-13, so that users of both versions can take advantage of the
feature.

> > When it comes to glibc itself, there is a configure check to enable BTI
> > support [2], and it uses CFLAGS as passed to ./configure. I have done
> > the following here on my arm64 machine:
> > 
> > - built gcc-12 with PAC/BTI (see #1055711)
> > - built glibc with the PAC/BTI enabled gcc-12 and the attached patch

> Is it necessary to wait for gcc-12 to be fixed before doing the change
> on the glibc side?

Yes, it is. Without gcc support, glibc would fail building due to the
use of -z force-bti when not all inputs have BTI in the NOTE section.
See: https://people.debian.org/~ema/glibc-no-gcc-bti_2.37-12.1_arm64.build

> > diff -Nru glibc-2.37/debian/rules.d/build.mk 
> > glibc-2.37/debian/rules.d/build.mk
> > --- glibc-2.37/debian/rules.d/build.mk  2023-10-02 22:29:12.0 
> > +0200
> > +++ glibc-2.37/debian/rules.d/build.mk  2023-11-28 10:35:40.0 
> > +0100
> > @@ -97,6 +97,7 @@
> > echo -n "Build started: " ; date --rfc-2822; \
> > echo "---"; \
> > cd $(DEB_BUILDDIR) && \
> > +   $(if $(filter -mbranch-protection=standard,$(shell 
> > dpkg-buildflags --get CFLAGS)),CFLAGS=-mbranch-protection=standard) \
> 
> I don't get why this is necessary with the changes in debian/rules.

glibc's configure script is using CFLAGS when running the check
"checking for BTI support", and I couldn't find any other way to get
that check to say "yes" other than passing CFLAGS to ./configure. Happy
to do it otherwise, if you can find working alternatives. :-)

> Also I am concerned with the use of dpkg-buildflags in the cross build
> context, given that feature is arm64 specific. Is there any drawback in
> enabling branch protection unconditionally?

No, we just need to avoid passing the flag to non-aarch64 compilers when
cross-building. I understand that this is already taken care of if we
move the code to d/sysdeps/arm64.mk? For example, this would fail:

 x86_64-linux-gnu-gcc-13 -mbranch-protection=standard

The type of error is the same as passing -fcf-protection to an aarch64
gcc:

 aarch64-linux-gnu-gcc-13 -fcf-protection

For reference, I've now tried building glibc with a bti-enabled gcc-12
and the following patch. The resulting build has functioning BTI.

Thanks!

diff -Nru glibc-2.37/debian/sysdeps/arm64.mk glibc-2.37/debian/sysdeps/arm64.mk
--- glibc-2.37/debian/sysdeps/arm64.mk  2023-10-02 22:29:12.0 +0200
+++ glibc-2.37/debian/sysdeps/arm64.mk  2023-11-28 10:35:40.0 +0100
@@ -1,2 +1,5 @@
+HOST_CFLAGS += -mbranch-protection=standard
+HOST_CXXFLAGS += -mbranch-protection=standard
+
 # configuration options for all flavours
-extra_config_options = --enable-multi-arch --enable-memory-tagging
+extra_config_options = --enable-multi-arch --enable-memory-tagging 
CFLAGS=-mbranch-protection=standard



Re: Enabling PAC/BTI support on arm64

2023-12-03 Thread Aurelien Jarno
Hi Emanuele,

On 2023-11-29 09:56, Emanuele Rocca wrote:
> Hi!
> 
> I would like to ask for suggestions about the best way to enable PAC/BTI
> support in glibc and GCC on Debian.
> 
> PAC and BTI are two useful Arm security features, see this recent
> presentation at the Mini Debconf for all details: [0]
> 
> In order to properly support PAC/BTI in Debian we need to enable support
> in both GCC and glibc. An executable is marked as BTI compatible only if
> all the execution units of the program are BTI compatible. See pages
> 10-11 on the presentation slides. [1]
> 
> One can easily verify if things work fine by building a test program as
> follows:
> 
>  gcc -mbranch-protection=standard -z force-bti /tmp/test.c
> 
> On systems where both GCC and glibc support PAC/BTI, the command above
> returns no output, and the resulting executable has "BTI, PAC" listed in
> the output of readelf -n:
> 
>  readelf -n a.out | grep Properties
> Properties: AArch64 feature: BTI, PAC
> 
> If PAC/BTI support is not enabled in GCC/glibc, building the program
> with -z force-bti returns something like:
> 
>   /usr/bin/ld: 
> /usr/lib/gcc/aarch64-linux-gnu/13/../../../aarch64-linux-gnu/Scrt1.o: 
> warning: BTI turned on by -z force-bti when all inputs do not have BTI in 
> NOTE section.
>   /usr/bin/ld: 
> /usr/lib/gcc/aarch64-linux-gnu/13/../../../aarch64-linux-gnu/crti.o: warning: 
> BTI turned on by -z force-bti when all inputs do not have BTI in NOTE section.
>   /usr/bin/ld: /usr/lib/gcc/aarch64-linux-gnu/13/crtbeginS.o: warning: BTI 
> turned on by -z force-bti when all inputs do not have BTI in NOTE section.
>   /usr/bin/ld: /usr/lib/gcc/aarch64-linux-gnu/13/crtendS.o: warning: BTI 
> turned on by -z force-bti when all inputs do not have BTI in NOTE section.
>   /usr/bin/ld: 
> /usr/lib/gcc/aarch64-linux-gnu/13/../../../aarch64-linux-gnu/crtn.o: warning: 
> BTI turned on by -z force-bti when all inputs do not have BTI in NOTE section.
> 
> To add BTI to the NOTE section of the above, we would need to build both
> GCC and glibc with -mbranch-protection=standard. For gcc-13 I have
> proposed https://bugs.debian.org/1055711. Given that glibc in Debian is
> built with gcc-12, we will also need to build gcc-12 with
> -mbranch-protection=standard.

The plan is to switch to gcc-13 once we have glibc 2.38 in testing. I
hope to be able to push glibc 2.38 to unstable in the next weeks.

> When it comes to glibc itself, there is a configure check to enable BTI
> support [2], and it uses CFLAGS as passed to ./configure. I have done
> the following here on my arm64 machine:
> 
> - built gcc-12 with PAC/BTI (see #1055711)
> - built glibc with the PAC/BTI enabled gcc-12 and the attached patch

I have some comments about this patch (see below), but once I understand
it, I do no see any issue to enable this feature on the glibc side, and
I can propose a patch.

Is it necessary to wait for gcc-12 to be fixed before doing the change
on the glibc side?

> diff -Nru glibc-2.37/debian/rules glibc-2.37/debian/rules
> --- glibc-2.37/debian/rules   2023-10-02 22:29:12.0 +0200
> +++ glibc-2.37/debian/rules   2023-11-28 10:35:40.0 +0100
> @@ -112,6 +112,11 @@
>  BUILD_CFLAGS = -O2 -g -fdebug-prefix-map=$(CURDIR)=.
>  HOST_CFLAGS = -pipe -O2 -g -fdebug-prefix-map=$(CURDIR)=. $(call 
> xx,extra_cflags)
>  
> +ifeq ($(DEB_BUILD_ARCH),arm64)
> +HOST_CFLAGS += -mbranch-protection=standard
> +HOST_CXXFLAGS += -mbranch-protection=standard
> +endif

Given this change is arm64 specific, it would be better to move it to
debian/sysdeps/arm64.mk instead.

> diff -Nru glibc-2.37/debian/rules.d/build.mk 
> glibc-2.37/debian/rules.d/build.mk
> --- glibc-2.37/debian/rules.d/build.mk2023-10-02 22:29:12.0 
> +0200
> +++ glibc-2.37/debian/rules.d/build.mk2023-11-28 10:35:40.0 
> +0100
> @@ -97,6 +97,7 @@
>   echo -n "Build started: " ; date --rfc-2822; \
>   echo "---"; \
>   cd $(DEB_BUILDDIR) && \
> + $(if $(filter -mbranch-protection=standard,$(shell 
> dpkg-buildflags --get CFLAGS)),CFLAGS=-mbranch-protection=standard) \

I don't get why this is necessary with the changes in debian/rules. Also
here the branch protection is enabled depending on dpkg-buildflags while
in the debian/rules change, this is done unconditionally. Any reason for
that?

Also I am concerned with the use of dpkg-buildflags in the cross build
context, given that feature is arm64 specific. Is there any drawback in
enabling branch protection unconditionally?

Regards
Aurelien

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://aurel32.net



Enabling PAC/BTI support on arm64

2023-11-29 Thread Emanuele Rocca
Hi!

I would like to ask for suggestions about the best way to enable PAC/BTI
support in glibc and GCC on Debian.

PAC and BTI are two useful Arm security features, see this recent
presentation at the Mini Debconf for all details: [0]

In order to properly support PAC/BTI in Debian we need to enable support
in both GCC and glibc. An executable is marked as BTI compatible only if
all the execution units of the program are BTI compatible. See pages
10-11 on the presentation slides. [1]

One can easily verify if things work fine by building a test program as
follows:

 gcc -mbranch-protection=standard -z force-bti /tmp/test.c

On systems where both GCC and glibc support PAC/BTI, the command above
returns no output, and the resulting executable has "BTI, PAC" listed in
the output of readelf -n:

 readelf -n a.out | grep Properties
Properties: AArch64 feature: BTI, PAC

If PAC/BTI support is not enabled in GCC/glibc, building the program
with -z force-bti returns something like:

  /usr/bin/ld: 
/usr/lib/gcc/aarch64-linux-gnu/13/../../../aarch64-linux-gnu/Scrt1.o: warning: 
BTI turned on by -z force-bti when all inputs do not have BTI in NOTE section.
  /usr/bin/ld: 
/usr/lib/gcc/aarch64-linux-gnu/13/../../../aarch64-linux-gnu/crti.o: warning: 
BTI turned on by -z force-bti when all inputs do not have BTI in NOTE section.
  /usr/bin/ld: /usr/lib/gcc/aarch64-linux-gnu/13/crtbeginS.o: warning: BTI 
turned on by -z force-bti when all inputs do not have BTI in NOTE section.
  /usr/bin/ld: /usr/lib/gcc/aarch64-linux-gnu/13/crtendS.o: warning: BTI turned 
on by -z force-bti when all inputs do not have BTI in NOTE section.
  /usr/bin/ld: 
/usr/lib/gcc/aarch64-linux-gnu/13/../../../aarch64-linux-gnu/crtn.o: warning: 
BTI turned on by -z force-bti when all inputs do not have BTI in NOTE section.

To add BTI to the NOTE section of the above, we would need to build both
GCC and glibc with -mbranch-protection=standard. For gcc-13 I have
proposed https://bugs.debian.org/1055711. Given that glibc in Debian is
built with gcc-12, we will also need to build gcc-12 with
-mbranch-protection=standard.

When it comes to glibc itself, there is a configure check to enable BTI
support [2], and it uses CFLAGS as passed to ./configure. I have done
the following here on my arm64 machine:

- built gcc-12 with PAC/BTI (see #1055711)
- built glibc with the PAC/BTI enabled gcc-12 and the attached patch

The resulting GCC and glibc have working PAC/BTI support.

There is maybe a better way in glibc and GCC to achieve the same goal,
do you maintainers have any suggestions?

Thanks!
  Emanuele

[0] https://wiki.debian.org/DebianEvents/gb/2023/MiniDebConfCambridge/Capper
[1] 
https://wiki.debian.org/DebianEvents/gb/2023/MiniDebConfCambridge/Capper?action=AttachFile&do=view&target=miniconf-2023-PAC-and-BTI.pdf
[2] 
https://sources.debian.org/src/glibc/2.37-12/sysdeps/aarch64/configure/?hl=184#L188
diff -Nru glibc-2.37/debian/rules glibc-2.37/debian/rules
--- glibc-2.37/debian/rules 2023-10-02 22:29:12.0 +0200
+++ glibc-2.37/debian/rules 2023-11-28 10:35:40.0 +0100
@@ -112,6 +112,11 @@
 BUILD_CFLAGS = -O2 -g -fdebug-prefix-map=$(CURDIR)=.
 HOST_CFLAGS = -pipe -O2 -g -fdebug-prefix-map=$(CURDIR)=. $(call 
xx,extra_cflags)
 
+ifeq ($(DEB_BUILD_ARCH),arm64)
+HOST_CFLAGS += -mbranch-protection=standard
+HOST_CXXFLAGS += -mbranch-protection=standard
+endif
+
 # 32-bit MIPS builders have a 2GB memory space. This is not enough to
 # build test-tgmath3.o with GCC, unless tweaking the garbage collector.
 ifeq ($(findstring mips,$(DEB_BUILD_ARCH))-$(DEB_BUILD_ARCH_BITS), mips-32)
diff -Nru glibc-2.37/debian/rules.d/build.mk glibc-2.37/debian/rules.d/build.mk
--- glibc-2.37/debian/rules.d/build.mk  2023-10-02 22:29:12.0 +0200
+++ glibc-2.37/debian/rules.d/build.mk  2023-11-28 10:35:40.0 +0100
@@ -97,6 +97,7 @@
echo -n "Build started: " ; date --rfc-2822; \
echo "---"; \
cd $(DEB_BUILDDIR) && \
+   $(if $(filter -mbranch-protection=standard,$(shell 
dpkg-buildflags --get CFLAGS)),CFLAGS=-mbranch-protection=standard) \
CC="$(call xx,CC)" \
CXX=$(if $(filter nocheck,$(DEB_BUILD_OPTIONS)),:,"$(call 
xx,CXX)") \
MIG="$(call xx,MIG)" \