Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GCC 11 warnings read/accessing out of regions #123

Closed
blocksebastian opened this issue Mar 21, 2022 · 4 comments
Closed

GCC 11 warnings read/accessing out of regions #123

blocksebastian opened this issue Mar 21, 2022 · 4 comments

Comments

@blocksebastian
Copy link

I just downloaded the Arm GNU Toolchain Version 11.2-2022.02 and want to cross-compile libhydrogen (current git).
It builds and runs, but the warnings about accessing data out of region make me feel uncomfortable.
System is a 32 Bit ARM.

Full build log

devshell bloc:/tmp/libhydrogen/build$ cmake --log-level=VERBOSE -DCMAKE_C_COMPILER=arm-none-linux-gnueabihf-gcc -DCMAKE_C_FLAGS="-mcpu=cortex-a9 -mfpu=vfpv3" -DCMAKE_SYSTEM_NAME=Linux ..
-- The C compiler identification is GNU 11.2.1
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /opt/gcc-arm-none-linux-gnueabihf-x86_64-festo-11.2-2022.02/bin/arm-none-linux-gnueabihf-gcc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Configuring done
-- Generating done
-- Build files have been written to: /tmp/libhydrogen/build
devshell bloc:/tmp/libhydrogen/build$ make 
Scanning dependencies of target hydrogen
[ 50%] Building C object CMakeFiles/hydrogen.dir/hydrogen.c.o
In file included from /tmp/libhydrogen/hydrogen.c:15:
/tmp/libhydrogen/impl/x25519.h: In function ‘hydro_x25519_ladder_part1’:
/tmp/libhydrogen/impl/x25519.h:232:5: warning: ‘hydro_x25519_mul’ reading 32 bytes from a region of size 4 [-Wstringop-overread]
  232 |     hydro_x25519_mul(z2, x2, hydro_x25519_a24, // z2 = E*a24
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  233 |                      sizeof(hydro_x25519_a24) / sizeof(hydro_x25519_a24[0]));
      |                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/libhydrogen/impl/x25519.h:232:5: note: referencing argument 3 of type ‘const hydro_x25519_limb_t *’ {aka ‘const unsigned int *’}
/tmp/libhydrogen/impl/x25519.h:141:1: note: in a call to function ‘hydro_x25519_mul’
  141 | hydro_x25519_mul(hydro_x25519_fe out, const hydro_x25519_fe a, const hydro_x25519_fe b, int nb)
      | ^~~~~~~~~~~~~~~~
In file included from /tmp/libhydrogen/hydrogen.c:19:
In function ‘hydro_sign_verify_p2’,
    inlined from ‘hydro_sign_verify_challenge’ at /tmp/libhydrogen/impl/sign.h:117:12,
    inlined from ‘hydro_sign_final_verify’ at /tmp/libhydrogen/impl/sign.h:178:12:
/tmp/libhydrogen/impl/sign.h:103:5: warning: ‘hydro_x25519_core’ accessing 160 bytes in a region of size 32 [-Wstringop-overflow=]
  103 |     hydro_x25519_core(&xs[0], challenge, pk, 0);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/libhydrogen/impl/sign.h: In function ‘hydro_sign_final_verify’:
/tmp/libhydrogen/impl/sign.h:103:5: note: referencing argument 1 of type ‘hydro_x25519_limb_t (*)[8]’ {aka ‘unsigned int (*)[8]’}
In file included from /tmp/libhydrogen/hydrogen.c:15:
/tmp/libhydrogen/impl/x25519.h:251:1: note: in a call to function ‘hydro_x25519_core’
  251 | hydro_x25519_core(hydro_x25519_fe xs[5], const uint8_t scalar[hydro_x25519_BYTES],
      | ^~~~~~~~~~~~~~~~~
In file included from /tmp/libhydrogen/hydrogen.c:19:
In function ‘hydro_sign_verify_p2’,
    inlined from ‘hydro_sign_verify_challenge’ at /tmp/libhydrogen/impl/sign.h:117:12,
    inlined from ‘hydro_sign_final_verify’ at /tmp/libhydrogen/impl/sign.h:178:12:
/tmp/libhydrogen/impl/sign.h:104:5: warning: ‘hydro_x25519_core’ accessing 160 bytes in a region of size 32 [-Wstringop-overflow=]
  104 |     hydro_x25519_core(&xs[2], sig, hydro_x25519_BASE_POINT, 0);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/libhydrogen/impl/sign.h: In function ‘hydro_sign_final_verify’:
/tmp/libhydrogen/impl/sign.h:104:5: note: referencing argument 1 of type ‘hydro_x25519_limb_t (*)[8]’ {aka ‘unsigned int (*)[8]’}
In file included from /tmp/libhydrogen/hydrogen.c:15:
/tmp/libhydrogen/impl/x25519.h:251:1: note: in a call to function ‘hydro_x25519_core’
  251 | hydro_x25519_core(hydro_x25519_fe xs[5], const uint8_t scalar[hydro_x25519_BYTES],
      | ^~~~~~~~~~~~~~~~~
In file included from /tmp/libhydrogen/hydrogen.c:19:
In function ‘hydro_sign_verify_core’,
    inlined from ‘hydro_sign_verify_p2’ at /tmp/libhydrogen/impl/sign.h:106:12,
    inlined from ‘hydro_sign_verify_challenge’ at /tmp/libhydrogen/impl/sign.h:117:12,
    inlined from ‘hydro_sign_final_verify’ at /tmp/libhydrogen/impl/sign.h:178:12:
/tmp/libhydrogen/impl/sign.h:74:5: warning: ‘hydro_x25519_ladder_part1’ accessing 160 bytes in a region of size 32 [-Wstringop-overflow=]
   74 |     hydro_x25519_ladder_part1(xs);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/libhydrogen/impl/sign.h: In function ‘hydro_sign_final_verify’:
/tmp/libhydrogen/impl/sign.h:74:5: note: referencing argument 1 of type ‘hydro_x25519_limb_t (*)[8]’ {aka ‘unsigned int (*)[8]’}
In file included from /tmp/libhydrogen/hydrogen.c:15:
/tmp/libhydrogen/impl/x25519.h:217:1: note: in a call to function ‘hydro_x25519_ladder_part1’
  217 | hydro_x25519_ladder_part1(hydro_x25519_fe xs[5])
      | ^~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /tmp/libhydrogen/hydrogen.c:19:
In function ‘hydro_sign_verify_core’,
    inlined from ‘hydro_sign_verify_p2’ at /tmp/libhydrogen/impl/sign.h:106:12,
    inlined from ‘hydro_sign_verify_challenge’ at /tmp/libhydrogen/impl/sign.h:117:12,
    inlined from ‘hydro_sign_final_verify’ at /tmp/libhydrogen/impl/sign.h:178:12:
/tmp/libhydrogen/impl/sign.h:81:5: warning: ‘hydro_x25519_mul’ reading 32 bytes from a region of size 4 [-Wstringop-overread]
   81 |     hydro_x25519_mul(z2, z2, &sixteen, 1);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/tmp/libhydrogen/impl/sign.h: In function ‘hydro_sign_final_verify’:
/tmp/libhydrogen/impl/sign.h:81:5: note: referencing argument 3 of type ‘const hydro_x25519_limb_t *’ {aka ‘const unsigned int *’}
In file included from /tmp/libhydrogen/hydrogen.c:15:
/tmp/libhydrogen/impl/x25519.h:141:1: note: in a call to function ‘hydro_x25519_mul’
  141 | hydro_x25519_mul(hydro_x25519_fe out, const hydro_x25519_fe a, const hydro_x25519_fe b, int nb)
      | ^~~~~~~~~~~~~~~~
[100%] Linking C static library libhydrogen.a
[100%] Built target hydrogen

Patches

The first is quite easy to patch as the number of elements is given to the function and hydro_x25519_a24 is only defined as an array with length 1.

diff --git a/impl/x25519.h b/impl/x25519.h
index efeb9a4..09bd96f 100644
--- a/impl/x25519.h
+++ b/impl/x25519.h
@@ -138,7 +139,7 @@ hydro_x25519_swapout(uint8_t *out, hydro_x25519_limb_t *x)
 }
 
 static void
-hydro_x25519_mul(hydro_x25519_fe out, const hydro_x25519_fe a, const hydro_x25519_fe b, int nb)
+hydro_x25519_mul(hydro_x25519_fe out, const hydro_x25519_fe a, const hydro_x25519_limb_t *b, int nb)
 {
     hydro_x25519_limb_t accum[2 * hydro_x25519_NLIMBS] = { 0 };
     hydro_x25519_limb_t carry2;

The other warnings are a little harder since it want hydro_x25519_fe[5] but there are different size of arrays.
I'm not aware how to cast a "hydro_x25519_fe *" as it is used in hydro_sign_verify_p2 to a "hydro_x25519_fe[5]".

diff --git a/impl/x25519.h b/impl/x25519.h
index efeb9a4..09bd96f 100644
--- a/impl/x25519.h
+++ b/impl/x25519.h
@@ -214,7 +215,7 @@ hydro_x25519_canon(hydro_x25519_fe x)
 }
 
 static void
-hydro_x25519_ladder_part1(hydro_x25519_fe xs[5])
+hydro_x25519_ladder_part1(hydro_x25519_fe *xs)
 {
     hydro_x25519_limb_t *x2 = xs[0], *z2 = xs[1], *x3 = xs[2], *z3 = xs[3], *t1 = xs[4];
 
@@ -248,7 +249,7 @@ hydro_x25519_ladder_part2(hydro_x25519_fe xs[5], const hydro_x25519_fe x1)
 }
 
 static void
-hydro_x25519_core(hydro_x25519_fe xs[5], const uint8_t scalar[hydro_x25519_BYTES],
+hydro_x25519_core(hydro_x25519_fe *xs, const uint8_t scalar[hydro_x25519_BYTES],
                   const uint8_t *x1, bool clamp)
 {
     hydro_x25519_limb_t  swap;

@solardiz
Copy link
Contributor

I'm not convinced this is in fact fixed. Here are the warnings I'm getting when using current hydrogen, gcc version 11.2.1 20210728 (Red Hat 11.2.1-1) on x86_64 (here keygen.c is my program, into which hydrogen.c is included):

In file included from hydrogen/hydrogen.c:18,
                 from keygen.c:3:
In function 'hydro_sign_verify_p2',
    inlined from 'hydro_sign_verify_challenge' at hydrogen/impl/sign.h:117:12,
    inlined from 'hydro_sign_final_verify' at hydrogen/impl/sign.h:178:12:
hydrogen/impl/sign.h:103:5: warning: 'hydro_x25519_core' accessing 160 bytes in a region of size 32 [�]8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstringop-overflow=-Wstringop-overflow=�]8;;]
  103 |     hydro_x25519_core(&xs[0], challenge, pk, 0);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
hydrogen/impl/sign.h: In function 'hydro_sign_final_verify':
hydrogen/impl/sign.h:103:5: note: referencing argument 1 of type 'hydro_x25519_limb_t (*)[4]' {aka 'long unsigned int (*)[4]'}
In file included from hydrogen/hydrogen.c:15,
                 from keygen.c:3:
hydrogen/impl/x25519.h:253:1: note: in a call to function 'hydro_x25519_core'
  253 | hydro_x25519_core(hydro_x25519_fe xs[5], const uint8_t scalar[hydro_x25519_BYTES],
      | ^~~~~~~~~~~~~~~~~
In file included from hydrogen/hydrogen.c:18,
                 from keygen.c:3:
In function 'hydro_sign_verify_p2',
    inlined from 'hydro_sign_verify_challenge' at hydrogen/impl/sign.h:117:12,
    inlined from 'hydro_sign_final_verify' at hydrogen/impl/sign.h:178:12:
hydrogen/impl/sign.h:104:5: warning: 'hydro_x25519_core' accessing 160 bytes in a region of size 32 [�]8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstringop-overflow=-Wstringop-overflow=�]8;;]
  104 |     hydro_x25519_core(&xs[2], sig, hydro_x25519_BASE_POINT, 0);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
