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

gnrc_netif: fix double free with netdev_new_api & gnrc_netif_pktq #20976

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

benpicco
Copy link
Contributor

Contribution description

Return early to prevent freeing the same packet again.

Testing procedure

Issues/PRs references

@benpicco benpicco requested a review from maribu November 11, 2024 19:32
@github-actions github-actions bot added Area: network Area: Networking Area: sys Area: System labels Nov 11, 2024
@benpicco benpicco added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Nov 11, 2024
@@ -1813,6 +1813,7 @@ static void _tx_done(gnrc_netif_t *netif, gnrc_pktsnip_t *pkt,
else {
/* remove previously held packet */
gnrc_pktbuf_release(pkt);
return;
}
#endif /* IS_USED(MODULE_GNRC_NETIF_PKTQ) */

Copy link
Member

Choose a reason for hiding this comment

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

Maybe, just to prevent stuff like this in the future, also add an early/explicit return, in the if (gnrc_netif_netdev_new_api(netif)) { block?

@miri64
Copy link
Member

miri64 commented Nov 11, 2024

Makes sense and saw the pain of @benpicco debugging this live unfolding.

@maribu maribu enabled auto-merge November 11, 2024 20:59
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 11, 2024
@riot-ci
Copy link

riot-ci commented Nov 11, 2024

Murdock results

✔️ PASSED

0bed0c6 gnrc_netif: fix double free with netdev_new_api & gnrc_netif_pktq

Success Failures Total Runtime
10215 0 10215 13m:32s

Artifacts

@maribu maribu added this pull request to the merge queue Nov 11, 2024
Merged via the queue into RIOT-OS:master with commit 0d4256f Nov 12, 2024
29 checks passed
@benpicco benpicco deleted the gnrc_netif-double-free branch November 12, 2024 09:49
@MrKevinWeiss MrKevinWeiss added this to the Release 2025.01 milestone Jan 20, 2025
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.

5 participants