Skip to content
New issue

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

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

Already on GitHub? # to your account

nib/_nib-6ln: bail out early if address is no longer assigned #19999

Merged
merged 2 commits into from
Oct 24, 2023

Conversation

benpicco
Copy link
Contributor

Contribution description

The function checks for netif == NULL several times, but if netif really is NULL we will already crash on _nib_drl_get(NULL, netif->pid);.

Testing procedure

Issues/PRs references

@github-actions github-actions bot added Area: network Area: Networking Area: sys Area: System labels Oct 19, 2023
@benpicco benpicco added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed Area: network Area: Networking Area: sys Area: System labels Oct 19, 2023
@riot-ci
Copy link

riot-ci commented Oct 19, 2023

Murdock results

✔️ PASSED

8779b5e nib/_nib-6ln: don't check twice if address is assigned

Success Failures Total Runtime
7953 0 7953 17m:42s

Artifacts

@miri64 miri64 added the Area: network Area: Networking label Oct 24, 2023
@miri64
Copy link
Member

miri64 commented Oct 24, 2023

Any hint how to test this?

@benpicco
Copy link
Contributor Author

I'm not sure, I just saw a random crash when I was tinkering with CONFIG_GNRC_IPV6_NIB_MULTIHOP_P6C and fed the core into gdb where it showed a NULL pointer dereference of netif->pid.

Since the function checks below for netif to be not NULL, I think it should do so here too.

@benpicco benpicco force-pushed the _handle_rereg_address-NULL branch from 68e77e5 to 8779b5e Compare October 24, 2023 14:48
@github-actions github-actions bot added the Area: sys Area: System label Oct 24, 2023
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@miri64
Copy link
Member

miri64 commented Oct 24, 2023

bors merge

@MrKevinWeiss MrKevinWeiss added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Oct 24, 2023
@bors
Copy link
Contributor

bors bot commented Oct 24, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 647a13c into RIOT-OS:master Oct 24, 2023
@benpicco benpicco deleted the _handle_rereg_address-NULL branch October 25, 2023 08:19
bors bot added a commit that referenced this pull request Nov 2, 2023
20037: nib/_nib-6ln: bail out early if address is no longer assigned [backport 2023.10] r=benpicco a=MrKevinWeiss

# Backport of #19999



20038: nanocoap: prevent integer underflow in coap_opt_put_uri_pathquery() [backport 2023.10] r=benpicco a=MrKevinWeiss

# Backport of #19994





20039: sys/psa_crypto: Fix macro for public key max size and SE example [backport 2023.10] r=benpicco a=MrKevinWeiss

# Backport of #19995

### Contribution description
#### 1. Wrong public key size when using secure elements, introduced by  #19954
Fixed conditions for key size macros in `crypto_sizes.h`.

#### 2. EdDSA and ECDSA examples fail when using a secure element because of unsopported changes introduced by #19954
Updated `example/psa_crypto` to use only supported functions for secure elements.

### Testing procedure
Build `example/psa_crypto` for secure elements and run application

Output on master:
```
2023-10-19 14:33:24,372 # main(): This is RIOT! (Version: 2019.07-devel-22378-gb6772)
2023-10-19 14:33:24,372 # HMAC SHA256 took 56393 us
2023-10-19 14:33:24,372 # Cipher AES 128 took 68826 us
2023-10-19 14:33:24,372 # *** RIOT kernel panic:
2023-10-19 14:33:24,373 # HARD FAULT HANDLER
2023-10-19 14:33:24,373 # 
2023-10-19 14:33:24,373 # *** rebooting...

```
Output with fixes:
```
2023-10-19 13:35:24,715 # main(): This is RIOT! (Version: 2019.07-devel-22384-g8ef66-dev/psa-crypto-fixes)
2023-10-19 13:35:24,715 # HMAC SHA256 took 56374 us
2023-10-19 13:35:24,715 # Cipher AES 128 took 68805 us
2023-10-19 13:35:24,715 # ECDSA took 281164 us
2023-10-19 13:35:24,715 # All Done
```


Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
Co-authored-by: Lena Boeckmann <lena.boeckmann@haw-hamburg.de>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area: network Area: Networking Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants