[Linux-ATM-General] [PATCH 1/2 v2] atm: br2684: Improve encapsulation checks

[Pascal H. <pa...@pl...>]

- Check that the LLC header matches the expected payload type.
- Check that the PAD field is 0x00-00 also in LLC bridged mode.
- Check that the frame length in bridged mode is at least ETH_HLEN (14)
  without FCS or ETH_ZLEN + ETH_FCS_LEN (64) with FCS.
- Trim the trailing FCS field only if the PID is ethernet with FCS.
- Check the data length is not null before reading the IP header version
  field in VC-MUX routed mode.

Signed-off-by: Pascal Hambourg <pa...@pl...>
---
 net/atm/br2684.c |   73 ++++++++++++++++++++---------------
 1 file changed, 42 insertions(+), 31 deletions(-)

--- a/net/atm/br2684.c	2011-08-01 23:54:44.000000000 +0200
+++ b/net/atm/br2684.c	2011-08-12 21:29:22.000000000 +0200
@@ -404,41 +404,51 @@ static void br2684_push(struct atm_vcc *
 	atm_return(atmvcc, skb->truesize);
 	pr_debug("skb from brdev %p\n", brdev);
 	if (brvcc->encaps == e_llc) {
-
-		if (skb->len > 7 && skb->data[7] == 0x01)
-			__skb_trim(skb, skb->len - 4);
-
-		/* accept packets that have "ipv[46]" in the snap header */
-		if ((skb->len >= (sizeof(llc_oui_ipv4))) &&
-		    (memcmp(skb->data, llc_oui_ipv4,
-			    sizeof(llc_oui_ipv4) - BR2684_ETHERTYPE_LEN) == 0)) {
-			if (memcmp(skb->data + 6, ethertype_ipv6,
-				   sizeof(ethertype_ipv6)) == 0)
-				skb->protocol = htons(ETH_P_IPV6);
-			else if (memcmp(skb->data + 6, ethertype_ipv4,
-					sizeof(ethertype_ipv4)) == 0)
-				skb->protocol = htons(ETH_P_IP);
-			else
+		if (brdev->payload == p_routed) {
+			/* accept packets that have "ipv[46]" in the snap header */
+			if ((skb->len >= (sizeof(llc_oui_ipv4))) &&
+			    (memcmp(skb->data, llc_oui_ipv4,
+				    sizeof(llc_oui_ipv4) - BR2684_ETHERTYPE_LEN) == 0)) {
+				if (memcmp(skb->data + 6, ethertype_ipv6,
+					   sizeof(ethertype_ipv6)) == 0)
+					skb->protocol = htons(ETH_P_IPV6);
+				else if (memcmp(skb->data + 6, ethertype_ipv4,
+						sizeof(ethertype_ipv4)) == 0)
+					skb->protocol = htons(ETH_P_IP);
+				else
+					goto error;
+				skb_pull(skb, sizeof(llc_oui_ipv4));
+				skb_reset_network_header(skb);
+				skb->pkt_type = PACKET_HOST;
+			} else {
 				goto error;
-			skb_pull(skb, sizeof(llc_oui_ipv4));
-			skb_reset_network_header(skb);
-			skb->pkt_type = PACKET_HOST;
-		/*
-		 * Let us waste some time for checking the encapsulation.
-		 * Note, that only 7 char is checked so frames with a valid FCS
-		 * are also accepted (but FCS is not checked of course).
-		 */
-		} else if ((skb->len >= sizeof(llc_oui_pid_pad)) &&
-			   (memcmp(skb->data, llc_oui_pid_pad, 7) == 0)) {
-			skb_pull(skb, sizeof(llc_oui_pid_pad));
-			skb->protocol = eth_type_trans(skb, net_dev);
-		} else
-			goto error;
-
+			}
+		} else { /* p_bridged */
+			/*
+			 * Let us waste some time for checking the encapsulation.
+			 * Frames with a valid FCS
+			 * are also accepted (but FCS is not checked of course).
+			 */
+			if ((skb->len >= sizeof(llc_oui_pid_pad) + ETH_HLEN) &&
+			    (memcmp(skb->data, llc_oui_pid_pad, 7) == 0) &&
+			    (*((u16 *) (skb->data + 8)) == 0)) {
+				if (skb->data[7] == 0x01 &&
+				    skb->len >= sizeof(llc_oui_pid_pad) + ETH_ZLEN + ETH_FCS_LEN)
+					__skb_trim(skb, skb->len - ETH_FCS_LEN); /* trim FCS */
+				else if (skb->data[7] != 0x07)
+					goto error;
+				skb_pull(skb, sizeof(llc_oui_pid_pad));
+				skb->protocol = eth_type_trans(skb, net_dev);
+			} else {
+				goto error;
+			}
+		}
 	} else { /* e_vc */
 		if (brdev->payload == p_routed) {
 			struct iphdr *iph;

+			if (skb->len == 0)
+				goto error;
 			skb_reset_network_header(skb);
 			iph = ip_hdr(skb);
 			if (iph->version == 4)
@@ -450,7 +460,8 @@ static void br2684_push(struct atm_vcc *
 			skb->pkt_type = PACKET_HOST;
 		} else { /* p_bridged */
 			/* first 2 chars should be 0 */
-			if (*((u16 *) (skb->data)) != 0)
+			if (skb->len < BR2684_PAD_LEN + ETH_HLEN ||
+			    *((u16 *) (skb->data)) != 0)
 				goto error;
 			skb_pull(skb, BR2684_PAD_LEN);
 			skb->protocol = eth_type_trans(skb, net_dev);