Open
Conversation
bettio
commented
Mar 16, 2026
| st_mode := integer(), | ||
| st_nlink := integer(), | ||
| st_uid := integer(), | ||
| st_gid := integer(), |
Collaborator
Author
There was a problem hiding this comment.
st_gid or gid? Should be easy to use / intuitive or match 1:1 fstat & stat structs?
Collaborator
There was a problem hiding this comment.
I'd favor POSIX names for documentation purposes
Collaborator
There was a problem hiding this comment.
+1 for using the POSIX names.
Fix memory leak in resource destructor, and a missing NULL check in the other dtor (this was relevant in case of smp_mutex_create failure). Also fix a few typos and similar minor issues. Signed-off-by: Davide Bettio <davide@uninstall.it>
Initialize all length variables to zero since cleanup uses secure_free, add marker comments for goto cleanup boundaries, and convert remaining RAISE_ERROR calls to goto cleanup where needed. Signed-off-by: Davide Bettio <davide@uninstall.it>
The existing test_use_after_final only covers CTR mode where crypto_final returns empty output. CBC with PKCS7 padding is more interesting because crypto_final produces the padded last block. This test verifies that both crypto_final called twice and crypto_update after crypto_final raise badarg in that mode too. Signed-off-by: Davide Bettio <davide@uninstall.it>
crypto_one_time declared cipher_ctx on the stack without calling mbedtls_cipher_init(), passing uninitialized memory to mbedtls_cipher_setup() (undefined behavior per the mbedTLS API). The mbed_error cleanup path also skipped mbedtls_cipher_free(), leaking internal mbedTLS allocations when setup succeeded but a later step failed. Signed-off-by: Davide Bettio <davide@uninstall.it>
strong_rand_bytes/1, generate_key/2, and sign/4 all depend on the platform CSPRNG. Add generic edoc warnings pointing users at their platform documentation, and an ESP32-specific warning box in the programmer's guide explaining that RF must be active for true randomness (predictable ECDSA nonces can lead to key recovery). Also strengthen the source comment in the ESP32 entropy init path to make the security implications explicit. Maintenance: the ESP32 documentation and warning should be revised once the TODO in src/platforms/esp32/components/avm_sys/sys.c (bootloader_random_enable() when radio is inactive) is addressed. Signed-off-by: Davide Bettio <davide@uninstall.it>
Use secure_free() instead of free() for allocated key and IV buffers in crypto_one_time so sensitive material is zeroed before being returned to the heap. Signed-off-by: Davide Bettio <davide@uninstall.it>
Export mac_state/0 opaque type, matching hash_state/0 and crypto_state/0 which were already exported. Align crypto_opts/0 with OTP: add the boolean() alternative so the type is `boolean() | [crypto_opt()]` instead of just `[crypto_opt()]`. This removes the need for a redundant `boolean() |` union in the crypto_init/3,4 specs. Signed-off-by: Davide Bettio <davide@uninstall.it>
Transfer key_id ownership to the cipher resource immediately after storing it, rather than deferring until after setup/IV steps. A failure in psa_cipher_encrypt_setup or psa_cipher_set_iv would jump to cleanup which destroys key_id directly and then releases the resource whose destructor destroys it again. Signed-off-by: Davide Bettio <davide@uninstall.it>
Fix crypto memory safety and document entropy requirements Fix several memory safety issues in `otp_crypto` and add documentation for platform entropy requirements. Fixes: - Resource destructor memory leak and missing NULL check - Goto cleanup inconsistencies (`secure_free` with uninitialized lengths) - Uninitialized `mbedtls_cipher_context_t` and missing `cipher_free` in `crypto_one_time` error path - Key and IV buffers freed with `free()` instead of `secure_free()` in `crypto_one_time` - Double-destroy of PSA key in `crypto_init` when cipher setup or set-IV fails - `temp_buf_size` overwritten by `mbedtls_cipher_crypt` output length before `secure_free` in `crypto_one_time`, leaving tail bytes unzeroed - Uninitialized `key_len` in `crypto_one_time` cleanup path Documentation: - Add edoc warnings to `strong_rand_bytes/1`, `generate_key/2`, and `sign/4` about platform entropy source dependency - Add ESP32-specific warning in programmer's guide: hardware RNG is only cryptographic when RF (Wi-Fi/Bluetooth) is active - Strengthen source comment in ESP32 entropy init path Tests: - Add use-after-final test for CBC with PKCS7 padding These changes are made under both the "Apache 2.0" and the "GNU Lesser General Public License 2.1 or later" license terms (dual license). SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later
On 32-bit platforms BOXED_TERMS_REQUIRED_FOR_INT differs from BOXED_TERMS_REQUIRED_FOR_INT64, so the if/else-if could fall through without returning. Restructure the control flow so every path returns a value, fixing -Werror=return-type on 32-bit archs. Signed-off-by: Davide Bettio <davide@uninstall.it>
Add seek, pread, pwrite, fsync, ftruncate, rename, stat and fstat NIFs to the atomvm module. Disable unavailable functions on RP2 (pread, pwrite, fsync, ftruncate) and STM32 (all new functions) where newlib declares the symbols but does not implement them. Signed-off-by: Davide Bettio <davide@uninstall.it>
globalcontext_make_atom returns term_invalid_term on allocation failure. Several call sites in posix_nifs.c used the result directly without checking, which could propagate an invalid term into map keys or return values. Signed-off-by: Davide Bettio <davide@uninstall.it>
Add mkdir(2) and rmdir(2) wrappers. Disable both on RP2 and STM32 where the underlying syscalls are not available. Signed-off-by: Davide Bettio <davide@uninstall.it>
bettio
force-pushed
the
more-posix-file-funcs
branch
from
7ea1f73 to
0a17088
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
| term result = term_alloc_tuple(2, &ctx->heap); | ||
| term_put_tuple_element(result, 0, OK_ATOM); | ||
| term_put_tuple_element(result, 1, term_make_maybe_boxed_int64(res, &ctx->heap)); |
Check failure
Code scanning / CodeQL
Allocations exceeding ensure_free Error
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extend the atomvm module with additional POSIX file operation NIFs:
seek, pread, pwrite, fsync, ftruncate, rename, stat, fstat, mkdir
and rmdir.
Highlights:
These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).
SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later