Skip to content

Commit 676d236

Browse files
davem330davem330
committed
net: Fix use after free by removing length arg from sk_data_ready callbacks.
Several spots in the kernel perform a sequence like: skb_queue_tail(&sk->s_receive_queue, skb); sk->sk_data_ready(sk, skb->len); But at the moment we place the SKB onto the socket receive queue it can be consumed and freed up. So this skb->len access is potentially to freed up memory. Furthermore, the skb->len can be modified by the consumer so it is possible that the value isn't accurate. And finally, no actual implementation of this callback actually uses the length argument. And since nobody actually cared about it's value, lots of call sites pass arbitrary values in such as '0' and even '1'. So just remove the length argument from the callback, that way there is no confusion whatsoever and all of these use-after-free cases get fixed as a side effect. Based upon a patch by Eric Dumazet and his suggestion to audit this issue tree-wide. Signed-off-by: David S. Miller <[email protected]>
1 parent ad20d5f commit 676d236

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

58 files changed

+112
-121
lines changed

‎drivers/scsi/iscsi_tcp.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ static inline int iscsi_sw_sk_state_check(struct sock *sk)
125125
return 0;
126126
}
127127

128-
static void iscsi_sw_tcp_data_ready(struct sock *sk, int flag)
128+
static void iscsi_sw_tcp_data_ready(struct sock *sk)
129129
{
130130
struct iscsi_conn *conn;
131131
struct iscsi_tcp_conn *tcp_conn;

‎drivers/scsi/iscsi_tcp.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ struct iscsi_sw_tcp_conn {
4040

4141
struct iscsi_sw_tcp_send out;
4242
/* old values for socket callbacks */
43-
void (*old_data_ready)(struct sock *, int);
43+
void (*old_data_ready)(struct sock *);
4444
void (*old_state_change)(struct sock *);
4545
void (*old_write_space)(struct sock *);
4646

‎drivers/staging/lustre/lnet/klnds/socklnd/socklnd_lib-linux.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -655,7 +655,7 @@ extern void ksocknal_write_callback (ksock_conn_t *conn);
655655
* socket call back in Linux
656656
*/
657657
static void
658-
ksocknal_data_ready (struct sock *sk, int n)
658+
ksocknal_data_ready (struct sock *sk)
659659
{
660660
ksock_conn_t *conn;
661661

@@ -666,7 +666,7 @@ ksocknal_data_ready (struct sock *sk, int n)
666666
conn = sk->sk_user_data;
667667
if (conn == NULL) { /* raced with ksocknal_terminate_conn */
668668
LASSERT (sk->sk_data_ready != &ksocknal_data_ready);
669-
sk->sk_data_ready (sk, n);
669+
sk->sk_data_ready (sk);
670670
} else
671671
ksocknal_read_callback(conn);
672672

‎drivers/target/iscsi/iscsi_target_core.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,7 @@ struct iscsi_conn {
556556
struct completion rx_half_close_comp;
557557
/* socket used by this connection */
558558
struct socket *sock;
559-
void (*orig_data_ready)(struct sock *, int);
559+
void (*orig_data_ready)(struct sock *);
560560
void (*orig_state_change)(struct sock *);
561561
#define LOGIN_FLAGS_READ_ACTIVE 1
562562
#define LOGIN_FLAGS_CLOSED 2

‎drivers/target/iscsi/iscsi_target_nego.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ static int iscsi_target_do_tx_login_io(struct iscsi_conn *conn, struct iscsi_log
375375
return 0;
376376
}
377377

378-
static void iscsi_target_sk_data_ready(struct sock *sk, int count)
378+
static void iscsi_target_sk_data_ready(struct sock *sk)
379379
{
380380
struct iscsi_conn *conn = sk->sk_user_data;
381381
bool rc;

‎fs/dlm/lowcomms.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ int dlm_lowcomms_addr(int nodeid, struct sockaddr_storage *addr, int len)
424424
}
425425

426426
/* Data available on socket or listen socket received a connect */
427-
static void lowcomms_data_ready(struct sock *sk, int count_unused)
427+
static void lowcomms_data_ready(struct sock *sk)
428428
{
429429
struct connection *con = sock2con(sk);
430430
if (con && !test_and_set_bit(CF_READ_PENDING, &con->flags))

‎fs/ncpfs/ncp_fs_sb.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ struct ncp_server {
111111

112112
spinlock_t requests_lock; /* Lock accesses to tx.requests, tx.creq and rcv.creq when STREAM mode */
113113

114-
void (*data_ready)(struct sock* sk, int len);
114+
void (*data_ready)(struct sock* sk);
115115
void (*error_report)(struct sock* sk);
116116
void (*write_space)(struct sock* sk); /* STREAM mode only */
117117
struct {
@@ -153,7 +153,7 @@ extern void ncp_tcp_tx_proc(struct work_struct *work);
153153
extern void ncpdgram_rcv_proc(struct work_struct *work);
154154
extern void ncpdgram_timeout_proc(struct work_struct *work);
155155
extern void ncpdgram_timeout_call(unsigned long server);
156-
extern void ncp_tcp_data_ready(struct sock* sk, int len);
156+
extern void ncp_tcp_data_ready(struct sock* sk);
157157
extern void ncp_tcp_write_space(struct sock* sk);
158158
extern void ncp_tcp_error_report(struct sock* sk);
159159

‎fs/ncpfs/sock.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,11 @@ static void ncp_req_put(struct ncp_request_reply *req)
9696
kfree(req);
9797
}
9898

99-
void ncp_tcp_data_ready(struct sock *sk, int len)
99+
void ncp_tcp_data_ready(struct sock *sk)
100100
{
101101
struct ncp_server *server = sk->sk_user_data;
102102

103-
server->data_ready(sk, len);
103+
server->data_ready(sk);
104104
schedule_work(&server->rcv.tq);
105105
}
106106

‎fs/ocfs2/cluster/tcp.c

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ static int o2net_sys_err_translations[O2NET_ERR_MAX] =
137137
static void o2net_sc_connect_completed(struct work_struct *work);
138138
static void o2net_rx_until_empty(struct work_struct *work);
139139
static void o2net_shutdown_sc(struct work_struct *work);
140-
static void o2net_listen_data_ready(struct sock *sk, int bytes);
140+
static void o2net_listen_data_ready(struct sock *sk);
141141
static void o2net_sc_send_keep_req(struct work_struct *work);
142142
static void o2net_idle_timer(unsigned long data);
143143
static void o2net_sc_postpone_idle(struct o2net_sock_container *sc);
@@ -597,9 +597,9 @@ static void o2net_set_nn_state(struct o2net_node *nn,
597597
}
598598

599599
/* see o2net_register_callbacks() */
600-
static void o2net_data_ready(struct sock *sk, int bytes)
600+
static void o2net_data_ready(struct sock *sk)
601601
{
602-
void (*ready)(struct sock *sk, int bytes);
602+
void (*ready)(struct sock *sk);
603603

604604
read_lock(&sk->sk_callback_lock);
605605
if (sk->sk_user_data) {
@@ -613,7 +613,7 @@ static void o2net_data_ready(struct sock *sk, int bytes)
613613
}
614614
read_unlock(&sk->sk_callback_lock);
615615

616-
ready(sk, bytes);
616+
ready(sk);
617617
}
618618

619619
/* see o2net_register_callbacks() */
@@ -1953,9 +1953,9 @@ static void o2net_accept_many(struct work_struct *work)
19531953
cond_resched();
19541954
}
19551955

1956-
static void o2net_listen_data_ready(struct sock *sk, int bytes)
1956+
static void o2net_listen_data_ready(struct sock *sk)
19571957
{
1958-
void (*ready)(struct sock *sk, int bytes);
1958+
void (*ready)(struct sock *sk);
19591959

19601960
read_lock(&sk->sk_callback_lock);
19611961
ready = sk->sk_user_data;
@@ -1978,7 +1978,6 @@ static void o2net_listen_data_ready(struct sock *sk, int bytes)
19781978
*/
19791979

19801980
if (sk->sk_state == TCP_LISTEN) {
1981-
mlog(ML_TCP, "bytes: %d\n", bytes);
19821981
queue_work(o2net_wq, &o2net_listen_work);
19831982
} else {
19841983
ready = NULL;
@@ -1987,7 +1986,7 @@ static void o2net_listen_data_ready(struct sock *sk, int bytes)
19871986
out:
19881987
read_unlock(&sk->sk_callback_lock);
19891988
if (ready != NULL)
1990-
ready(sk, bytes);
1989+
ready(sk);
19911990
}
19921991

19931992
static int o2net_open_listening_sock(__be32 addr, __be16 port)

‎fs/ocfs2/cluster/tcp_internal.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ struct o2net_sock_container {
165165

166166
/* original handlers for the sockets */
167167
void (*sc_state_change)(struct sock *sk);
168-
void (*sc_data_ready)(struct sock *sk, int bytes);
168+
void (*sc_data_ready)(struct sock *sk);
169169

170170
u32 sc_msg_key;
171171
u16 sc_msg_type;

‎include/linux/sunrpc/svcsock.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ struct svc_sock {
2222

2323
/* We keep the old state_change and data_ready CB's here */
2424
void (*sk_ostate)(struct sock *);
25-
void (*sk_odata)(struct sock *, int bytes);
25+
void (*sk_odata)(struct sock *);
2626
void (*sk_owspace)(struct sock *);
2727

2828
/* private TCP part */

‎include/net/sctp/sctp.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ void sctp_addr_wq_mgmt(struct net *, struct sctp_sockaddr_entry *, int);
101101
int sctp_backlog_rcv(struct sock *sk, struct sk_buff *skb);
102102
int sctp_inet_listen(struct socket *sock, int backlog);
103103
void sctp_write_space(struct sock *sk);
104-
void sctp_data_ready(struct sock *sk, int len);
104+
void sctp_data_ready(struct sock *sk);
105105
unsigned int sctp_poll(struct file *file, struct socket *sock,
106106
poll_table *wait);
107107
void sctp_sock_rfree(struct sk_buff *skb);

‎include/net/sock.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,7 @@ struct sock {
418418
u32 sk_classid;
419419
struct cg_proto *sk_cgrp;
420420
void (*sk_state_change)(struct sock *sk);
421-
void (*sk_data_ready)(struct sock *sk, int bytes);
421+
void (*sk_data_ready)(struct sock *sk);
422422
void (*sk_write_space)(struct sock *sk);
423423
void (*sk_error_report)(struct sock *sk);
424424
int (*sk_backlog_rcv)(struct sock *sk,

‎net/atm/clip.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ static int to_atmarpd(enum atmarp_ctrl_type type, int itf, __be32 ip)
6868

6969
sk = sk_atm(atmarpd);
7070
skb_queue_tail(&sk->sk_receive_queue, skb);
71-
sk->sk_data_ready(sk, skb->len);
71+
sk->sk_data_ready(sk);
7272
return 0;
7373
}
7474

‎net/atm/lec.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ static void lec_handle_bridge(struct sk_buff *skb, struct net_device *dev)
152152
atm_force_charge(priv->lecd, skb2->truesize);
153153
sk = sk_atm(priv->lecd);
154154
skb_queue_tail(&sk->sk_receive_queue, skb2);
155-
sk->sk_data_ready(sk, skb2->len);
155+
sk->sk_data_ready(sk);
156156
}
157157
}
158158
#endif /* defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE) */
@@ -447,7 +447,7 @@ static int lec_atm_send(struct atm_vcc *vcc, struct sk_buff *skb)
447447
atm_force_charge(priv->lecd, skb2->truesize);
448448
sk = sk_atm(priv->lecd);
449449
skb_queue_tail(&sk->sk_receive_queue, skb2);
450-
sk->sk_data_ready(sk, skb2->len);
450+
sk->sk_data_ready(sk);
451451
}
452452
}
453453
#endif /* defined(CONFIG_BRIDGE) || defined(CONFIG_BRIDGE_MODULE) */
@@ -530,13 +530,13 @@ send_to_lecd(struct lec_priv *priv, atmlec_msg_type type,
530530
atm_force_charge(priv->lecd, skb->truesize);
531531
sk = sk_atm(priv->lecd);
532532
skb_queue_tail(&sk->sk_receive_queue, skb);
533-
sk->sk_data_ready(sk, skb->len);
533+
sk->sk_data_ready(sk);
534534

535535
if (data != NULL) {
536536
pr_debug("about to send %d bytes of data\n", data->len);
537537
atm_force_charge(priv->lecd, data->truesize);
538538
skb_queue_tail(&sk->sk_receive_queue, data);
539-
sk->sk_data_ready(sk, skb->len);
539+
sk->sk_data_ready(sk);
540540
}
541541

542542
return 0;
@@ -616,7 +616,7 @@ static void lec_push(struct atm_vcc *vcc, struct sk_buff *skb)
616616

617617
pr_debug("%s: To daemon\n", dev->name);
618618
skb_queue_tail(&sk->sk_receive_queue, skb);
619-
sk->sk_data_ready(sk, skb->len);
619+
sk->sk_data_ready(sk);
620620
} else { /* Data frame, queue to protocol handlers */
621621
struct lec_arp_table *entry;
622622
unsigned char *src, *dst;

‎net/atm/mpc.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -706,7 +706,7 @@ static void mpc_push(struct atm_vcc *vcc, struct sk_buff *skb)
706706
dprintk("(%s) control packet arrived\n", dev->name);
707707
/* Pass control packets to daemon */
708708
skb_queue_tail(&sk->sk_receive_queue, skb);
709-
sk->sk_data_ready(sk, skb->len);
709+
sk->sk_data_ready(sk);
710710
return;
711711
}
712712

@@ -992,7 +992,7 @@ int msg_to_mpoad(struct k_message *mesg, struct mpoa_client *mpc)
992992

993993
sk = sk_atm(mpc->mpoad_vcc);
994994
skb_queue_tail(&sk->sk_receive_queue, skb);
995-
sk->sk_data_ready(sk, skb->len);
995+
sk->sk_data_ready(sk);
996996

997997
return 0;
998998
}
@@ -1273,7 +1273,7 @@ static void purge_egress_shortcut(struct atm_vcc *vcc, eg_cache_entry *entry)
12731273

12741274
sk = sk_atm(vcc);
12751275
skb_queue_tail(&sk->sk_receive_queue, skb);
1276-
sk->sk_data_ready(sk, skb->len);
1276+
sk->sk_data_ready(sk);
12771277
dprintk("exiting\n");
12781278
}
12791279

‎net/atm/raw.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ static void atm_push_raw(struct atm_vcc *vcc, struct sk_buff *skb)
2525
struct sock *sk = sk_atm(vcc);
2626

2727
skb_queue_tail(&sk->sk_receive_queue, skb);
28-
sk->sk_data_ready(sk, skb->len);
28+
sk->sk_data_ready(sk);
2929
}
3030
}
3131

‎net/atm/signaling.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ static void sigd_put_skb(struct sk_buff *skb)
5151
#endif
5252
atm_force_charge(sigd, skb->truesize);
5353
skb_queue_tail(&sk_atm(sigd)->sk_receive_queue, skb);
54-
sk_atm(sigd)->sk_data_ready(sk_atm(sigd), skb->len);
54+
sk_atm(sigd)->sk_data_ready(sk_atm(sigd));
5555
}
5656

5757
static void modify_qos(struct atm_vcc *vcc, struct atmsvc_msg *msg)

‎net/ax25/ax25_in.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,7 @@ static int ax25_rcv(struct sk_buff *skb, struct net_device *dev,
422422

423423
if (sk) {
424424
if (!sock_flag(sk, SOCK_DEAD))
425-
sk->sk_data_ready(sk, skb->len);
425+
sk->sk_data_ready(sk);
426426
sock_put(sk);
427427
} else {
428428
free:

‎net/bluetooth/l2cap_sock.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,7 +1271,7 @@ static void l2cap_sock_teardown_cb(struct l2cap_chan *chan, int err)
12711271

12721272
if (parent) {
12731273
bt_accept_unlink(sk);
1274-
parent->sk_data_ready(parent, 0);
1274+
parent->sk_data_ready(parent);
12751275
} else {
12761276
sk->sk_state_change(sk);
12771277
}
@@ -1327,7 +1327,7 @@ static void l2cap_sock_ready_cb(struct l2cap_chan *chan)
13271327
sk->sk_state_change(sk);
13281328

13291329
if (parent)
1330-
parent->sk_data_ready(parent, 0);
1330+
parent->sk_data_ready(parent);
13311331

13321332
release_sock(sk);
13331333
}
@@ -1340,7 +1340,7 @@ static void l2cap_sock_defer_cb(struct l2cap_chan *chan)
13401340

13411341
parent = bt_sk(sk)->parent;
13421342
if (parent)
1343-
parent->sk_data_ready(parent, 0);
1343+
parent->sk_data_ready(parent);
13441344

13451345
release_sock(sk);
13461346
}

