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

sys/net/rpl: fix possible NULL dereference #19611

Merged
merged 1 commit into from
May 22, 2023
Merged

Conversation

maribu
Copy link
Member

@maribu maribu commented May 17, 2023

Contribution description

As the title says

Testing procedure

No regression in RPL. (Beware: Fully untested!)

Issues/PRs references

fixes #15006 (or rather the last issue of the three instances reported there)

@maribu maribu 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 labels May 17, 2023
@github-actions github-actions bot added Area: network Area: Networking Area: sys Area: System labels May 17, 2023
@riot-ci
Copy link

riot-ci commented May 17, 2023

Murdock results

✔️ PASSED

273f92a sys/net/rpl: fix possible NULL dereference

Success Failures Total Runtime
6931 0 6931 10m:26s

Artifacts

Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

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

ACK. I tracked gnrc_rpl_validation_DAO_ACK() until the first access of dst without any prior NULL check.

@chrysn
Copy link
Member

chrysn commented May 22, 2023

I see no user that uses this with a NULL argument, nor is it documented as optional -- is silently returning a good thing to do here? (I'd softly lean towards "passing NULL to pointer arguments that don't say they can be NULL is generally not good").

@maribu
Copy link
Member Author

maribu commented May 22, 2023

I think github cut of crucial context here. Just one line above the snipped shown here we have:

#ifdef MODULE_NETSTATS_RPL
    gnrc_rpl_netstats_rx_DAO_ACK(&gnrc_rpl_netstats, len, (dst && !ipv6_addr_is_multicast(dst)));
#endif

As there is already a null pointer check for dst here, I think dst being NULL indeed is indeed intended to be allowed here.

bors bot added a commit that referenced this pull request May 22, 2023
17425: cpu/rpx0xx: initial PIO support r=benpicco a=fabian18



19611: sys/net/rpl: fix possible NULL dereference r=benpicco a=maribu

### Contribution description

As the title says


19640: core/thread: drop unused thread_arch_t r=benpicco a=maribu

### Contribution description

No architecture makes use of thread_arch_t anymore, so let's drop it.


Co-authored-by: Fabian Hüßler <fabian.huessler@st.ovgu.de>
Co-authored-by: Marian Buschsieweke <marian.buschsieweke@ovgu.de>
@bors
Copy link
Contributor

bors bot commented May 22, 2023

Build failed (retrying...):

@bors
Copy link
Contributor

bors bot commented May 22, 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 dc0fe52 into RIOT-OS:master May 22, 2023
@maribu maribu deleted the sys/net/rpl/fix branch May 24, 2023 19:08
@maribu
Copy link
Member Author

maribu commented May 24, 2023

Thx :)

@benpicco benpicco added this to the Release 2023.07 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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 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.

Two bugs may lead to NULL dereference.
5 participants