| Message ID | 1462218931-4573-2-git-send-email-thuth@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
On Mon, May 02, 2016 at 09:55:27PM +0200, Thomas Huth wrote: > The "struct neighbor *n" pointer in send_ipv6() can be NULL, e.g. > when we're sending to multicast addresses or to a server that sits > behind a router (since we do not have an entry in the neighbor cache > in this case). > However, the final code in send_ipv6() is always using n->eth_frame > to assemble the ethernet packet, and thus silently writes the data > into the low memory (which happens unnoticed because SLOF does not > use the MMU for memory protection). > This issue is now fixed by using a separate buffer for assembling > those ethernet packets instead. The block for using the router's > MAC address is also moved out of the block that is supposed to handle > the unicast transfers, so that we do not accidentially end up in the > neighbour solicitation code here (which also relies on n != NULL). > > Reported-by: Andrew Jones <drjones@redhat.com> > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > clients/net-snk/app/netlib/ipv6.c | 32 ++++++++++++++++---------------- > 1 file changed, 16 insertions(+), 16 deletions(-) Reviewed-by: Andrew Jones <drjones@redhat.com> > > diff --git a/clients/net-snk/app/netlib/ipv6.c b/clients/net-snk/app/netlib/ipv6.c > index dfa16b3..6c041d6 100644 > --- a/clients/net-snk/app/netlib/ipv6.c > +++ b/clients/net-snk/app/netlib/ipv6.c > @@ -488,12 +488,12 @@ static unsigned short ip6_checksum(struct ip6hdr *ip6h, unsigned short *packet, > */ > int send_ipv6(int fd, void* buffer, int len) > { > - struct neighbor *n; > struct ip6hdr *ip6h; > struct udphdr *udph; > struct icmp6hdr *icmp6h; > ip6_addr_t ip_dst; > uint8_t *mac_addr, mc_mac[6]; > + static uint8_t ethframe[ETH_MTU_SIZE]; > > mac_addr = null_mac; > > @@ -524,21 +524,20 @@ int send_ipv6(int fd, void* buffer, int len) > (unsigned short *) icmp6h, > ip6h->pl >> 1); > > - n = find_neighbor (&ip_dst); > - > - // If address is a multicast address, create a proper mac address > if (ip6_is_multicast (&ip_dst)) { > + /* If multicast, then create a proper multicast mac address */ > mac_addr = ip6_to_multicast_mac (&ip_dst, mc_mac); > - } > - else { > - // Check if the MAC address is already cached > - if (n) { > + } else if (!is_ip6addr_in_my_net(&ip_dst)) { > + /* If server is not in same subnet, user MAC of the router */ > + struct router *gw; > + gw = ipv6_get_default_router(&ip6h->src); > + mac_addr = gw ? gw->mac : null_mac; > + } else { > + /* Normal unicast, so use neighbor cache to look up MAC */ > + struct neighbor *n = find_neighbor (&ip_dst); > + if (n) { /* Already cached ? */ > if (memcmp(n->mac, null_mac, ETH_ALEN) != 0) > mac_addr = n->mac; /* found it */ > - } else if (!is_ip6addr_in_my_net(&ip_dst)) { > - struct router *gw; > - gw = ipv6_get_default_router(&ip6h->src); > - mac_addr = gw ? gw->mac : null_mac; > } else { > mac_addr = null_mac; > n = malloc(sizeof(struct neighbor)); > @@ -575,10 +574,11 @@ int send_ipv6(int fd, void* buffer, int len) > if (mac_addr == null_mac) > return -1; > > - fill_ethhdr (n->eth_frame, htons(ETHERTYPE_IPv6), get_mac_address(), > - mac_addr); > - memcpy (&(n->eth_frame[sizeof(struct ethhdr)]), buffer, len); > - return send_ether (fd, n->eth_frame, len + sizeof(struct ethhdr)); > + fill_ethhdr(ethframe, htons(ETHERTYPE_IPv6), get_mac_address(), > + mac_addr); > + memcpy(ðframe[sizeof(struct ethhdr)], buffer, len); > + > + return send_ether(fd, ethframe, len + sizeof(struct ethhdr)); > } > > static int check_colons(const char *str) > -- > 1.8.3.1 >
diff --git a/clients/net-snk/app/netlib/ipv6.c b/clients/net-snk/app/netlib/ipv6.c index dfa16b3..6c041d6 100644 --- a/clients/net-snk/app/netlib/ipv6.c +++ b/clients/net-snk/app/netlib/ipv6.c @@ -488,12 +488,12 @@ static unsigned short ip6_checksum(struct ip6hdr *ip6h, unsigned short *packet, */ int send_ipv6(int fd, void* buffer, int len) { - struct neighbor *n; struct ip6hdr *ip6h; struct udphdr *udph; struct icmp6hdr *icmp6h; ip6_addr_t ip_dst; uint8_t *mac_addr, mc_mac[6]; + static uint8_t ethframe[ETH_MTU_SIZE]; mac_addr = null_mac; @@ -524,21 +524,20 @@ int send_ipv6(int fd, void* buffer, int len) (unsigned short *) icmp6h, ip6h->pl >> 1); - n = find_neighbor (&ip_dst); - - // If address is a multicast address, create a proper mac address if (ip6_is_multicast (&ip_dst)) { + /* If multicast, then create a proper multicast mac address */ mac_addr = ip6_to_multicast_mac (&ip_dst, mc_mac); - } - else { - // Check if the MAC address is already cached - if (n) { + } else if (!is_ip6addr_in_my_net(&ip_dst)) { + /* If server is not in same subnet, user MAC of the router */ + struct router *gw; + gw = ipv6_get_default_router(&ip6h->src); + mac_addr = gw ? gw->mac : null_mac; + } else { + /* Normal unicast, so use neighbor cache to look up MAC */ + struct neighbor *n = find_neighbor (&ip_dst); + if (n) { /* Already cached ? */ if (memcmp(n->mac, null_mac, ETH_ALEN) != 0) mac_addr = n->mac; /* found it */ - } else if (!is_ip6addr_in_my_net(&ip_dst)) { - struct router *gw; - gw = ipv6_get_default_router(&ip6h->src); - mac_addr = gw ? gw->mac : null_mac; } else { mac_addr = null_mac; n = malloc(sizeof(struct neighbor)); @@ -575,10 +574,11 @@ int send_ipv6(int fd, void* buffer, int len) if (mac_addr == null_mac) return -1; - fill_ethhdr (n->eth_frame, htons(ETHERTYPE_IPv6), get_mac_address(), - mac_addr); - memcpy (&(n->eth_frame[sizeof(struct ethhdr)]), buffer, len); - return send_ether (fd, n->eth_frame, len + sizeof(struct ethhdr)); + fill_ethhdr(ethframe, htons(ETHERTYPE_IPv6), get_mac_address(), + mac_addr); + memcpy(ðframe[sizeof(struct ethhdr)], buffer, len); + + return send_ether(fd, ethframe, len + sizeof(struct ethhdr)); } static int check_colons(const char *str)
The "struct neighbor *n" pointer in send_ipv6() can be NULL, e.g. when we're sending to multicast addresses or to a server that sits behind a router (since we do not have an entry in the neighbor cache in this case). However, the final code in send_ipv6() is always using n->eth_frame to assemble the ethernet packet, and thus silently writes the data into the low memory (which happens unnoticed because SLOF does not use the MMU for memory protection). This issue is now fixed by using a separate buffer for assembling those ethernet packets instead. The block for using the router's MAC address is also moved out of the block that is supposed to handle the unicast transfers, so that we do not accidentially end up in the neighbour solicitation code here (which also relies on n != NULL). Reported-by: Andrew Jones <drjones@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com> --- clients/net-snk/app/netlib/ipv6.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-)