[riot-commits] [RIOT-OS/RIOT] b82693: tests: add test case for pointer confusion

Martine Lenders noreply at github.com
Wed Jul 3 23:50:10 CEST 2019


  Branch: refs/heads/master
  Home:   https://github.com/RIOT-OS/RIOT
  Commit: b8269316e6f34d5514a4a4d13b4f2cdf6e59ec32
      https://github.com/RIOT-OS/RIOT/commit/b8269316e6f34d5514a4a4d13b4f2cdf6e59ec32
  Author: Martine S. Lenders <m.lenders at fu-berlin.de>
  Date:   2019-07-03 (Wed, 03 Jul 2019)

  Changed paths:
    A tests/gnrc_ipv6_fwd_w_sub/Makefile
    A tests/gnrc_ipv6_fwd_w_sub/README.md
    A tests/gnrc_ipv6_fwd_w_sub/common.h
    A tests/gnrc_ipv6_fwd_w_sub/main.c
    A tests/gnrc_ipv6_fwd_w_sub/mockup_netif.c
    A tests/gnrc_ipv6_fwd_w_sub/tests/01-run.py

  Log Message:
  -----------
  tests: add test case for pointer confusion

When subscribing to IPv6 packets on a router for sniffing, the NETIF
header is released prematurely, because of a wrong
`gnrc_pktbuf_start_write()` call. This test aims to reproduce this
error case.


  Commit: ea449f3f9e96d01de3f16c10f06ded16b15fc9ea
      https://github.com/RIOT-OS/RIOT/commit/ea449f3f9e96d01de3f16c10f06ded16b15fc9ea
  Author: Martine S. Lenders <m.lenders at fu-berlin.de>
  Date:   2019-07-03 (Wed, 03 Jul 2019)

  Changed paths:
    M sys/net/gnrc/network_layer/ipv6/gnrc_ipv6.c

  Log Message:
  -----------
  gnrc_ipv6: remove obsolete and harmful reception code

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


  Commit: 16f0751102005383477498f62f5a8345d544b16b
      https://github.com/RIOT-OS/RIOT/commit/16f0751102005383477498f62f5a8345d544b16b
  Author: Martine Lenders <m.lenders at fu-berlin.de>
  Date:   2019-07-03 (Wed, 03 Jul 2019)

  Changed paths:
    M sys/net/gnrc/network_layer/ipv6/gnrc_ipv6.c
    A tests/gnrc_ipv6_fwd_w_sub/Makefile
    A tests/gnrc_ipv6_fwd_w_sub/README.md
    A tests/gnrc_ipv6_fwd_w_sub/common.h
    A tests/gnrc_ipv6_fwd_w_sub/main.c
    A tests/gnrc_ipv6_fwd_w_sub/mockup_netif.c
    A tests/gnrc_ipv6_fwd_w_sub/tests/01-run.py

  Log Message:
  -----------
  Merge pull request #11745 from miri64/gnrc_ipv6/fix/rm-dangerous-dup

 gnrc_ipv6: remove obsolete and harmful reception code


Compare: https://github.com/RIOT-OS/RIOT/compare/0cc4c5091993...16f075110200


More information about the commits mailing list