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_ipv6_ext: don't read from pkt->data after _demux() #11580

Merged
merged 1 commit into from May 25, 2019

Conversation

miri64
Copy link
Member

@miri64 miri64 commented May 24, 2019

Contribution description

_demux() might change pkt->data in all kind of ways (moving it due
to gnrc_pktbuf_mark(), though unlikely; releasing it, because e.g. it
starts with a fragment header that marks a fragmented packet containing
only one fragment, etc.) so accessing the pointer after calling
_demux() is somewhat playing with fire. This change avoids this by
storing the value of ext_hdr->nh (all we are interested in here) in a
temporary variable that then is used to set the out-parameter nh.

protnum needs to be unchanged before the call to _demux() as it was
set by the previous iteration and determines what extension header
actually is handled (this is why a temporary variable is needed and we
can't just set *protnum).

Testing procedure

export BOARD="<your favorite>"
make -C tests/gnrc_ipv6_ext flash
sudo make -C tests/gnrc_ipv6_ext test

should still pass.

Issues/PRs references

None.

@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking labels May 24, 2019
@miri64 miri64 added this to the Release 2019.07 milestone May 24, 2019
`_demux()` might change `pkt->data` in all kind of ways (moving it due
to `gnrc_pktbuf_mark()`, though unlikely; releasing it, because e.g. it
starts with a fragment header that marks a fragmented packet containing
only one fragment, etc.) so accessing the pointer *after* calling
`_demux()` is somewhat playing with fire. This change avoids this by
storing the value of `ext_hdr->nh` (all we are interested in here) in a
temporary variable that then is used to set the out-parameter `nh`.

`protnum` needs to be unchanged before the call to `_demux()` as it was
set by the previous iteration and determines what extension header
actually is handled.
@miri64 miri64 force-pushed the gnrc_ipv6_ext/fix/nh-after-change branch from d9089bd to ea1708b Compare May 24, 2019 20:11
@kaspar030 kaspar030 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 25, 2019
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. Change looks good to me. Tested on master.

@miri64
Copy link
Member Author

miri64 commented May 25, 2019

Tested on master.

only on master? 🤨

@kaspar030
Copy link
Contributor

only on master? 🤨

nah, tested on native. 😉

@kaspar030 kaspar030 merged commit 4d052a6 into RIOT-OS:master May 25, 2019
@miri64 miri64 deleted the gnrc_ipv6_ext/fix/nh-after-change branch May 25, 2019 23:26
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 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.

None yet

2 participants