-
Notifications
You must be signed in to change notification settings - Fork 5
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
Various bug fixes in the network stack #46
Merged
Merged
Conversation
This file contains 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
hlef
force-pushed
the
hlefeuvre/bug-fixes
branch
3 times, most recently
from
October 24, 2024 17:46
05a1ee6
to
229ec5e
Compare
This got introduced in 5269a8e as part of a refactoring and prevents the file to build when debugging is enabled. Signed-off-by: Hugo Lefeuvre <[email protected]>
We had copies of `ntohs` and `htons` in the firewall, the netapi, and the TCP/IP stack, implemented differently. In practice these differences shouldn't matter, but this is still confusing. Signed-off-by: Hugo Lefeuvre <[email protected]>
We have a number of APIs in the firewall and the netAPI which do not specify in which endianness arguments should be provided. Further, we have many cases where we use `ntohs` and `htons` the incorrect way around. Ensure that endianness of APIs is documented everywhere and conversions are done correctly. Also, consistently use `htons` and `ntohs`, not `FreeRTOS_htons` and `FreeRTOS_ntohs`. Signed-off-by: Hugo Lefeuvre <[email protected]>
This got outdated when we added UDP, it can be called from both NetAPI and TCP/IP. Signed-off-by: Hugo Lefeuvre <[email protected]>
The definition of the struct in `struct ConnectionCapability` and in `DECLARE_AND_DEFINE_CONNECTION_CAPABILITY` differ in the type of `port`. The struct definition uses `uint16_t` and the macro `short`. `short` is signed and may be longer that 16 bits. This is not a problem right now but may lead to portability issues. Signed-off-by: Hugo Lefeuvre <[email protected]>
The TLS currently uses null as error value for pointer types, which does not work well when the compartment fails with `-ECOMPARTMENTFAIL` or `-ENOTENOUGHSTACK`. Document error values for pointer types as untagged values instead of null. This does not require changes in the TLS implementation, as null is itself untagged. Also clarify blocking behavior for each API. Signed-off-by: Hugo Lefeuvre <[email protected]>
The `topic` and `payload` capabilities of the publish callback are only valid within the context of the callback. They should thus passed as a read-only, non-capturable capabilities. Currently we pass them as capturable and writable capabilities, which may allow API users to compromise the MQTT compartment. This addresses issue #43. Signed-off-by: Hugo Lefeuvre <[email protected]>
hlef
force-pushed
the
hlefeuvre/bug-fixes
branch
from
October 29, 2024 00:24
229ec5e
to
9409c18
Compare
Sounds like CI is failing because of CHERIoT-Platform/cheriot-rtos@c3c0fb1 |
Comments addressed! |
The RTOS core changed the queue API: CHERIoT-Platform/cheriot-rtos@c3c0fb1 This requires slight changes to the network stack reset code, which plugs in at this lower-level API. Signed-off-by: Hugo Lefeuvre <[email protected]>
Should be fixed now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR clears some of my bugfix TODO list. This addresses a lot of documentation issues, but also actual bugs such as #43.