hydrogen/impl/sign.h: In function 'hydro_sign_final_verify':
hydrogen/impl/sign.h:104:5: note: referencing argument 1 of type 'hydro_x25519_limb_t (*)[4]' {aka 'long unsigned int (*)[4]'}
In file included from hydrogen/hydrogen.c:15,
                 from keygen.c:3:
hydrogen/impl/x25519.h:253:1: note: in a call to function 'hydro_x25519_core'
  253 | hydro_x25519_core(hydro_x25519_fe xs[5], const uint8_t scalar[hydro_x25519_BYTES],
      | ^~~~~~~~~~~~~~~~~
In file included from hydrogen/hydrogen.c:18,
                 from keygen.c:3:
In function 'hydro_sign_verify_core',
    inlined from 'hydro_sign_verify_p2' at hydrogen/impl/sign.h:106:12,
    inlined from 'hydro_sign_verify_challenge' at hydrogen/impl/sign.h:117:12,
    inlined from 'hydro_sign_final_verify' at hydrogen/impl/sign.h:178:12:
hydrogen/impl/sign.h:74:5: warning: 'hydro_x25519_ladder_part1' accessing 160 bytes in a region of size 32 [�]8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstringop-overflow=-Wstringop-overflow=�]8;;]
   74 |     hydro_x25519_ladder_part1(xs);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
hydrogen/impl/sign.h: In function 'hydro_sign_final_verify':
hydrogen/impl/sign.h:74:5: note: referencing argument 1 of type 'hydro_x25519_limb_t (*)[4]' {aka 'long unsigned int (*)[4]'}
In file included from hydrogen/hydrogen.c:15,
                 from keygen.c:3:
hydrogen/impl/x25519.h:219:1: note: in a call to function 'hydro_x25519_ladder_part1'
  219 | hydro_x25519_ladder_part1(hydro_x25519_fe xs[5])
      | ^~~~~~~~~~~~~~~~~~~~~~~~~

and with gcc version 11.3.0 (Ubuntu 11.3.0-1ubuntu1~22.04) on aarch64 (looks exactly the same):

In file included from hydrogen/hydrogen.c:18,
                 from keygen.c:3:
In function ‘hydro_sign_verify_p2’,
    inlined from ‘hydro_sign_verify_challenge’ at hydrogen/impl/sign.h:117:12,
    inlined from ‘hydro_sign_final_verify’ at hydrogen/impl/sign.h:178:12:
hydrogen/impl/sign.h:103:5: warning: ‘hydro_x25519_core’ accessing 160 bytes in a region of size 32 [�]8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstringop-overflow=-Wstringop-overflow=�]8;;]
  103 |     hydro_x25519_core(&xs[0], challenge, pk, 0);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
hydrogen/impl/sign.h: In function ‘hydro_sign_final_verify’:
hydrogen/impl/sign.h:103:5: note: referencing argument 1 of type ‘hydro_x25519_limb_t (*)[4]’ {aka ‘long unsigned int (*)[4]’}
In file included from hydrogen/hydrogen.c:15,
                 from keygen.c:3:
hydrogen/impl/x25519.h:253:1: note: in a call to function ‘hydro_x25519_core’
  253 | hydro_x25519_core(hydro_x25519_fe xs[5], const uint8_t scalar[hydro_x25519_BYTES],
      | ^~~~~~~~~~~~~~~~~
In file included from hydrogen/hydrogen.c:18,
                 from keygen.c:3:
In function ‘hydro_sign_verify_p2’,
    inlined from ‘hydro_sign_verify_challenge’ at hydrogen/impl/sign.h:117:12,
    inlined from ‘hydro_sign_final_verify’ at hydrogen/impl/sign.h:178:12:
hydrogen/impl/sign.h:104:5: warning: ‘hydro_x25519_core’ accessing 160 bytes in a region of size 32 [�]8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstringop-overflow=-Wstringop-overflow=�]8;;]
  104 |     hydro_x25519_core(&xs[2], sig, hydro_x25519_BASE_POINT, 0);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
hydrogen/impl/sign.h: In function ‘hydro_sign_final_verify’:
hydrogen/impl/sign.h:104:5: note: referencing argument 1 of type ‘hydro_x25519_limb_t (*)[4]’ {aka ‘long unsigned int (*)[4]’}
In file included from hydrogen/hydrogen.c:15,
                 from keygen.c:3:
hydrogen/impl/x25519.h:253:1: note: in a call to function ‘hydro_x25519_core’
  253 | hydro_x25519_core(hydro_x25519_fe xs[5], const uint8_t scalar[hydro_x25519_BYTES],
      | ^~~~~~~~~~~~~~~~~
In file included from hydrogen/hydrogen.c:18,
                 from keygen.c:3:
In function ‘hydro_sign_verify_core’,
    inlined from ‘hydro_sign_verify_p2’ at hydrogen/impl/sign.h:106:12,
    inlined from ‘hydro_sign_verify_challenge’ at hydrogen/impl/sign.h:117:12,
    inlined from ‘hydro_sign_final_verify’ at hydrogen/impl/sign.h:178:12:
hydrogen/impl/sign.h:74:5: warning: ‘hydro_x25519_ladder_part1’ accessing 160 bytes in a region of size 32 [�]8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstringop-overflow=-Wstringop-overflow=�]8;;]
   74 |     hydro_x25519_ladder_part1(xs);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
hydrogen/impl/sign.h: In function ‘hydro_sign_final_verify’:
hydrogen/impl/sign.h:74:5: note: referencing argument 1 of type ‘hydro_x25519_limb_t (*)[4]’ {aka ‘long unsigned int (*)[4]’}
In file included from hydrogen/hydrogen.c:15,
                 from keygen.c:3:
hydrogen/impl/x25519.h:219:1: note: in a call to function ‘hydro_x25519_ladder_part1’
  219 | hydro_x25519_ladder_part1(hydro_x25519_fe xs[5])
      | ^~~~~~~~~~~~~~~~~~~~~~~~~

@solardiz
Copy link
Contributor

@jedisct1 You'll probably want to reopen this issue until it's actually confirmed fixed. Thanks.

jedisct1 added a commit that referenced this issue Nov 15, 2022
@jedisct1
Copy link
Owner

@solardiz That looks like a bug in the analyzer.

1eee2b2 works around it.

@solardiz
Copy link
Contributor

@jedisct1 Thank you for the prompt fix! I confirm it avoids the warnings on my two systems above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants