From 11a297b557aba407c8d1d95eff1b86b8c5ea7d5d Mon Sep 17 00:00:00 2001 From: Martin Stein Date: Wed, 18 Apr 2018 19:38:43 +0200 Subject: [PATCH] net: consider tail of Ethernet frame The Ethernet payload may be followed by padding of variable length and the FCS (Frame Check Sequence). Thus, we should consider the value "Ethernet-frame size minus Ethernet-header size" to be only the maximum size of the encapsulated IP packet. But until now, we considered it to be also the actual size of the encapsulated IP packet. This commit fixes the problem for all affected components of the Genode base-repository. Fixes #2775 --- repos/os/include/net/ipv4.h | 2 ++ repos/os/src/app/ping/main.cc | 5 +++-- repos/os/src/lib/net/ipv4.cc | 7 +++++++ repos/os/src/server/nic_bridge/component.cc | 20 +++++++++++-------- repos/os/src/server/nic_bridge/nic.cc | 15 ++++++++------ repos/os/src/server/nic_router/dhcp_client.cc | 13 ++++++------ repos/os/src/server/nic_router/interface.cc | 12 ++++++----- 7 files changed, 47 insertions(+), 27 deletions(-) diff --git a/repos/os/include/net/ipv4.h b/repos/os/include/net/ipv4.h index 0bfbccfb0..dc61a7964 100644 --- a/repos/os/include/net/ipv4.h +++ b/repos/os/include/net/ipv4.h @@ -151,6 +151,8 @@ class Net::Ipv4_packet return *Genode::construct_at(_data); } + Genode::size_t size(Genode::size_t max_size) const; + /*************** ** Accessors ** diff --git a/repos/os/src/app/ping/main.cc b/repos/os/src/app/ping/main.cc index e06a50c35..5b6194364 100644 --- a/repos/os/src/app/ping/main.cc +++ b/repos/os/src/app/ping/main.cc @@ -199,8 +199,9 @@ void Main::_handle_ip(Ethernet_frame ð, size_t const eth_size) { /* drop packet if IP does not target us */ - size_t const ip_size = eth_size - sizeof(Ethernet_frame); - Ipv4_packet &ip = eth.data(ip_size); + size_t const ip_max_size = eth_size - sizeof(Ethernet_frame); + Ipv4_packet &ip = eth.data(ip_max_size); + size_t const ip_size = ip.size(ip_max_size); if (ip.dst() != _src_ip && ip.dst() != Ipv4_packet::broadcast()) { diff --git a/repos/os/src/lib/net/ipv4.cc b/repos/os/src/lib/net/ipv4.cc index 9d0fc432b..34615641a 100644 --- a/repos/os/src/lib/net/ipv4.cc +++ b/repos/os/src/lib/net/ipv4.cc @@ -146,3 +146,10 @@ bool Ipv4_packet::checksum_error() const { return internet_checksum((uint16_t *)this, sizeof(Ipv4_packet)); } + + +size_t Ipv4_packet::size(size_t max_size) const +{ + size_t const stated_size = total_length(); + return stated_size < max_size ? stated_size : max_size; +} diff --git a/repos/os/src/server/nic_bridge/component.cc b/repos/os/src/server/nic_bridge/component.cc index f637d030f..e3df0be11 100644 --- a/repos/os/src/server/nic_bridge/component.cc +++ b/repos/os/src/server/nic_bridge/component.cc @@ -19,6 +19,8 @@ #include using namespace Net; +using namespace Genode; + bool Session_component::handle_arp(Ethernet_frame *eth, Genode::size_t size) { @@ -50,17 +52,19 @@ bool Session_component::handle_arp(Ethernet_frame *eth, Genode::size_t size) bool Session_component::handle_ip(Ethernet_frame *eth, Genode::size_t size) { - Ipv4_packet &ip = eth->data(size - sizeof(Ethernet_frame)); + size_t const ip_max_size = size - sizeof(Ethernet_frame); + Ipv4_packet &ip = eth->data(ip_max_size); + size_t const ip_size = ip.size(ip_max_size); + if (ip.protocol() == Ipv4_packet::Protocol::UDP) { - if (ip.protocol() == Ipv4_packet::Protocol::UDP) - { - Udp_packet &udp = ip.data(size - sizeof(Ethernet_frame) - - sizeof(Ipv4_packet)); + size_t const udp_size = ip_size - sizeof(Ipv4_packet); + Udp_packet &udp = ip.data(udp_size); if (Dhcp_packet::is_dhcp(&udp)) { - Dhcp_packet &dhcp = udp.data(size - sizeof(Ethernet_frame) - - sizeof(Ipv4_packet) - - sizeof(Udp_packet)); + + size_t const dhcp_size = udp_size - sizeof(Udp_packet); + Dhcp_packet &dhcp = udp.data(dhcp_size); if (dhcp.op() == Dhcp_packet::REQUEST) { + dhcp.broadcast(true); udp.update_checksum(ip.src(), ip.dst()); } diff --git a/repos/os/src/server/nic_bridge/nic.cc b/repos/os/src/server/nic_bridge/nic.cc index d5e8528a3..c5e5a1762 100644 --- a/repos/os/src/server/nic_bridge/nic.cc +++ b/repos/os/src/server/nic_bridge/nic.cc @@ -21,6 +21,7 @@ #include using namespace Net; +using namespace Genode; bool Net::Nic::handle_arp(Ethernet_frame *eth, Genode::size_t size) { @@ -65,19 +66,21 @@ bool Net::Nic::handle_arp(Ethernet_frame *eth, Genode::size_t size) { bool Net::Nic::handle_ip(Ethernet_frame *eth, Genode::size_t size) { - Ipv4_packet &ip = eth->data(size - sizeof(Ethernet_frame)); + + size_t const ip_max_size = size - sizeof(Ethernet_frame); + Ipv4_packet &ip = eth->data(ip_max_size); + size_t const ip_size = ip.size(ip_max_size); /* is it an UDP packet ? */ if (ip.protocol() == Ipv4_packet::Protocol::UDP) { - Udp_packet &udp = ip.data(size - sizeof(Ethernet_frame) - - sizeof(Ipv4_packet)); + size_t const udp_size = ip_size - sizeof(Ipv4_packet); + Udp_packet &udp = ip.data(udp_size); /* is it a DHCP packet ? */ if (Dhcp_packet::is_dhcp(&udp)) { - Dhcp_packet &dhcp = udp.data(size - sizeof(Ethernet_frame) - - sizeof(Ipv4_packet) - - sizeof(Udp_packet)); + size_t const dhcp_size = udp_size - sizeof(Udp_packet); + Dhcp_packet &dhcp = udp.data(dhcp_size); /* check for DHCP ACKs containing new client ips */ if (dhcp.op() == Dhcp_packet::REPLY) { diff --git a/repos/os/src/server/nic_router/dhcp_client.cc b/repos/os/src/server/nic_router/dhcp_client.cc index 4926ad0d8..6ddf3a6fa 100644 --- a/repos/os/src/server/nic_router/dhcp_client.cc +++ b/repos/os/src/server/nic_router/dhcp_client.cc @@ -119,20 +119,21 @@ void Dhcp_client::handle_ip(Ethernet_frame ð, size_t eth_size) { throw Drop_packet_inform("DHCP client expects Ethernet targeting the router"); } - Ipv4_packet &ip = eth.data(eth_size - sizeof(Ethernet_frame)); + size_t const ip_max_size = eth_size - sizeof(Ethernet_frame); + Ipv4_packet &ip = eth.data(ip_max_size); + size_t const ip_size = ip.size(ip_max_size); if (ip.protocol() != Ipv4_packet::Protocol::UDP) { throw Drop_packet_inform("DHCP client expects UDP packet"); } - Udp_packet &udp = ip.data(eth_size - sizeof(Ethernet_frame) - - sizeof(Ipv4_packet)); + size_t const udp_size = ip_size - sizeof(Ipv4_packet); + Udp_packet &udp = ip.data(udp_size); if (!Dhcp_packet::is_dhcp(&udp)) { throw Drop_packet_inform("DHCP client expects DHCP packet"); } - Dhcp_packet &dhcp = udp.data(eth_size - sizeof(Ethernet_frame) - - sizeof(Ipv4_packet) - - sizeof(Udp_packet)); + size_t const dhcp_size = udp_size - sizeof(Udp_packet); + Dhcp_packet &dhcp = udp.data(dhcp_size); if (dhcp.op() != Dhcp_packet::REPLY) { throw Drop_packet_inform("DHCP client expects DHCP reply"); diff --git a/repos/os/src/server/nic_router/interface.cc b/repos/os/src/server/nic_router/interface.cc index b4ad0d848..a65be0c75 100644 --- a/repos/os/src/server/nic_router/interface.cc +++ b/repos/os/src/server/nic_router/interface.cc @@ -892,7 +892,9 @@ void Interface::_handle_ip(Ethernet_frame ð, Domain &local_domain) { /* read packet information */ - Ipv4_packet &ip = eth.data(eth_size - sizeof(Ethernet_frame)); + size_t const ip_max_size = eth_size - sizeof(Ethernet_frame); + Ipv4_packet &ip = eth.data(ip_max_size); + size_t const ip_size = ip.size(ip_max_size); Ipv4_address_prefix const &local_intf = local_domain.ip_config().interface; /* try handling subnet-local IP packets */ @@ -910,18 +912,18 @@ void Interface::_handle_ip(Ethernet_frame ð, /* try to route via transport layer rules */ try { L3_protocol const prot = ip.protocol(); - size_t const prot_size = ip.total_length() - ip.header_length() * 4; + size_t const prot_size = ip_size - sizeof(Ipv4_packet); void *const prot_base = _prot_base(prot, prot_size, ip); /* try handling DHCP requests before trying any routing */ if (prot == L3_protocol::UDP) { - Udp_packet &udp = ip.data(eth_size - sizeof(Ipv4_packet)); + Udp_packet &udp = ip.data(prot_size); if (Dhcp_packet::is_dhcp(&udp)) { /* get DHCP packet */ - Dhcp_packet &dhcp = udp.data(eth_size - sizeof(Ipv4_packet) - - sizeof(Udp_packet)); + size_t const dhcp_size = prot_size - sizeof(Udp_packet); + Dhcp_packet &dhcp = udp.data(dhcp_size); if (dhcp.op() == Dhcp_packet::REQUEST) { try {