Patch "x25: Handle undersized/fragmented skbs" has been added to the 3.0-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    x25: Handle undersized/fragmented skbs

to the 3.0-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     x25-handle-undersized-fragmented-skbs.patch
and it can be found in the queue-3.0 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.


>From jslaby@xxxxxxx  Mon Apr  1 16:35:58 2013
From: Jiri Slaby <jslaby@xxxxxxx>
Date: Tue, 19 Mar 2013 12:36:48 +0100
Subject: x25: Handle undersized/fragmented skbs
To: gregkh@xxxxxxxxxxxxxxxxxxx
Cc: jirislaby@xxxxxxxxx, stable@xxxxxxxxxxxxxxx, Matthew Daley <mattjd@xxxxxxxxx>, Eric Dumazet <eric.dumazet@xxxxxxxxx>, Andrew Hendry <andrew.hendry@xxxxxxxxx>, stable <stable@xxxxxxxxxx>, "David S. Miller" <davem@xxxxxxxxxxxxx>, Jiri Slaby <jslaby@xxxxxxx>
Message-ID: <1363693019-14812-3-git-send-email-jslaby@xxxxxxx>


From: Matthew Daley <mattjd@xxxxxxxxx>

commit cb101ed2c3c7c0224d16953fe77bfb9d6c2cb9df upstream.

There are multiple locations in the X.25 packet layer where a skb is
assumed to be of at least a certain size and that all its data is
currently available at skb->data.  These assumptions are not checked,
hence buffer overreads may occur.  Use pskb_may_pull to check these
minimal size assumptions and ensure that data is available at skb->data
when necessary, as well as use skb_copy_bits where needed.

Signed-off-by: Matthew Daley <mattjd@xxxxxxxxx>
Cc: Eric Dumazet <eric.dumazet@xxxxxxxxx>
Cc: Andrew Hendry <andrew.hendry@xxxxxxxxx>
Acked-by: Andrew Hendry <andrew.hendry@xxxxxxxxx>
Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
Signed-off-by: Jiri Slaby <jslaby@xxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
 net/x25/af_x25.c         |   31 ++++++++++++++++++++++++-------
 net/x25/x25_dev.c        |    6 ++++++
 net/x25/x25_facilities.c |   10 ++++++----
 net/x25/x25_in.c         |   40 +++++++++++++++++++++++++++++++++++-----
 net/x25/x25_link.c       |    3 +++
 net/x25/x25_subr.c       |   14 +++++++++++++-
 6 files changed, 87 insertions(+), 17 deletions(-)

