Skip to content

gnrc_icmpv6_echo: avoid crashing when pktbuf full #10869

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

Merged
merged 1 commit into from
Jan 25, 2019

Conversation

gschorcht
Copy link
Contributor

Contribution description

This PR fixes the unchecked access to (gnrc_netif_hdr_t *)hdr->data where hdr is returned from gnrc_netif_hdr_build. If packet buffer is full, gnrc_netif_hdr_build may return NULL. The following unchecked access to the pointer can then lead to a crash.

Testing procedure

Produce a lot of network traffic using ping command with maximum data size and an intervall of 0, if necessary from multiple terminals so that the packet buffer becomes full, e.g.

sudo ping6 fe80::5ecf:7fff:fe80:3f08 -Ieth0 -s1392 -i0

This test should not lead to a crash.

Issues/PRs references

Problem was found during testing PR #10862 and described in issue #10861.

Once the packet buffer is full on heavy network load, gnrc_netif_hdr_build may return NULL. In that case, the following unchecked access to hdr->data leads to a crash.
@gschorcht gschorcht requested a review from miri64 January 25, 2019 16:21
@gschorcht gschorcht added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking labels Jan 25, 2019
@miri64
Copy link
Member

miri64 commented Jan 25, 2019

With native this isn't really that easy to reproduce, since the TAP interface is able to handle the load quite fast (it even swallowed

for _ in $(seq 10); do sudo ping6 fe80::b41f:ceff:febe:78d3%tapbr0 -s1452 -i0 -c 100000 & done

or

for _ in $(seq 10); do sudo ping6 fe80::b41f:ceff:febe:78d3%tapbr0 -s1452 -f -c 100000 & done

mostly without problems). So I can't really test. However

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.

Changes make sense ACK

@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 25, 2019
@miri64 miri64 merged commit 2d54069 into RIOT-OS:master Jan 25, 2019
@miri64 miri64 added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Jan 25, 2019
@miri64
Copy link
Member

miri64 commented Jan 25, 2019

Please provide a backport to the 2019.01-branch (You can use the script in #8968 for that if you want to).

@gschorcht
Copy link
Contributor Author

@miri64

With native this isn't really that easy to reproduce, since the TAP interface is able to handle the load quite fast (it even swallowed ...

When bombarding examples/gnrc_networking on native using

sudo ping6 fe80::280d:21ff:fed1:c5ed -Itap0 -s1392 -i 0

in 4 terminals, it crashes with

> examples/gnrc_networking/../../Makefile.include:540: recipe for target 'term' failed
make: *** [term] segmentation violation (core dumped)

Where can I find the core file to take a look why it crashed?

@kaspar030
Copy link
Contributor

Where can I find the core file to take a look why it crashed?

I had the same question for a while. On systemd systems, it is probably in /var/lib/systemd/coredump. Otherwise, /proc/sys/kernel/core_pattern might give a hint.

@kaspar030
Copy link
Contributor

why it crashed

I usually compile native with CFLAGS += -g, then run it within gdb. On crash, get a backtrace with "bt".

@gschorcht
Copy link
Contributor Author

I had the same question for a while. On systemd systems, it is probably in /var/lib/systemd/coredump. Otherwise, /proc/sys/kernel/core_pattern might give a hint.

Thanks. I'm using UNIX/Linux since 25 years, systemd is a hell. In the past it was so easy, the core file was simply created in the current directory.

@gschorcht gschorcht deleted the gnrc_icmpv6_echo_fix branch January 26, 2019 09:14
@aabadie aabadie added this to the Release 2019.01 milestone Jan 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking 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