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

tests: Fix untermined string initializaions #450

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Jakuje
Copy link
Contributor

@Jakuje Jakuje commented Feb 5, 2025

The new GCC 15 reports error when the string initializers overflow the size of the underlying structure. This is common when the byte strings are constructed in quotes such as "\xBB" as such this string has trailing null byte and therefore the size two.

This is not an issue in the tests as they do not expect the string to be NULL terminated, but it might uncover issues in other cases.

Example of the error:

/builddir/build/BUILD/yubihsm-shell-2.6.0-build/yubihsm-shell-2.6.0/pkcs11/tests/aes_encrypt_test.c:38:3: error: initializer-string for array of ‘unsigned char’ is too long [-Werror=unterminated-string-initialization]
   38 |   "\x6b\xc1\xbe\xe2\x2e\x40\x9f\x96\xe9\x3d\x7e\x11\x73\x93\x17\x2a"
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Some cases are easy to rewrite to byte list, some places already had suspicious +1 in the buffers to accommodate this and for string where really a strlen is expected, I tried to change it that way. Let me know if this works for you or there is something to improve.

Untested with HW.

The new GCC 15 reports error when the string initializers overflow the
size of the underlying structure. This is common when the byte strings
are constructed in quotes such as "\xBB" as such this string has
trailing null byte and therefore the size two.

This is not an issue in the tests as they do not expect the string to be
NULL terminated, but it might uncover issues in other cases.

Example of the error:

/builddir/build/BUILD/yubihsm-shell-2.6.0-build/yubihsm-shell-2.6.0/pkcs11/tests/aes_encrypt_test.c:38:3: error: initializer-string for array of ‘unsigned char’ is too long [-Werror=unterminated-string-initialization]
   38 |   "\x6b\xc1\xbe\xe2\x2e\x40\x9f\x96\xe9\x3d\x7e\x11\x73\x93\x17\x2a"
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Jakub Jelen <[email protected]>
@qpernil
Copy link
Contributor

qpernil commented Feb 5, 2025

Would you mind rebasing this on master, to get CI to run correctly. Thanks.

@Jakuje
Copy link
Contributor Author

Jakuje commented Feb 5, 2025

Would you mind rebasing this on master, to get CI to run correctly. Thanks.

I see this commit directly on top of current master on my end and also github thinks so:

This branch is 1 commit ahead of Yubico/yubihsm-shell:master.

https://github.com/Jakuje/yubihsm-shell/tree/gcc15

Or is there something else I should look into?

@qpernil
Copy link
Contributor

qpernil commented Feb 6, 2025

Sorry I didn't check closely enough, we had very similar failures just before this caused by an expired certificate, so I assumed you hadn't rebased on top of the fix for that. But when I check properly I see your failures are because of lack of access to a git secret for external PRs. I will run tests manually instead.

@qpernil
Copy link
Contributor

qpernil commented Feb 6, 2025

Running tests against HW succeeds.

@qpernil
Copy link
Contributor

qpernil commented Feb 6, 2025

Thanks for the contribution, I have approved it. I always thought it a bit strange that string literals were silently truncated when (only) the trailing null was overflowing. Apparently GCC 15 has had enough of such nonsense :-)

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

Successfully merging this pull request may close these issues.

2 participants