--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -91,7 +91,7 @@ int x25_parse_address_block(struct sk_bu
 	int needed;
 	int rc;
 
-	if (skb->len < 1) {
+	if (!pskb_may_pull(skb, 1)) {
 		/* packet has no address block */
 		rc = 0;
 		goto empty;
@@ -100,7 +100,7 @@ int x25_parse_address_block(struct sk_bu
 	len = *skb->data;
 	needed = 1 + (len >> 4) + (len & 0x0f);
 
-	if (skb->len < needed) {
+	if (!pskb_may_pull(skb, needed)) {
 		/* packet is too short to hold the addresses it claims
 		   to hold */
 		rc = -1;
@@ -952,10 +952,10 @@ int x25_rx_call_request(struct sk_buff *
 	 *
 	 *	Facilities length is mandatory in call request packets
 	 */
-	if (skb->len < 1)
+	if (!pskb_may_pull(skb, 1))
 		goto out_clear_request;
 	len = skb->data[0] + 1;
-	if (skb->len < len)
+	if (!pskb_may_pull(skb, len))
 		goto out_clear_request;
 	skb_pull(skb,len);
 
@@ -966,6 +966,13 @@ int x25_rx_call_request(struct sk_buff *
 		goto out_clear_request;
 
 	/*
+	 *	Get all the call user data so it can be used in
+	 *	x25_find_listener and skb_copy_from_linear_data up ahead.
+	 */
+	if (!pskb_may_pull(skb, skb->len))
+		goto out_clear_request;
+
+	/*
 	 *	Find a listener for the particular address/cud pair.
 	 */
 	sk = x25_find_listener(&source_addr,skb);
@@ -1173,6 +1180,9 @@ static int x25_sendmsg(struct kiocb *ioc
 	 *	byte of the user data is the logical value of the Q Bit.
 	 */
 	if (test_bit(X25_Q_BIT_FLAG, &x25->flags)) {
+		if (!pskb_may_pull(skb, 1))
+			goto out_kfree_skb;
+
 		qbit = skb->data[0];
 		skb_pull(skb, 1);
 	}
@@ -1251,7 +1261,9 @@ static int x25_recvmsg(struct kiocb *ioc
 	struct x25_sock *x25 = x25_sk(sk);
 	struct sockaddr_x25 *sx25 = (struct sockaddr_x25 *)msg->msg_name;
 	size_t copied;
-	int qbit;
+	int qbit, header_len = x25->neighbour->extended ?
+		X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
+
 	struct sk_buff *skb;
 	unsigned char *asmptr;
 	int rc = -ENOTCONN;
@@ -1272,6 +1284,9 @@ static int x25_recvmsg(struct kiocb *ioc
 
 		skb = skb_dequeue(&x25->interrupt_in_queue);
 
+		if (!pskb_may_pull(skb, X25_STD_MIN_LEN))
+			goto out_free_dgram;
+
 		skb_pull(skb, X25_STD_MIN_LEN);
 
 		/*
@@ -1292,10 +1307,12 @@ static int x25_recvmsg(struct kiocb *ioc
 		if (!skb)
 			goto out;
 
+		if (!pskb_may_pull(skb, header_len))
+			goto out_free_dgram;
+
 		qbit = (skb->data[0] & X25_Q_BIT) == X25_Q_BIT;
 
-		skb_pull(skb, x25->neighbour->extended ?
-				X25_EXT_MIN_LEN : X25_STD_MIN_LEN);
+		skb_pull(skb, header_len);
 
 		if (test_bit(X25_Q_BIT_FLAG, &x25->flags)) {
 			asmptr  = skb_push(skb, 1);
--- a/net/x25/x25_dev.c
+++ b/net/x25/x25_dev.c
@@ -32,6 +32,9 @@ static int x25_receive_data(struct sk_bu
 	unsigned short frametype;
 	unsigned int lci;
 
+	if (!pskb_may_pull(skb, X25_STD_MIN_LEN))
+		return 0;
+
 	frametype = skb->data[2];
 	lci = ((skb->data[0] << 8) & 0xF00) + ((skb->data[1] << 0) & 0x0FF);
 
@@ -115,6 +118,9 @@ int x25_lapb_receive_frame(struct sk_buf
 		goto drop;
 	}
 
+	if (!pskb_may_pull(skb, 1))
+		return 0;
+
 	switch (skb->data[0]) {
 
 	case X25_IFACE_DATA:
--- a/net/x25/x25_facilities.c
+++ b/net/x25/x25_facilities.c
@@ -44,7 +44,7 @@
 int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
 		struct x25_dte_facilities *dte_facs, unsigned long *vc_fac_mask)
 {
-	unsigned char *p = skb->data;
+	unsigned char *p;
 	unsigned int len;
 
 	*vc_fac_mask = 0;
@@ -60,14 +60,16 @@ int x25_parse_facilities(struct sk_buff
 	memset(dte_facs->called_ae, '\0', sizeof(dte_facs->called_ae));
 	memset(dte_facs->calling_ae, '\0', sizeof(dte_facs->calling_ae));
 
-	if (skb->len < 1)
+	if (!pskb_may_pull(skb, 1))
 		return 0;
 
-	len = *p++;
+	len = skb->data[0];
 
-	if (len >= skb->len)
+	if (!pskb_may_pull(skb, 1 + len))
 		return -1;
 
+	p = skb->data + 1;
+
 	while (len > 0) {
 		switch (*p & X25_FAC_CLASS_MASK) {
 		case X25_FAC_CLASS_A:
--- a/net/x25/x25_in.c
+++ b/net/x25/x25_in.c
@@ -107,6 +107,8 @@ static int x25_state1_machine(struct soc
 			/*
 			 *	Parse the data in the frame.
 			 */
+			if (!pskb_may_pull(skb, X25_STD_MIN_LEN))
+				goto out_clear;
 			skb_pull(skb, X25_STD_MIN_LEN);
 
 			len = x25_parse_address_block(skb, &source_addr,
@@ -130,9 +132,8 @@ static int x25_state1_machine(struct soc
 				if (skb->len > X25_MAX_CUD_LEN)
 					goto out_clear;
 
-				skb_copy_from_linear_data(skb,
-					      x25->calluserdata.cuddata,
-					      skb->len);
+				skb_copy_bits(skb, 0, x25->calluserdata.cuddata,
+					skb->len);
 				x25->calluserdata.cudlength = skb->len;
 			}
 			if (!sock_flag(sk, SOCK_DEAD))
@@ -140,6 +141,9 @@ static int x25_state1_machine(struct soc
 			break;
 		}
 		case X25_CLEAR_REQUEST:
+			if (!pskb_may_pull(skb, X25_STD_MIN_LEN + 2))
+				goto out_clear;
+
 			x25_write_internal(sk, X25_CLEAR_CONFIRMATION);
 			x25_disconnect(sk, ECONNREFUSED, skb->data[3], skb->data[4]);
 			break;
@@ -167,6 +171,9 @@ static int x25_state2_machine(struct soc
 	switch (frametype) {
 
 		case X25_CLEAR_REQUEST:
+			if (!pskb_may_pull(skb, X25_STD_MIN_LEN + 2))
+				goto out_clear;
+
 			x25_write_internal(sk, X25_CLEAR_CONFIRMATION);
 			x25_disconnect(sk, 0, skb->data[3], skb->data[4]);
 			break;
@@ -180,6 +187,11 @@ static int x25_state2_machine(struct soc
 	}
 
 	return 0;
+
+out_clear:
+	x25_write_internal(sk, X25_CLEAR_REQUEST);
+	x25_start_t23timer(sk);
+	return 0;
 }
 
 /*
@@ -209,6 +221,9 @@ static int x25_state3_machine(struct soc
 			break;
 
 		case X25_CLEAR_REQUEST:
+			if (!pskb_may_pull(skb, X25_STD_MIN_LEN + 2))
+				goto out_clear;
+
 			x25_write_internal(sk, X25_CLEAR_CONFIRMATION);
 			x25_disconnect(sk, 0, skb->data[3], skb->data[4]);
 			break;
@@ -307,6 +322,12 @@ static int x25_state3_machine(struct soc
 	}
 
 	return queued;
+
+out_clear:
+	x25_write_internal(sk, X25_CLEAR_REQUEST);
+	x25->state = X25_STATE_2;
+	x25_start_t23timer(sk);
+	return 0;
 }
 
 /*
@@ -316,13 +337,13 @@ static int x25_state3_machine(struct soc
  */
 static int x25_state4_machine(struct sock *sk, struct sk_buff *skb, int frametype)
 {
+	struct x25_sock *x25 = x25_sk(sk);
+
 	switch (frametype) {
 
 		case X25_RESET_REQUEST:
 			x25_write_internal(sk, X25_RESET_CONFIRMATION);
 		case X25_RESET_CONFIRMATION: {
-			struct x25_sock *x25 = x25_sk(sk);
-
 			x25_stop_timer(sk);
 			x25->condition = 0x00;
 			x25->va        = 0;
@@ -334,6 +355,9 @@ static int x25_state4_machine(struct soc
 			break;
 		}
 		case X25_CLEAR_REQUEST:
+			if (!pskb_may_pull(skb, X25_STD_MIN_LEN + 2))
+				goto out_clear;
+
 			x25_write_internal(sk, X25_CLEAR_CONFIRMATION);
 			x25_disconnect(sk, 0, skb->data[3], skb->data[4]);
 			break;
@@ -343,6 +367,12 @@ static int x25_state4_machine(struct soc
 	}
 
 	return 0;
+
+out_clear:
+	x25_write_internal(sk, X25_CLEAR_REQUEST);
+	x25->state = X25_STATE_2;
+	x25_start_t23timer(sk);
+	return 0;
 }
 
 /* Higher level upcall for a LAPB frame */
--- a/net/x25/x25_link.c
+++ b/net/x25/x25_link.c
@@ -90,6 +90,9 @@ void x25_link_control(struct sk_buff *sk
 			break;
 
 		case X25_DIAGNOSTIC:
+			if (!pskb_may_pull(skb, X25_STD_MIN_LEN + 4))
+				break;
+
 			printk(KERN_WARNING "x25: diagnostic #%d - "
 			       "%02X %02X %02X\n",
 			       skb->data[3], skb->data[4],
--- a/net/x25/x25_subr.c
+++ b/net/x25/x25_subr.c
@@ -271,7 +271,11 @@ int x25_decode(struct sock *sk, struct s
 	       int *d, int *m)
 {
 	struct x25_sock *x25 = x25_sk(sk);
-	unsigned char *frame = skb->data;
+	unsigned char *frame;
+
+	if (!pskb_may_pull(skb, X25_STD_MIN_LEN))
+		return X25_ILLEGAL;
+	frame = skb->data;
 
 	*ns = *nr = *q = *d = *m = 0;
 
@@ -296,6 +300,10 @@ int x25_decode(struct sock *sk, struct s
 		if (frame[2] == X25_RR  ||
 		    frame[2] == X25_RNR ||
 		    frame[2] == X25_REJ) {
+			if (!pskb_may_pull(skb, X25_EXT_MIN_LEN))
+				return X25_ILLEGAL;
+			frame = skb->data;
+
 			*nr = (frame[3] >> 1) & 0x7F;
 			return frame[2];
 		}
@@ -310,6 +318,10 @@ int x25_decode(struct sock *sk, struct s
 
 	if (x25->neighbour->extended) {
 		if ((frame[2] & 0x01) == X25_DATA) {
+			if (!pskb_may_pull(skb, X25_EXT_MIN_LEN))
+				return X25_ILLEGAL;
+			frame = skb->data;
+
 			*q  = (frame[0] & X25_Q_BIT) == X25_Q_BIT;
 			*d  = (frame[0] & X25_D_BIT) == X25_D_BIT;
 			*m  = (frame[3] & X25_EXT_M_BIT) == X25_EXT_M_BIT;


Patches currently in stable-queue which might be from jslaby@xxxxxxx are

queue-3.0/kvm-x86-prevent-starting-pit-timers-in-the-absence-of-irqchip-support.patch
queue-3.0/kvm-x86-invalid-opcode-oops-on-set_sregs-with-osxsave-bit-set-cve-2012-4461.patch
queue-3.0/mm-hotplug-correctly-add-new-zone-to-all-other-nodes-zone-lists.patch
queue-3.0/macvtap-zerocopy-validate-vectors-before-building-skb.patch
queue-3.0/x25-validate-incoming-call-user-data-lengths.patch
queue-3.0/batman-adv-bat_socket_read-missing-checks.patch
queue-3.0/kvm-fix-buffer-overflow-in-kvm_set_irq.patch
queue-3.0/x25-handle-undersized-fragmented-skbs.patch
queue-3.0/nfsv4-include-bitmap-in-nfsv4-get-acl-data.patch
queue-3.0/nfs-nfs_getaclargs.acl_len-is-a-size_t.patch
queue-3.0/nfsv4-fix-an-oops-in-the-nfsv4-getacl-code.patch
queue-3.0/kvm-clean-up-error-handling-during-vcpu-creation.patch
queue-3.0/kvm-ensure-all-vcpus-are-consistent-with-in-kernel-irqchip-settings.patch
queue-3.0/batman-adv-only-write-requested-number-of-byte-to-user-buffer.patch
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]