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

net: ip: net_pkt_pull(): packet corruption when using CONFIG_NET_BUF_DATA_SIZE larger than 256 #22307

Closed
a8961713 opened this issue Jan 29, 2020 · 2 comments · Fixed by #22335
Assignees
Labels
area: Networking bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug

Comments

@a8961713
Copy link
Contributor

Describe the bug
CONFIG_NET_BUF_DATA_SIZE has a default of 128.
When using CONFIG_NET_BUF_FIXED_DATA_SIZE=y and CONFIG_NET_BUF_DATA_SIZE > 256 (e.g. 512), net_pkt_pull() corrupts the packet, as it calculates the lengths using u8_t.

This can be reproduced when using ipv6 with packet fragmentation. The packet reassembling code uses net_pkt_pull() on received packets.
You can send large ipv6 pings (e.g. ping6 -s 2000 192.0.2.2) to see the corrupted data

Impact
This is a showstopper when working with fragmented IPV6 packets.

Environment (please complete the following information):

  • OS: Linux
  • Toolchain: Zephyr SDK
  • Version: current master

Additional context

The solution is to change the type of left, rem from u8_t to size_t:

diff --git a/subsys/net/ip/net_pkt.c b/subsys/net/ip/net_pkt.c
index dfd2596c8d..8ceb13ffea 100644
--- a/subsys/net/ip/net_pkt.c
+++ b/subsys/net/ip/net_pkt.c
@@ -1848,7 +1848,7 @@ int net_pkt_pull(struct net_pkt *pkt, size_t length)
        net_pkt_cursor_backup(pkt, &backup);
 
        while (length) {
-               u8_t left, rem;
+               size_t left, rem;
 
                pkt_cursor_advance(pkt, false);
@a8961713 a8961713 added the bug The issue is a bug, or the PR is fixing a bug label Jan 29, 2020
@tbursztyka
Copy link
Collaborator

Could you send a PR?

When you do have a solution already, no need to open an issue first: you could describe what the commit is fixing in its message

@a8961713
Copy link
Contributor Author

OK, first thing tomorrow...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants