From patchwork Mon May 2 19:55:27 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Huth X-Patchwork-Id: 617672 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3qzFRM1f8Hz9t6B for ; Tue, 3 May 2016 05:55:59 +1000 (AEST) Received: from ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 3qzFRL71BJzDqfP for ; Tue, 3 May 2016 05:55:58 +1000 (AEST) X-Original-To: slof@lists.ozlabs.org Delivered-To: slof@lists.ozlabs.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3qzFQz41YDzDqfN for ; Tue, 3 May 2016 05:55:39 +1000 (AEST) Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E7706C04B32A; Mon, 2 May 2016 19:55:37 +0000 (UTC) Received: from thh440s.fritz.box (vpn1-4-248.ams2.redhat.com [10.36.4.248]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u42JtWRq032719; Mon, 2 May 2016 15:55:36 -0400 From: Thomas Huth To: slof@lists.ozlabs.org, aik@ozlabs.ru Date: Mon, 2 May 2016 21:55:27 +0200 Message-Id: <1462218931-4573-2-git-send-email-thuth@redhat.com> In-Reply-To: <1462218931-4573-1-git-send-email-thuth@redhat.com> References: <1462218931-4573-1-git-send-email-thuth@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 Subject: [SLOF] [PATCH 1/5] ipv6: Fix possible NULL-pointer dereference in send_ipv6() X-BeenThere: slof@lists.ozlabs.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: "Patches for https://github.com/aik/SLOF" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: drjones@redhat.com MIME-Version: 1.0 Errors-To: slof-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "SLOF" 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 Signed-off-by: Thomas Huth Reviewed-by: Andrew Jones --- clients/net-snk/app/netlib/ipv6.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) 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)