While it is correct to not use an invalid address as a source address,
it is incorrect to assume that addresses not assigned to the interface
(`idx == -1` in the respective piece of code) are invalid: Other than
classic forwarding via a FIB, forwarded packets utilizing a IPv6
routing header will pass this check, like any other packet sent by this
node. The source address for these is not on the given node, so e.g.
source routing is not possible at the moment.
The IPv6 (extension) headers of the first fragment received are re-used
for the reassembled packet, so when receiving a subsequent packet we
need to distinguish, if we just want to release the payload or all of
the packet after the packet data was added to the reassembly buffer.
Due to some changes to the minimal forwarding draft and in preparation
for Selective Fragment Recovery some changes to the VRB API were
needed. Now the index of a VRB entry is only (L2 src, tag) not as
before (L2 src, L2 dst, length, tag).
I know that the current `rbuf_base` causes waste, as all the fields not
used by the new index are effectively not used by the VRB. I'd like to
fix that however in a later change, since that also requires some
modifications of the classic reassembly buffer, and thus would
complicate the review and testing of the change.
Sources for the index change:
- https://tools.ietf.org/html/draft-ietf-6lo-minimal-fragment-04#section-1
- https://mailarchive.ietf.org/arch/browse/6lo/?gbt=1&index=DLCTxC2X4bRNtYPHhtEkavMWlz4
before this commit the src address was checked for multicast, but the dst address should be checked. Therefore udp multicast packets would be flooded back to the src as ICMPv6 error, as not all nodes had a UDP receiver registered.
If an address was pre-configured by the upper layer its validity is
currently ignored. It is neither checked if the address is on the
interface at all nor is it checked if it is valid.
This change provides a fix for that by checking both facts.
When reworking the reception of IPv6 packets I reset a previously set
`ipv6` snip as follows when the IPv6 extension handler returns a
packet (see first hunk of this commit):
```C
ipv6 = pkt->next->next
```
With `gnrc_ipv6_ext` this makes *somewhat* sense, `pkt->next` was
previously equal to `ipv6` and after the function call `pkt->next`
is the marked extension header, while `pkt->next->next` is the IPv6
header. However, since `ipv6` is already write-protected i.e.
`ipv6->users == 1` (see ll. 665-675), any additional call of
`gnrc_pktbuf_start_write()` [won't][start-write-doc] duplicate the
packet. In fact, the only `gnrc_pktbuf_start_write()` in
`gnrc_ipv6_ext` is used to send the *result* to the subscribers of that
extension header type, leaving the original packet unchanged for the
caller. As such `ipv6` remains the pointer to the IPv6 header whether
we set it in the line above or not. So we actually don't need that
line.
However, the extension header handling also returns a packet when
`gnrc_ipv6_ext` is not compiled in. In that case it is just a dummy
define that returns the packet you give provide it which means that
this still holds true: `pkt->next == ipv6`.
So setting `ipv6` in this case is actually harmful, as `ipv6` now
points to the NETIF header [following the IPv6 header][pkt-structure]
in the packet and this causes the `user` counter of that NETIF header
`hdr` to be decremented if `hdr->users > 1` in the write-protection I
removed in hunk 2 of this commit:
```C
/* pkt might not be writable yet, if header was given above */
ipv6 = gnrc_pktbuf_start_write(ipv6);
if (ipv6 == NULL) {
DEBUG("ipv6: unable to get write access to packet: dropping it\n");
gnrc_pktbuf_release(pkt);
return;
}
```
But as we already established, `ipv6->users` is already 1, so we don't
actually need the write protection here either.
Since the packet stays unchanged after the `ipv6` snip, we also don't
need to re-search for `netif_hdr` after the other two lines are
removed.
[start-write-doc]: https://doc.riot-os.org/group__net__gnrc__pktbuf.html#ga640418467294ae3d408c109ab27bd617
[pkt-structure]: https://doc.riot-os.org/group__net__gnrc__pkt.html#ga278e783e56a5ee6f1bd7b81077ed82a7
The `addr` parameter of the NIB's `_handle_dad()` function can come
from anywhere (e.g. in the fallback to classic SLAAC the destination
address of the IP header is used), so putting that pointer in a timer
is not a good idea. Instead we use the version of the address that is
stored within the interface.
`_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.
If the interface's link-layer doesn't use link-layer addresses it
obviously doesn't make sense to auto-configure an IPv6 address from it.
Moreover, I think the address `fe80::` is actual illegal, but I
couldn't find any references for it.
According to the documentation of `gnrc_ipv6_nib_get_next_hop_l2addr()`
`pkt` may be `NULL`. However, whenever that function sends an error
message (the methods for that require `orig_pkt` not to be NULL) `pkt`
is not checked, which may lead to failed assertions.
Not only does this leave open a risk to crash the node for the check
in `_compressible()` but also is the `tmp` check below getting confused
when `ptr` is `NULL`, since `gnrc_pktbuf_start_write()` returns `NULL`
in that case.
With newlib nano-specs the debug output without this change will be
6lo: dispatch 0hx ... is not supported
With this PR this will provide a correct output, e.g.
6lo: dispatch 0x3f ... is not supported
Currently the constructed NA for a delayed NA case is neither used nor
released nor does it get an IPv6 header to be used properly. This fixes
that case.
When working on the previous commit I was unsure if a
garbage-collectible entry should remain in the list, so I added this
comment so I don't have to wonder about this in the future ;-).
Fragment size calculation previously failed for devices that are able to
transmit bigger layer 2 PDUs that 802.15.4 devices. This commit fixes the issue.
The `_next_removable` list manages the cache-out of the neighbor cache.
However, when a neighbor cache entry is removed, it is not removed
from that list, which may lead to a segmentation fault when that list is
accessed, since the whole entry (including its list pointer) is zeroed
after removal.
With this change the entry is removed from that list accordingly before
the zeroing happens.
When either `gnrc_sixlowpan_iphc_nhc` or `gnrc_udp` is not compiled
in `_compressible()` never returns `true`. This causes the
`dispatch` snip in `gnrc_sixlowpan_iphc_send()` to be of length 0,
meaning `dispatch->data` is `NULL`, causing possible crashes when
trying to send IPv6 packets over 6LoWPAN without NHC or UDP.
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.
`gnrc_sixlowpan_frag` internally derives the offset value directly
from the fragment header, so for normal usage within GNRC this
assertion is redundant, but to make the tests of `rbuf_add` 100%
water-tide I added it.
Currently the loop just continues to run after a viable type is found.
In #10851 this lead to a crash of the tests, when the dependency of
`gnrc_sixlowpan` to `gnrc_ipv6` was removed.
When a new queue entry is tried to be allocated for a neighbor who's
address is currently tried to be resolved there was no error case
before. The packet that was tried to be put in the queue was thus not
released and stayed in the packet buffer for ever.
The function to infer the link-layer address length from the length of
a S/TLLAO is very dependent on the IPv6 over X specification and thus
should be grouped with the other IP over X functions.
While the recursion in `gnrc_sixlowpan_frag` shouldn't be infinite we
still should avoid using recursions in general (also to be able to
statically analyze stack usage). This unrolls the recursion.
When having a non-6LN interface and a 6LN interface (e.g. on a border
router) the assertion can hit when a Router Advertisement is received.
This makes the check an `if` statement rather than an assertion, to
account for that case.
Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
When issueing the sending of the next fragment the current version of
`gnrc_sixlowpan_frag` doesn't check if the queue is full. This leads to
leakage of the packet buffer, since when it is full, the package never
gets released.
This change adds a checks and error exits in case the queue is full.
Since the recursion into `gnrc_ipv6_demux()` was removed in
`gnrc_ipv6_ext`, `gnrc_ipv6.c` is the only user of this function,
so it can be made private. It was only made public so it can be used
from `gnrc_ipv6_ext`.
As `pkt` isn't pre-parsed the write-protection of *the whole* packet
(except the netif-header) comes for free, when this was done in the
receive routine of IPv6.
Since with #10233 we now assume IPv6 packets always to not be
pre-parsed, we can iterate over the extension headers by gradually
"eating" them away. This allows us to move the iteration over them
out of `gnrc_ipv6_ext_demux()` and into `gnrc_ipv6_demux()`.
By moving the iteration over all extension headers out of
`gnrc_ipv6_ext_demux()` we also can
1. simplify the extension header handling a lot, as it now
just a loop inside `gnrc_ipv6_demux()`,
2. remove the recursion to `gnrc_ipv6_demux()` within
`gnrc_ipv6_ext_demux()`.
Since the packet is now guaranteed to be preparsed, the currently
handled IPv6 header will always be in the first snip. Because of this
the packet parser can't get confused anymore which IPv6 header is the
one to be handled so we don't need to remove the more outer ones.
Because of this we can just use the normal packet dispatching (which is
already used by other `GNRC_NETTYPE_*`-known protocol numbers such as
UDP).
This also reverts d54ac38f84.
Though this change might seem more complicated, it has the benefit, that
after #9484 we don't have to assume that a received packet within IPv6's
receive function can be handed to the function pre-parsed, making that
function far less complicated (will be provided in a future PR).
Also this might give the forwarding via routing header a little
performance boost, as we now don't *receive* the packet first only to
forward it later-on.
While it is an edge case in our configuration it is technically
possible for a (6Lo) router not to maintain an address resolution state
machine. This fix allows for that with the `gnrc_ndp` module.
Check for:
- if it exists (critical error condition -- non-IPv6 headers should
not trigger these functions) => assert
- if it has a multicast source (that shouldn't really happen but
people might try weird stuff ;-)
- if it has an unspecified source (can't determine receiver of error
message => don't send it, don't build it)
Without this the first packet to a new link-local address will not be
delivered in non-6Lo environments, since the interface is not provided.
With this change, if an internet was provided to the address resolver it
will be stored within an allocated `gnrc_netif_hdr_t`.
At this point [IPv6 already striped](netif strip) the packet of its
netif header, so there is no risk that there will be to, in case it was
provided and the `netif` came from its existence.
`_decapsulate()` is called by callees of `_receive()` so the call to
the latter function within the first creates a recursion we don't want.
Using `gnrc_netapi` instead removes that and provides the added benefit
that other subscribers to IPv6 are also informed.
