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

Backport 2.16: Fix build failure on gcc-11 #3925

Merged
merged 8 commits into from
Dec 1, 2020

Conversation

rodrigo-dc
Copy link
Contributor

Description

Backport of #3848 . One extra change was needed to comply with C90.

Function prototypes changed to use array parameters instead of
pointers.

Signed-off-by: Rodrigo Dias Correa <rodrigo@correas.us>
In GCC 11, parameters declared as arrays in function prototypes
cannot be declared as pointers in the function definition. The
same is true for the other way around.

The definition of `mbedtls_aes_cmac_prf_128` was changed to match
its public prototype in `cmac.h`. The type `output` was
`unsigned char *`, now is `unsigned char [16]`.

In `ssl_tls.c`, all the `ssl_calc_verify_*` variants now use pointers
for the output `hash` parameter. The array parameters were removed
because those functions must be compatible with the function pointer
`calc_verify` (defined in `ssl_internal.h`).

Signed-off-by: Rodrigo Dias Correa <rodrigo@correas.us>
GCC 11 generated the warnings because the parameter `ret_buf`
was declared as `const char[10]`, but some of the arguments
provided in `run_test_snprintf` are shorter literals, like "".

Now the type of `ret_buf` is `const char *`.
Both implementations of `test_snprintf` were fixed.

Signed-off-by: Rodrigo Dias Correa <rodrigo@correas.us>
GCC 11 generated a warning because `padbuf` was too small to be
used as an argument for `mbedtls_sha512_finish_ret`. The `output`
parameter of `mbedtls_sha512_finish_ret` has the type
`unsigned char[64]`, but `padbuf` was only 48 bytes long.

Even though `ssl_calc_finished_tls_sha384` uses only 48 bytes for
the hash output, the size of `padbuf` was increased to 64 bytes.

Signed-off-by: Rodrigo Dias Correa <rodrigo@correas.us>
Signed-off-by: Rodrigo Dias Correa <rodrigo@correas.us>
This commit fixes the same warning fixed by baeedbf, but without
wasting RAM. By casting `mbedtls_sha512_finish_ret()`, `padbuf`
could be kept 48 bytes long without triggering any warnings.

Signed-off-by: Rodrigo Dias Correa <rodrigo@correas.us>
`finish_sha384_t` was made more generic by using `unsigned char*`
instead of `unsigned char[48]` as the second parameter.
This change tries to make the function casting more robust against
future improvements of gcc analysis.

Signed-off-by: Rodrigo Dias Correa <rodrigo@correas.us>
"declaration-after-statement" was generated because that code was
backported from the development branch, which currently uses C99.

Signed-off-by: Rodrigo Dias Correa <rodrigo@correas.us>
@gilles-peskine-arm gilles-peskine-arm added Community component-platform Portability layer and build scripts needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Nov 30, 2020
@gabor-mezei-arm gabor-mezei-arm self-requested a review December 1, 2020 09:58
@gabor-mezei-arm gabor-mezei-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Dec 1, 2020
@gilles-peskine-arm gilles-peskine-arm merged commit e98bbbe into Mbed-TLS:mbedtls-2.16 Dec 1, 2020
@mirko
Copy link

mirko commented Feb 9, 2021

Not sure due to my even more recent GCC version (master/HEAD at the time of writing) or it was missed in this PR, but with rev 57d705da of GCC I'm getting (with this PR included):

../../lib/mbedtls/library/ssl_tls.c: In function ‘ssl_calc_finished_tls_sha384’:
../../lib/mbedtls/library/ssl_tls.c:6408:5: error: ‘mbedtls_sha512_finish_ret’ accessing 64 bytes in a region of size 48 [-Werror=stringop-overflow=]
 6408 |     finish_sha384( &sha512, padbuf );
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../lib/mbedtls/library/ssl_tls.c:6408:5: note: referencing argument 2 of type ‘unsigned char *’
In file included from ../../lib/mbedtls/include/mbedtls/ssl_internal.h:74,
                 from ../../lib/mbedtls/library/ssl_tls.c:73:
../../lib/mbedtls/include/mbedtls/sha512.h:165:5: note: in a call to function ‘mbedtls_sha512_finish_ret’
  165 | int mbedtls_sha512_finish_ret( mbedtls_sha512_context *ctx,
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

as discussed in here: #4130

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports component-platform Portability layer and build scripts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants