Re: Really enable -fstack-clash-protection on armhf/armel?

Wookey <[email protected]> Sat, 25 Nov 2023 00:37:44 +0000
Newsgroups	gmane.linux.debian.devel.dpkg.general,gmane.linux.debian.ports.arm
Organization	Wookware
Message-ID	<[email protected]>
On 2023-11-24 01:34 +0100, Guillem Jover wrote:
> On Thu, 2023-11-23 at 10:45:33 +0100, Matthias Klose wrote:
> > it looks like enabling this flag on armel/armhf is a little bit premature.

> > In Ubuntu, people tracked down segfaults due to this change in at least
> > valgrind and gnutls, maybe more.
>
> If there's some missing support somewhere that might make this a
> common thing instead of just affecting a handful of packages that
> could simply disable the flags, and the Arm porters consider that
> fixing that is not feasible in the short term, I guess it makes
> sense to stop emitting the flag for the arm32 arches.

Assuming this problem only affects some packages they can have their
build flags adjusted in the short term. dpkg-buildflags makes this straightforward.

And we can investigate and fix in the longer term.

So I don't think we need to turn it off for the whole architecture
unless we find loads of stuff that is broken.

Are there any bugs reports on how to reproduce issues?

I just tried building gnutls28 both with and without
fstack-clash-protection.
It is one test better with -fstack-clash-protection enabled: dtls/dtls-resume.sh

       -fstack-clash-protection
       enabled  disabled
TOTAL: 501      501
PASS:  461      460
SKIP:  20       20
XFAIL: 0        0
FAIL:  20       21
XPASS: 0        0
ERROR: 0        0

So that's worthy of investigation, but suggests there is a problem
here which scp isn't making worse.

Some additional info from Richard Earnshaw:
---
Note that for valgrind, I suspect the problem is that it has not been
updated for the following, relatively recent, relaxation in the AAPCS:

6.2.1.3 Stack probing
In order to ensure stack integrity a process may emit stack probes
immediately prior to allocating additional stack space (moving SP from
SP_old to SP_new). Stack probes must be in the region of [SP_new, SP_old
- 1] and may be either read or write operations. The minimum interval
for stack probing is defined by the target platform but must be a
minimum of 4KBytes. No recoverable data can be saved below the currently
allocated stack region.

Prior to this addition (2018Q4) all accesses below SP were forbidden,
and I think that's what valgrind still implements.
---

So that does sound like valgrind needs an update for this, and yes it
would have been better if that wasn't a surprise. My initial feeling
is that we should just fix that, rather than reverting at this stage,
but I understand what Adrien says about the Ubuntu cycle being at a
different point and this being a change that is causing trouble there.

Clearly only doing a rebuild for arm64 and assuming armhf would be
fine because the compiler team here said it would be was overly
optimistic. Sorry about that.

I see that this has already been reverted in Ubuntu dpkg, which seems
like the right thing to do there for the time being. For debian we'll
keep an eye on it, do a belated rebuild to see how much of a problem
we really have, and then decide if we should revert it too until some
stuff if fixed. We should have a better idea of whether to go back or
forward farily soon. I look forward to some more details on what
actually broke (other than valgrind) soon. 

Wookey
--
Principal hats:  Debian, Wookware, ARM
http://wookware.org/
signature.asc (application/pgp-signature, 833 B)