x25: Handle undersized/fragmented skbs

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@gmail.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Andrew Hendry <andrew.hendry@gmail.com>
Cc: stable <stable@kernel.org>
Acked-by: Andrew Hendry <andrew.hendry@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

6 files changed, 87 insertions, 17 deletions

diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index a4bd1720e39b..aa567b09ea9a 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -91,7 +91,7 @@ int x25_parse_address_block(struct sk_buff *skb,
         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_buff *skb,
         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;
@@ -951,10 +951,10 @@ int x25_rx_call_request(struct sk_buff *skb, struct x25_neigh *nb,
          *
          *      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);
 
@@ -965,6 +965,13 @@ int x25_rx_call_request(struct sk_buff *skb, struct x25_neigh *nb,
                 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);
@@ -1172,6 +1179,9 @@ static int x25_sendmsg(struct kiocb *iocb, struct socket *sock,
          *      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);
         }
@@ -1250,7 +1260,9 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
         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;
@@ -1271,6 +1283,9 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
 
                 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);
 
                 /*
@@ -1291,10 +1306,12 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
                 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);
diff --git a/net/x25/x25_dev.c b/net/x25/x25_dev.c
index e547ca1578c3..fa2b41888bd9 100644
--- a/net/x25/x25_dev.c
+++ b/net/x25/x25_dev.c
@@ -32,6 +32,9 @@ static int x25_receive_data(struct sk_buff *skb, struct x25_neigh *nb)
         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_buff *skb, struct net_device *dev,
                 goto drop;
         }
 
+        if (!pskb_may_pull(skb, 1))
+                return 0;
+
         switch (skb->data[0]) {
 
         case X25_IFACE_DATA:
diff --git a/net/x25/x25_facilities.c b/net/x25/x25_facilities.c
index f77e4e75f914..36384a1fa9f2 100644
--- 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 *skb, struct x25_facilities *facilities,
         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:
diff --git a/net/x25/x25_in.c b/net/x25/x25_in.c
index 63488fd4885a..a49cd4ec551a 100644
--- a/net/x25/x25_in.c
+++ b/net/x25/x25_in.c
@@ -107,6 +107,8 @@ static int x25_state1_machine(struct sock *sk, struct sk_buff *skb, int frametyp
                 /*
                  *      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 sock *sk, struct sk_buff *skb, int frametyp
                         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 sock *sk, struct sk_buff *skb, int frametyp
                 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 sock *sk, struct sk_buff *skb, int frametyp
         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 sock *sk, struct sk_buff *skb, int frametyp
         }
 
         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 sock *sk, struct sk_buff *skb, int frametyp
                         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 sock *sk, struct sk_buff *skb, int frametyp
         }
 
         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 sock *sk, struct sk_buff *skb, int frametyp
  */
 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 sock *sk, struct sk_buff *skb, int frametyp
                         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 sock *sk, struct sk_buff *skb, int frametyp
         }
 
         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 */
diff --git a/net/x25/x25_link.c b/net/x25/x25_link.c
index 037958ff8eed..4acacf3c6617 100644
--- a/net/x25/x25_link.c
+++ b/net/x25/x25_link.c
@@ -90,6 +90,9 @@ void x25_link_control(struct sk_buff *skb, struct x25_neigh *nb,
                 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],
                        skb->data[5], skb->data[6]);
diff --git a/net/x25/x25_subr.c b/net/x25/x25_subr.c
index 24a342ebc7f5..5170d52bfd96 100644
--- a/net/x25/x25_subr.c
+++ b/net/x25/x25_subr.c
@@ -269,7 +269,11 @@ int x25_decode(struct sock *sk, struct sk_buff *skb, int *ns, int *nr, int *q,
                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;
 
@@ -294,6 +298,10 @@ int x25_decode(struct sock *sk, struct sk_buff *skb, int *ns, int *nr, int *q,
                 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];
                 }
@@ -308,6 +316,10 @@ int x25_decode(struct sock *sk, struct sk_buff *skb, int *ns, int *nr, int *q,
 
         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;