Skip to content

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

Merged
bors[bot] merged 2 commits intoRIOT-OS:masterfrom
benpicco:_handle_rereg_address-NULL
Oct 24, 2023
Merged

nib/_nib-6ln: bail out early if address is no longer assigned#19999
bors[bot] merged 2 commits intoRIOT-OS:masterfrom
benpicco:_handle_rereg_address-NULL

Conversation

@benpicco
Copy link
Copy Markdown
Contributor

@benpicco benpicco commented Oct 19, 2023

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
Copy Markdown

riot-ci commented Oct 19, 2023 •
edited
Loading

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
Copy Markdown
Member

miri64 commented Oct 24, 2023

Any hint how to test this?

@benpicco
Copy link
Copy Markdown
Contributor Author

benpicco commented Oct 24, 2023

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.

miri64
miri64 requested changes Oct 24, 2023
View reviewed changes
@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
miri64
miri64 approved these changes Oct 24, 2023
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
@MrKevinWeiss MrKevinWeiss mentioned this pull request Nov 2, 2023
Merged
bors bot added a commit that referenced this pull request Nov 2, 2023
@bors @benpicco @Einhornhool
Merge #20037 #20038 #20039
2ffdc2c 
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 <[email protected]>
Co-authored-by: Lena Boeckmann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miri64 miri64 miri64 approved these changes

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann

@cgundogan cgundogan Awaiting requested review from cgundogan

Assignees

No one assigned

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

Milestone

Release 2023.10

Development

Successfully merging this pull request may close these issues.

4 participants

@benpicco @riot-ci @miri64 @MrKevinWeiss