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

sys/linux: ingress UDP coverage #1594

Open
dvyukov opened this issue Feb 10, 2020 · 3 comments
Open

sys/linux: ingress UDP coverage #1594

dvyukov opened this issue Feb 10, 2020 · 3 comments

Comments

@dvyukov
Copy link
Collaborator

dvyukov commented Feb 10, 2020

See discussion on #1590 for some context.
It's important to cover network ingress paths, but it's unclear if we are doing good job there.
For example, we don't seem to cover even udp_unicast_rcv_skb in net/ipv4/udp.c which is required to cover both UDP stack and, for example, wireguard (which should be reached via udp_queue_rcv_one_skb->encap_rcv).
Maybe these parts are only executed in background threads? But most likely there is something else. Either way we need to understand what exactly is it and if it's async paths, if it's possible to cover them with the remote coverage facility.
Another observation is that IFF_NAPI_FRAGS for tun diverges execution to completely different paths (something called gso). But it's unclear if it affects our ability to cover udp stack or not. Without IFF_NAPI_FRAGS I seem to get more coverage overall, but still no coverage of udp_unicast_rcv_skb.
If we remove IFF_NAPI_FRAGS, most likely we also need to remove IFF_NAPI (this actually affects our current fallback for failed IFF_NAPI_FRAGS). IFF_NAPI without IFF_NAPI_FRAGS leads to async receive path, which is bad for us.

@dvyukov
Copy link
Collaborator Author

dvyukov commented Feb 11, 2020

The referenced commit contains few tests I used. The one with syz_emit_ethernet does not work yet.

@dvyukov
Copy link
Collaborator Author

dvyukov commented Feb 17, 2020

Even without IFF_NAPI_FRAGS, all packets we inject get rejected on the following check:

static struct sk_buff *ip_rcv_core(struct sk_buff *skb, struct net *net)
{
	const struct iphdr *iph;
	u32 len;

	/* When the interface is in promisc. mode, drop all the crap
	 * that it receives, do not try to analyse it.
	 */
	if (skb->pkt_type == PACKET_OTHERHOST)
		goto drop;

Somehow we need to get something other than PACKET_OTHERHOST...
Why is it dropping all remote packets?...
How do remote packets get into stack then?...

@dvyukov
Copy link
Collaborator Author

dvyukov commented Feb 18, 2020

I've managed to create a packet that reaches wg_receive, that is:

syz_emit_ethernet(AUTO, &AUTO={@local, @empty, @void, {@ipv4={AUTO, @udp={{AUTO, AUTO, 0x0, 0x0, AUTO, 0x0, 0x0, 0x0, AUTO, 0x0, @empty, @empty, {[]}}, {0x0, 0x4e22, AUTO, 0x0, [], ""/10}}}}}, 0x0)

Had to enumerate all possible combinations of local/remote mac,
local/report ip, local/remote port.

However, this is only without IFF_NAPI_FRAGS. With IFF_NAPI_FRAGS it
reaches udp_gro_receive, but does not get past:

if (!sk || NAPI_GRO_CB(skb)->encap_mark ||
    (skb->ip_summed != CHECKSUM_PARTIAL &&
     NAPI_GRO_CB(skb)->csum_cnt == 0 &&
     !NAPI_GRO_CB(skb)->csum_valid) ||
    !udp_sk(sk)->gro_receive)
    goto out;

dvyukov added a commit that referenced this issue Feb 18, 2020
dvyukov added a commit that referenced this issue Feb 18, 2020
Fix the packet injection in udp test.
Now we know how to do it!
And without IFF_NAPI_FRAGS it actually reaches the socket.

Update #1594
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants