Avoid matching prefixes of a symbol as arm registers

[torben-hansen]

Issues:

Related to CryptoAlg-2625

Description of changes:

In some cases the peg parser incorrectly classify a prefix of a symbol reference instruction argument as an arm register. This causes the delocator to miss required re-writes.

The offending rule is:

ARMRegister <- [..] / ([xwdqshb] [0-9] [0-9]?) / [...]

e.g. the register x2 will match this rules, as it should. However, due to how the rest of the rules are formed this can cause strings such as x25519_.. to match the first prefix, when the string is an instruction argument, causing a wrong classification of tokens on a line. The parser is not architecture-aware, so this cause errors on other archs as well.

This was fine until now, because we had no symbols that required re-writes with a matching prefix. But to move curve25519 functions into the fips module, this assumption is no longer true. For example, x25519_ge_frombytes_vartime is one such function. Note we can't reduce the visibility of this symbol, because it's referenced in a compilation unit outside the fips module bcm.o (in spake25519.c).

The required re-write is due to relocations emitted for global symbols. The re-writing works as follow. Given a global symbol and a reference (inside fips module):

  .globl x25519_foo
x25519_foo:

[...]

bar:
  call x25519_foo

The correct delocater behaviour would be a re-write to *_local_target (suffix hardcoded in delocater):

  .globl x25519_foo
.Lx25519_foo_local_target
x25519_foo:

[...]

bar:
  call .Lx25519_foo_local_target

But in the current behaviour, the parser classify the prefix x25 of the instruction argument x25519_foo as an arm register. Subsequently, the delocater ignores call x25519_foo. The local target .Lx25519_foo_local_target is emitted, but the instruction argument is not re-written. That is, the following happens:

  .globl x25519_foo
.Lx25519_foo_local_target
x25519_foo:

[...]

bar:
  call x25519_foo

and a relocation is emitted, corrupting the FIPS integrity check at run-time.

Fix:

For the offending matching rule, we add an additional look-ahead condition !(ARMRegisterBoundary). Any matching on [xwdqshb] [0-9] [0-9]? must not contain a suffix of ARMRegisterBoundary.

Testing:

Added a test that global symbols with matching prefixing, e.g. x25519_foo, are re-written correctly.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.35%. Comparing base (35d5287) to head (c8ce45c).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1807      +/-   ##
==========================================
+ Coverage   78.34%   78.35%   +0.01%     
==========================================
  Files         581      581              
  Lines       97343    97343              
  Branches    13959    13959              
==========================================
+ Hits        76260    76270      +10     
+ Misses      20461    20451      -10     
  Partials      622      622              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.