Commit cb101ed2 authored by Matthew Daley's avatar Matthew Daley Committed by David S. Miller

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: default avatarMatthew 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: default avatarAndrew Hendry <andrew.hendry@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent c7fd0d48
...@@ -91,7 +91,7 @@ int x25_parse_address_block(struct sk_buff *skb, ...@@ -91,7 +91,7 @@ int x25_parse_address_block(struct sk_buff *skb,
int needed; int needed;
int rc; int rc;
if (skb->len < 1) { if (!pskb_may_pull(skb, 1)) {
/* packet has no address block */ /* packet has no address block */
rc = 0; rc = 0;
goto empty; goto empty;
...@@ -100,7 +100,7 @@ int x25_parse_address_block(struct sk_buff *skb, ...@@ -100,7 +100,7 @@ int x25_parse_address_block(struct sk_buff *skb,
len = *skb->data; len = *skb->data;
needed = 1 + (len >> 4) + (len & 0x0f); 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 /* packet is too short to hold the addresses it claims
to hold */ to hold */
rc = -1; rc = -1;
...@@ -951,10 +951,10 @@ int x25_rx_call_request(struct sk_buff *skb, struct x25_neigh *nb, ...@@ -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 * Facilities length is mandatory in call request packets
*/ */
if (skb->len < 1) if (!pskb_may_pull(skb, 1))
goto out_clear_request; goto out_clear_request;
len = skb->data[0] + 1; len = skb->data[0] + 1;
if (skb->len < len) if (!pskb_may_pull(skb, len))
goto out_clear_request; goto out_clear_request;
skb_pull(skb,len); skb_pull(skb,len);
...@@ -964,6 +964,13 @@ int x25_rx_call_request(struct sk_buff *skb, struct x25_neigh *nb, ...@@ -964,6 +964,13 @@ int x25_rx_call_request(struct sk_buff *skb, struct x25_neigh *nb,
if (skb->len > X25_MAX_CUD_LEN) if (skb->len > X25_MAX_CUD_LEN)
goto out_clear_request; 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. * Find a listener for the particular address/cud pair.
*/ */
...@@ -1172,6 +1179,9 @@ static int x25_sendmsg(struct kiocb *iocb, struct socket *sock, ...@@ -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. * byte of the user data is the logical value of the Q Bit.
*/ */
if (test_bit(X25_Q_BIT_FLAG, &x25->flags)) { if (test_bit(X25_Q_BIT_FLAG, &x25->flags)) {
if (!pskb_may_pull(skb, 1))
goto out_kfree_skb;
qbit = skb->data[0]; qbit = skb->data[0];
skb_pull(skb, 1); skb_pull(skb, 1);
} }
...@@ -1250,7 +1260,9 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock, ...@@ -1250,7 +1260,9 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
struct x25_sock *x25 = x25_sk(sk); struct x25_sock *x25 = x25_sk(sk);
struct sockaddr_x25 *sx25 = (struct sockaddr_x25 *)msg->msg_name; struct sockaddr_x25 *sx25 = (struct sockaddr_x25 *)msg->msg_name;
size_t copied; size_t copied;
int qbit; int qbit, header_len = x25->neighbour->extended ?
X25_EXT_MIN_LEN : X25_STD_MIN_LEN;
struct sk_buff *skb; struct sk_buff *skb;
unsigned char *asmptr; unsigned char *asmptr;
int rc = -ENOTCONN; int rc = -ENOTCONN;
...@@ -1271,6 +1283,9 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock, ...@@ -1271,6 +1283,9 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
skb = skb_dequeue(&x25->interrupt_in_queue); 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); skb_pull(skb, X25_STD_MIN_LEN);
/* /*
...@@ -1291,10 +1306,12 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock, ...@@ -1291,10 +1306,12 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
if (!skb) if (!skb)
goto out; goto out;
if (!pskb_may_pull(skb, header_len))
goto out_free_dgram;
qbit = (skb->data[0] & X25_Q_BIT) == X25_Q_BIT; qbit = (skb->data[0] & X25_Q_BIT) == X25_Q_BIT;
skb_pull(skb, x25->neighbour->extended ? skb_pull(skb, header_len);
X25_EXT_MIN_LEN : X25_STD_MIN_LEN);
if (test_bit(X25_Q_BIT_FLAG, &x25->flags)) { if (test_bit(X25_Q_BIT_FLAG, &x25->flags)) {
asmptr = skb_push(skb, 1); asmptr = skb_push(skb, 1);
......
...@@ -32,6 +32,9 @@ static int x25_receive_data(struct sk_buff *skb, struct x25_neigh *nb) ...@@ -32,6 +32,9 @@ static int x25_receive_data(struct sk_buff *skb, struct x25_neigh *nb)
unsigned short frametype; unsigned short frametype;
unsigned int lci; unsigned int lci;
if (!pskb_may_pull(skb, X25_STD_MIN_LEN))
return 0;
frametype = skb->data[2]; frametype = skb->data[2];
lci = ((skb->data[0] << 8) & 0xF00) + ((skb->data[1] << 0) & 0x0FF); 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, ...@@ -115,6 +118,9 @@ int x25_lapb_receive_frame(struct sk_buff *skb, struct net_device *dev,
goto drop; goto drop;
} }
if (!pskb_may_pull(skb, 1))
return 0;
switch (skb->data[0]) { switch (skb->data[0]) {
case X25_IFACE_DATA: case X25_IFACE_DATA:
......
...@@ -44,7 +44,7 @@ ...@@ -44,7 +44,7 @@
int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities, int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
struct x25_dte_facilities *dte_facs, unsigned long *vc_fac_mask) struct x25_dte_facilities *dte_facs, unsigned long *vc_fac_mask)
{ {
unsigned char *p = skb->data; unsigned char *p;
unsigned int len; unsigned int len;
*vc_fac_mask = 0; *vc_fac_mask = 0;
...@@ -60,14 +60,16 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities, ...@@ -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->called_ae, '\0', sizeof(dte_facs->called_ae));
memset(dte_facs->calling_ae, '\0', sizeof(dte_facs->calling_ae)); memset(dte_facs->calling_ae, '\0', sizeof(dte_facs->calling_ae));
if (skb->len < 1) if (!pskb_may_pull(skb, 1))
return 0; return 0;
len = *p++; len = skb->data[0];
if (len >= skb->len) if (!pskb_may_pull(skb, 1 + len))
return -1; return -1;
p = skb->data + 1;
while (len > 0) { while (len > 0) {
switch (*p & X25_FAC_CLASS_MASK) { switch (*p & X25_FAC_CLASS_MASK) {
case X25_FAC_CLASS_A: case X25_FAC_CLASS_A:
......
...@@ -107,6 +107,8 @@ static int x25_state1_machine(struct sock *sk, struct sk_buff *skb, int frametyp ...@@ -107,6 +107,8 @@ static int x25_state1_machine(struct sock *sk, struct sk_buff *skb, int frametyp
/* /*
* Parse the data in the frame. * 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); skb_pull(skb, X25_STD_MIN_LEN);
len = x25_parse_address_block(skb, &source_addr, 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 ...@@ -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) if (skb->len > X25_MAX_CUD_LEN)
goto out_clear; goto out_clear;
skb_copy_from_linear_data(skb, skb_copy_bits(skb, 0, x25->calluserdata.cuddata,
x25->calluserdata.cuddata, skb->len);
skb->len);
x25->calluserdata.cudlength = skb->len; x25->calluserdata.cudlength = skb->len;
} }
if (!sock_flag(sk, SOCK_DEAD)) if (!sock_flag(sk, SOCK_DEAD))
...@@ -140,6 +141,9 @@ static int x25_state1_machine(struct sock *sk, struct sk_buff *skb, int frametyp ...@@ -140,6 +141,9 @@ static int x25_state1_machine(struct sock *sk, struct sk_buff *skb, int frametyp
break; break;
} }
case X25_CLEAR_REQUEST: 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_write_internal(sk, X25_CLEAR_CONFIRMATION);
x25_disconnect(sk, ECONNREFUSED, skb->data[3], skb->data[4]); x25_disconnect(sk, ECONNREFUSED, skb->data[3], skb->data[4]);
break; break;
...@@ -167,6 +171,9 @@ static int x25_state2_machine(struct sock *sk, struct sk_buff *skb, int frametyp ...@@ -167,6 +171,9 @@ static int x25_state2_machine(struct sock *sk, struct sk_buff *skb, int frametyp
switch (frametype) { switch (frametype) {
case X25_CLEAR_REQUEST: 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_write_internal(sk, X25_CLEAR_CONFIRMATION);
x25_disconnect(sk, 0, skb->data[3], skb->data[4]); x25_disconnect(sk, 0, skb->data[3], skb->data[4]);
break; break;
...@@ -180,6 +187,11 @@ static int x25_state2_machine(struct sock *sk, struct sk_buff *skb, int frametyp ...@@ -180,6 +187,11 @@ static int x25_state2_machine(struct sock *sk, struct sk_buff *skb, int frametyp
} }
return 0; 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 ...@@ -209,6 +221,9 @@ static int x25_state3_machine(struct sock *sk, struct sk_buff *skb, int frametyp
break; break;
case X25_CLEAR_REQUEST: 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_write_internal(sk, X25_CLEAR_CONFIRMATION);
x25_disconnect(sk, 0, skb->data[3], skb->data[4]); x25_disconnect(sk, 0, skb->data[3], skb->data[4]);
break; break;
...@@ -307,6 +322,12 @@ static int x25_state3_machine(struct sock *sk, struct sk_buff *skb, int frametyp ...@@ -307,6 +322,12 @@ static int x25_state3_machine(struct sock *sk, struct sk_buff *skb, int frametyp
} }
return queued; 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 ...@@ -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) static int x25_state4_machine(struct sock *sk, struct sk_buff *skb, int frametype)
{ {
struct x25_sock *x25 = x25_sk(sk);
switch (frametype) { switch (frametype) {
case X25_RESET_REQUEST: case X25_RESET_REQUEST:
x25_write_internal(sk, X25_RESET_CONFIRMATION); x25_write_internal(sk, X25_RESET_CONFIRMATION);
case X25_RESET_CONFIRMATION: { case X25_RESET_CONFIRMATION: {
struct x25_sock *x25 = x25_sk(sk);
x25_stop_timer(sk); x25_stop_timer(sk);
x25->condition = 0x00; x25->condition = 0x00;
x25->va = 0; x25->va = 0;
...@@ -334,6 +355,9 @@ static int x25_state4_machine(struct sock *sk, struct sk_buff *skb, int frametyp ...@@ -334,6 +355,9 @@ static int x25_state4_machine(struct sock *sk, struct sk_buff *skb, int frametyp
break; break;
} }
case X25_CLEAR_REQUEST: 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_write_internal(sk, X25_CLEAR_CONFIRMATION);
x25_disconnect(sk, 0, skb->data[3], skb->data[4]); x25_disconnect(sk, 0, skb->data[3], skb->data[4]);
break; break;
...@@ -343,6 +367,12 @@ static int x25_state4_machine(struct sock *sk, struct sk_buff *skb, int frametyp ...@@ -343,6 +367,12 @@ static int x25_state4_machine(struct sock *sk, struct sk_buff *skb, int frametyp
} }
return 0; 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 */ /* Higher level upcall for a LAPB frame */
......
...@@ -90,6 +90,9 @@ void x25_link_control(struct sk_buff *skb, struct x25_neigh *nb, ...@@ -90,6 +90,9 @@ void x25_link_control(struct sk_buff *skb, struct x25_neigh *nb,
break; break;
case X25_DIAGNOSTIC: case X25_DIAGNOSTIC:
if (!pskb_may_pull(skb, X25_STD_MIN_LEN + 4))
break;
printk(KERN_WARNING "x25: diagnostic #%d - %02X %02X %02X\n", printk(KERN_WARNING "x25: diagnostic #%d - %02X %02X %02X\n",
skb->data[3], skb->data[4], skb->data[3], skb->data[4],
skb->data[5], skb->data[6]); skb->data[5], skb->data[6]);
......
...@@ -269,7 +269,11 @@ int x25_decode(struct sock *sk, struct sk_buff *skb, int *ns, int *nr, int *q, ...@@ -269,7 +269,11 @@ int x25_decode(struct sock *sk, struct sk_buff *skb, int *ns, int *nr, int *q,
int *d, int *m) int *d, int *m)
{ {
struct x25_sock *x25 = x25_sk(sk); 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; *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, ...@@ -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 || if (frame[2] == X25_RR ||
frame[2] == X25_RNR || frame[2] == X25_RNR ||
frame[2] == X25_REJ) { frame[2] == X25_REJ) {
if (!pskb_may_pull(skb, X25_EXT_MIN_LEN))
return X25_ILLEGAL;
frame = skb->data;
*nr = (frame[3] >> 1) & 0x7F; *nr = (frame[3] >> 1) & 0x7F;
return frame[2]; return frame[2];
} }
...@@ -308,6 +316,10 @@ int x25_decode(struct sock *sk, struct sk_buff *skb, int *ns, int *nr, int *q, ...@@ -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 (x25->neighbour->extended) {
if ((frame[2] & 0x01) == X25_DATA) { 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; *q = (frame[0] & X25_Q_BIT) == X25_Q_BIT;
*d = (frame[0] & X25_D_BIT) == X25_D_BIT; *d = (frame[0] & X25_D_BIT) == X25_D_BIT;
*m = (frame[3] & X25_EXT_M_BIT) == X25_EXT_M_BIT; *m = (frame[3] & X25_EXT_M_BIT) == X25_EXT_M_BIT;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment