From: Matthew Daley Date: Tue, 19 Mar 2013 11:36:48 +0000 (+0100) Subject: x25: Handle undersized/fragmented skbs X-Git-Tag: v3.0.72~38 X-Git-Url: https://git.karo-electronics.de/?a=commitdiff_plain;h=7f3ea0c12493c9ff38a13a89bcf08846b50c1f1c;p=karo-tx-linux.git x25: Handle undersized/fragmented skbs 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 Cc: Eric Dumazet Cc: Andrew Hendry Acked-by: Andrew Hendry Signed-off-by: David S. Miller Signed-off-by: Jiri Slaby Signed-off-by: Greg Kroah-Hartman --- diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c index 8c0346f00e68..fb37356e41ec 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; @@ -952,10 +952,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, if (skb->len > X25_MAX_CUD_LEN) 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. */ @@ -1173,6 +1180,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); } @@ -1251,7 +1261,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; @@ -1272,6 +1284,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); /* @@ -1292,10 +1307,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 9005f6daeab5..60749c5f4038 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 b1180cc28669..36ab913b3866 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 21306928d47f..0a9e0741d24f 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], diff --git a/net/x25/x25_subr.c b/net/x25/x25_subr.c index dc20cf12f39b..faf98d8ebe21 100644 --- a/net/x25/x25_subr.c +++ b/net/x25/x25_subr.c @@ -271,7 +271,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; @@ -296,6 +300,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]; } @@ -310,6 +318,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;