‎net/bluetooth/rfcomm/core.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,9 @@ static void rfcomm_l2state_change(struct sock *sk)
186186
rfcomm_schedule();
187187
}
188188

189-
static void rfcomm_l2data_ready(struct sock *sk, int bytes)
189+
static void rfcomm_l2data_ready(struct sock *sk)
190190
{
191-
BT_DBG("%p bytes %d", sk, bytes);
191+
BT_DBG("%p", sk);
192192
rfcomm_schedule();
193193
}
194194

‎net/bluetooth/rfcomm/sock.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ static void rfcomm_sk_data_ready(struct rfcomm_dlc *d, struct sk_buff *skb)
5454

5555
atomic_add(skb->len, &sk->sk_rmem_alloc);
5656
skb_queue_tail(&sk->sk_receive_queue, skb);
57-
sk->sk_data_ready(sk, skb->len);
57+
sk->sk_data_ready(sk);
5858

5959
if (atomic_read(&sk->sk_rmem_alloc) >= sk->sk_rcvbuf)
6060
rfcomm_dlc_throttle(d);
@@ -84,7 +84,7 @@ static void rfcomm_sk_state_change(struct rfcomm_dlc *d, int err)
8484
sock_set_flag(sk, SOCK_ZAPPED);
8585
bt_accept_unlink(sk);
8686
}
87-
parent->sk_data_ready(parent, 0);
87+
parent->sk_data_ready(parent);
8888
} else {
8989
if (d->state == BT_CONNECTED)
9090
rfcomm_session_getaddr(d->session,

‎net/bluetooth/sco.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1024,7 +1024,7 @@ static void sco_conn_ready(struct sco_conn *conn)
10241024
sk->sk_state = BT_CONNECTED;
10251025

10261026
/* Wake up parent */
1027-
parent->sk_data_ready(parent, 1);
1027+
parent->sk_data_ready(parent);
10281028

10291029
bh_unlock_sock(parent);
10301030

‎net/caif/caif_socket.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,6 @@ static void caif_flow_ctrl(struct sock *sk, int mode)
124124
static int caif_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
125125
{
126126
int err;
127-
int skb_len;
128127
unsigned long flags;
129128
struct sk_buff_head *list = &sk->sk_receive_queue;
130129
struct caifsock *cf_sk = container_of(sk, struct caifsock, sk);
@@ -153,14 +152,13 @@ static int caif_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
153152
* may be freed by other threads of control pulling packets
154153
* from the queue.
155154
*/
156-
skb_len = skb->len;
157155
spin_lock_irqsave(&list->lock, flags);
158156
if (!sock_flag(sk, SOCK_DEAD))
159157
__skb_queue_tail(list, skb);
160158
spin_unlock_irqrestore(&list->lock, flags);
161159

162160
if (!sock_flag(sk, SOCK_DEAD))
163-
sk->sk_data_ready(sk, skb_len);
161+
sk->sk_data_ready(sk);
164162
else
165163
kfree_skb(skb);
166164
return 0;

‎net/ceph/messenger.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ static void con_sock_state_closed(struct ceph_connection *con)
383383
*/
384384

385385
/* data available on socket, or listen socket received a connect */
386-
static void ceph_sock_data_ready(struct sock *sk, int count_unused)
386+
static void ceph_sock_data_ready(struct sock *sk)
387387
{
388388
struct ceph_connection *con = sk->sk_user_data;
389389
if (atomic_read(&con->msgr->stopping)) {

0 commit comments

Comments
 (0)