Skip to content

Commit f3d3342

Browse files
strssndktndavem330
strssndktn
authored and
davem330
committed
net: rework recvmsg handler msg_name and msg_namelen logic
This patch now always passes msg->msg_namelen as 0. recvmsg handlers must set msg_namelen to the proper size <= sizeof(struct sockaddr_storage) to return msg_name to the user. This prevents numerous uninitialized memory leaks we had in the recvmsg handlers and makes it harder for new code to accidentally leak uninitialized memory. Optimize for the case recvfrom is called with NULL as address. We don't need to copy the address at all, so set it to NULL before invoking the recvmsg handler. We can do so, because all the recvmsg handlers must cope with the case a plain read() is called on them. read() also sets msg_name to NULL. Also document these changes in include/linux/net.h as suggested by David Miller. Changes since RFC: Set msg->msg_name = NULL if user specified a NULL in msg_name but had a non-null msg_namelen in verify_iovec/verify_compat_iovec. This doesn't affect sendto as it would bail out earlier while trying to copy-in the address. It also more naturally reflects the logic by the callers of verify_iovec. With this change in place I could remove " if (!uaddr || msg_sys->msg_namelen == 0) msg->msg_name = NULL ". This change does not alter the user visible error logic as we ignore msg_namelen as long as msg_name is NULL. Also remove two unnecessary curly brackets in ___sys_recvmsg and change comments to netdev style. Cc: David Miller <[email protected]> Suggested-by: Eric Dumazet <[email protected]> Signed-off-by: Hannes Frederic Sowa <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent f873042 commit f3d3342

File tree

35 files changed

+67
-115
lines changed

35 files changed

+67
-115
lines changed

‎crypto/algif_hash.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,6 @@ static int hash_recvmsg(struct kiocb *unused, struct socket *sock,
161161
else if (len < ds)
162162
msg->msg_flags |= MSG_TRUNC;
163163

164-
msg->msg_namelen = 0;
165-
166164
lock_sock(sk);
167165
if (ctx->more) {
168166
ctx->more = 0;

‎crypto/algif_skcipher.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,6 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
432432
long copied = 0;
433433

434434
lock_sock(sk);
435-
msg->msg_namelen = 0;
436435
for (iov = msg->msg_iov, iovlen = msg->msg_iovlen; iovlen > 0;
437436
iovlen--, iov++) {
438437
unsigned long seglen = iov->iov_len;

‎drivers/isdn/mISDN/socket.c

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,6 @@ mISDN_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
117117
{
118118
struct sk_buff *skb;
119119
struct sock *sk = sock->sk;
120-
struct sockaddr_mISDN *maddr;
121120

122121
int copied, err;
123122

@@ -135,9 +134,9 @@ mISDN_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
135134
if (!skb)
136135
return err;
137136

138-
if (msg->msg_namelen >= sizeof(struct sockaddr_mISDN)) {
139-
msg->msg_namelen = sizeof(struct sockaddr_mISDN);
140-
maddr = (struct sockaddr_mISDN *)msg->msg_name;
137+
if (msg->msg_name) {
138+
struct sockaddr_mISDN *maddr = msg->msg_name;
139+
141140
maddr->family = AF_ISDN;
142141
maddr->dev = _pms(sk)->dev->id;
143142
if ((sk->sk_protocol == ISDN_P_LAPD_TE) ||
@@ -150,11 +149,7 @@ mISDN_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
150149
maddr->sapi = _pms(sk)->ch.addr & 0xFF;
151150
maddr->tei = (_pms(sk)->ch.addr >> 8) & 0xFF;
152151
}
153-
} else {
154-
if (msg->msg_namelen)
155-
printk(KERN_WARNING "%s: too small namelen %d\n",
156-
__func__, msg->msg_namelen);
157-
msg->msg_namelen = 0;
152+
msg->msg_namelen = sizeof(*maddr);
158153
}
159154

160155
copied = skb->len + MISDN_HEADER_LEN;

‎drivers/net/ppp/pppoe.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -979,8 +979,6 @@ static int pppoe_recvmsg(struct kiocb *iocb, struct socket *sock,
979979
if (error < 0)
980980
goto end;
981981

982-
m->msg_namelen = 0;
983-
984982
if (skb) {
985983
total_len = min_t(size_t, total_len, skb->len);
986984
error = skb_copy_datagram_iovec(skb, 0, m->msg_iov, total_len);

‎include/linux/net.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,14 @@ struct proto_ops {
164164
#endif
165165
int (*sendmsg) (struct kiocb *iocb, struct socket *sock,
166166
struct msghdr *m, size_t total_len);
167+
/* Notes for implementing recvmsg:
168+
* ===============================
169+
* msg->msg_namelen should get updated by the recvmsg handlers
170+
* iff msg_name != NULL. It is by default 0 to prevent
171+
* returning uninitialized memory to user space. The recvfrom
172+
* handlers can assume that msg.msg_name is either NULL or has
173+
* a minimum size of sizeof(struct sockaddr_storage).
174+
*/
167175
int (*recvmsg) (struct kiocb *iocb, struct socket *sock,
168176
struct msghdr *m, size_t total_len,
169177
int flags);

‎net/appletalk/ddp.c

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1735,7 +1735,6 @@ static int atalk_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr
17351735
size_t size, int flags)
17361736
{
17371737
struct sock *sk = sock->sk;
1738-
struct sockaddr_at *sat = (struct sockaddr_at *)msg->msg_name;
17391738
struct ddpehdr *ddp;
17401739
int copied = 0;
17411740
int offset = 0;
@@ -1764,14 +1763,13 @@ static int atalk_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr
17641763
}
17651764
err = skb_copy_datagram_iovec(skb, offset, msg->msg_iov, copied);
17661765

1767-
if (!err) {
1768-
if (sat) {
1769-
sat->sat_family = AF_APPLETALK;
1770-
sat->sat_port = ddp->deh_sport;
1771-
sat->sat_addr.s_node = ddp->deh_snode;
1772-
sat->sat_addr.s_net = ddp->deh_snet;
1773-
}
1774-
msg->msg_namelen = sizeof(*sat);
1766+
if (!err && msg->msg_name) {
1767+
struct sockaddr_at *sat = msg->msg_name;
1768+
sat->sat_family = AF_APPLETALK;
1769+
sat->sat_port = ddp->deh_sport;
1770+
sat->sat_addr.s_node = ddp->deh_snode;
1771+
sat->sat_addr.s_net = ddp->deh_snet;
1772+
msg->msg_namelen = sizeof(*sat);
17751773
}
17761774

17771775
skb_free_datagram(sk, skb); /* Free the datagram. */

‎net/atm/common.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -531,8 +531,6 @@ int vcc_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg,
531531
struct sk_buff *skb;
532532
int copied, error = -EINVAL;
533533

534-
msg->msg_namelen = 0;
535-
536534
if (sock->state != SS_CONNECTED)
537535
return -ENOTCONN;
538536

‎net/ax25/af_ax25.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1636,11 +1636,11 @@ static int ax25_recvmsg(struct kiocb *iocb, struct socket *sock,
16361636

16371637
skb_copy_datagram_iovec(skb, 0, msg->msg_iov, copied);
16381638

1639-
if (msg->msg_namelen != 0) {
1640-
struct sockaddr_ax25 *sax = (struct sockaddr_ax25 *)msg->msg_name;
1639+
if (msg->msg_name) {
16411640
ax25_digi digi;
16421641
ax25_address src;
16431642
const unsigned char *mac = skb_mac_header(skb);
1643+
struct sockaddr_ax25 *sax = msg->msg_name;
16441644

16451645
memset(sax, 0, sizeof(struct full_sockaddr_ax25));
16461646
ax25_addr_parse(mac + 1, skb->data - mac - 1, &src, NULL,

‎net/bluetooth/af_bluetooth.c

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -224,10 +224,9 @@ int bt_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
224224

225225
skb = skb_recv_datagram(sk, flags, noblock, &err);
226226
if (!skb) {
227-
if (sk->sk_shutdown & RCV_SHUTDOWN) {
228-
msg->msg_namelen = 0;
227+
if (sk->sk_shutdown & RCV_SHUTDOWN)
229228
return 0;
230-
}
229+
231230
return err;
232231
}
233232

@@ -245,8 +244,6 @@ int bt_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
245244
if (bt_sk(sk)->skb_msg_name)
246245
bt_sk(sk)->skb_msg_name(skb, msg->msg_name,
247246
&msg->msg_namelen);
248-
else
249-
msg->msg_namelen = 0;
250247
}
251248

252249
skb_free_datagram(sk, skb);
@@ -295,8 +292,6 @@ int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
295292
if (flags & MSG_OOB)
296293
return -EOPNOTSUPP;
297294

298-
msg->msg_namelen = 0;
299-
300295
BT_DBG("sk %p size %zu", sk, size);
301296

302297
lock_sock(sk);

‎net/bluetooth/hci_sock.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -856,8 +856,6 @@ static int hci_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
856856
if (!skb)
857857
return err;
858858

859-
msg->msg_namelen = 0;
860-
861859
copied = skb->len;
862860
if (len < copied) {
863861
msg->msg_flags |= MSG_TRUNC;

‎net/bluetooth/rfcomm/sock.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,6 @@ static int rfcomm_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
615615

616616
if (test_and_clear_bit(RFCOMM_DEFER_SETUP, &d->flags)) {
617617
rfcomm_dlc_accept(d);
618-
msg->msg_namelen = 0;
619618
return 0;
620619
}
621620

‎net/bluetooth/sco.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -711,7 +711,6 @@ static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
711711
test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
712712
sco_conn_defer_accept(pi->conn->hcon, pi->setting);
713713
sk->sk_state = BT_CONFIG;
714-
msg->msg_namelen = 0;
715714

716715
release_sock(sk);
717716
return 0;

‎net/caif/caif_socket.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,6 @@ static int caif_seqpkt_recvmsg(struct kiocb *iocb, struct socket *sock,
286286
if (m->msg_flags&MSG_OOB)
287287
goto read_error;
288288

289-
m->msg_namelen = 0;
290-
291289
skb = skb_recv_datagram(sk, flags, 0 , &ret);
292290
if (!skb)
293291
goto read_error;
@@ -361,8 +359,6 @@ static int caif_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
361359
if (flags&MSG_OOB)
362360
goto out;
363361

364-
msg->msg_namelen = 0;
365-
366362
/*
367363
* Lock the socket to prevent queue disordering
368364
* while sleeps in memcpy_tomsg

‎net/compat.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,8 @@ int verify_compat_iovec(struct msghdr *kern_msg, struct iovec *kern_iov,
9393
if (err < 0)
9494
return err;
9595
}
96-
kern_msg->msg_name = kern_address;
96+
if (kern_msg->msg_name)
97+
kern_msg->msg_name = kern_address;
9798
} else
9899
kern_msg->msg_name = NULL;
99100

‎net/core/iovec.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr_storage *a
4848
if (err < 0)
4949
return err;
5050
}
51-
m->msg_name = address;
51+
if (m->msg_name)
52+
m->msg_name = address;
5253
} else {
5354
m->msg_name = NULL;
5455
}

‎net/ipx/af_ipx.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1823,15 +1823,14 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
18231823
if (skb->tstamp.tv64)
18241824
sk->sk_stamp = skb->tstamp;
18251825

1826-
msg->msg_namelen = sizeof(*sipx);
1827-
18281826
if (sipx) {
18291827
sipx->sipx_family = AF_IPX;
18301828
sipx->sipx_port = ipx->ipx_source.sock;
18311829
memcpy(sipx->sipx_node, ipx->ipx_source.node, IPX_NODE_LEN);
18321830
sipx->sipx_network = IPX_SKB_CB(skb)->ipx_source_net;
18331831
sipx->sipx_type = ipx->ipx_type;
18341832
sipx->sipx_zero = 0;
1833+
msg->msg_namelen = sizeof(*sipx);
18351834
}
18361835
rc = copied;
18371836

‎net/irda/af_irda.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1385,8 +1385,6 @@ static int irda_recvmsg_dgram(struct kiocb *iocb, struct socket *sock,
13851385

13861386
IRDA_DEBUG(4, "%s()\n", __func__);
13871387

1388-
msg->msg_namelen = 0;
1389-
13901388
skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
13911389
flags & MSG_DONTWAIT, &err);
13921390
if (!skb)
@@ -1451,8 +1449,6 @@ static int irda_recvmsg_stream(struct kiocb *iocb, struct socket *sock,
14511449
target = sock_rcvlowat(sk, flags & MSG_WAITALL, size);
14521450
timeo = sock_rcvtimeo(sk, noblock);
14531451

1454-
msg->msg_namelen = 0;
1455-
14561452
do {
14571453
int chunk;
14581454
struct sk_buff *skb = skb_dequeue(&sk->sk_receive_queue);

‎net/iucv/af_iucv.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1324,8 +1324,6 @@ static int iucv_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
13241324
int err = 0;
13251325
u32 offset;
13261326

1327-
msg->msg_namelen = 0;
1328-
13291327
if ((sk->sk_state == IUCV_DISCONN) &&
13301328
skb_queue_empty(&iucv->backlog_skb_q) &&
13311329
skb_queue_empty(&sk->sk_receive_queue) &&

‎net/key/af_key.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3616,7 +3616,6 @@ static int pfkey_recvmsg(struct kiocb *kiocb,
36163616
if (flags & ~(MSG_PEEK|MSG_DONTWAIT|MSG_TRUNC|MSG_CMSG_COMPAT))
36173617
goto out;
36183618

3619-
msg->msg_namelen = 0;
36203619
skb = skb_recv_datagram(sk, flags, flags & MSG_DONTWAIT, &err);
36213620
if (skb == NULL)
36223621
goto out;

‎net/l2tp/l2tp_ppp.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,6 @@ static int pppol2tp_recvmsg(struct kiocb *iocb, struct socket *sock,
197197
if (sk->sk_state & PPPOX_BOUND)
198198
goto end;
199199

200-
msg->msg_namelen = 0;
201-
202200
err = 0;
203201
skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
204202
flags & MSG_DONTWAIT, &err);

‎net/llc/af_llc.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -720,8 +720,6 @@ static int llc_ui_recvmsg(struct kiocb *iocb, struct socket *sock,
720720
int target; /* Read at least this many bytes */
721721
long timeo;
722722

723-
msg->msg_namelen = 0;
724-
725723
lock_sock(sk);
726724
copied = -ENOTCONN;
727725
if (unlikely(sk->sk_type == SOCK_STREAM && sk->sk_state == TCP_LISTEN))

‎net/netlink/af_netlink.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2335,8 +2335,6 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock,
23352335
}
23362336
#endif
23372337

2338-
msg->msg_namelen = 0;
2339-
23402338
copied = data_skb->len;
23412339
if (len < copied) {
23422340
msg->msg_flags |= MSG_TRUNC;

‎net/netrom/af_netrom.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1179,10 +1179,9 @@ static int nr_recvmsg(struct kiocb *iocb, struct socket *sock,
11791179
sax->sax25_family = AF_NETROM;
11801180
skb_copy_from_linear_data_offset(skb, 7, sax->sax25_call.ax25_call,
11811181
AX25_ADDR_LEN);
1182+
msg->msg_namelen = sizeof(*sax);
11821183
}
11831184

1184-
msg->msg_namelen = sizeof(*sax);
1185-
11861185
skb_free_datagram(sk, skb);
11871186

11881187
release_sock(sk);

‎net/nfc/llcp_sock.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -807,8 +807,6 @@ static int llcp_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
807807

808808
pr_debug("%p %zu\n", sk, len);
809809

810-
msg->msg_namelen = 0;
811-
812810
lock_sock(sk);
813811

814812
if (sk->sk_state == LLCP_CLOSED &&

‎net/nfc/rawsock.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,6 @@ static int rawsock_recvmsg(struct kiocb *iocb, struct socket *sock,
244244
if (!skb)
245245
return rc;
246246

247-
msg->msg_namelen = 0;
248-
249247
copied = skb->len;
250248
if (len < copied) {
251249
msg->msg_flags |= MSG_TRUNC;

‎net/packet/af_packet.c

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2660,7 +2660,6 @@ static int packet_recvmsg(struct kiocb *iocb, struct socket *sock,
26602660
struct sock *sk = sock->sk;
26612661
struct sk_buff *skb;
26622662
int copied, err;
2663-
struct sockaddr_ll *sll;
26642663
int vnet_hdr_len = 0;
26652664

26662665
err = -EINVAL;
@@ -2744,22 +2743,10 @@ static int packet_recvmsg(struct kiocb *iocb, struct socket *sock,
27442743
goto out_free;
27452744
}
27462745

2747-
/*
2748-
* If the address length field is there to be filled in, we fill
2749-
* it in now.
2750-
*/
2751-
2752-
sll = &PACKET_SKB_CB(skb)->sa.ll;
2753-
if (sock->type == SOCK_PACKET)
2754-
msg->msg_namelen = sizeof(struct sockaddr_pkt);
2755-
else
2756-
msg->msg_namelen = sll->sll_halen + offsetof(struct sockaddr_ll, sll_addr);
2757-
2758-
/*
2759-
* You lose any data beyond the buffer you gave. If it worries a
2760-
* user program they can ask the device for its MTU anyway.
2746+
/* You lose any data beyond the buffer you gave. If it worries
2747+
* a user program they can ask the device for its MTU
2748+
* anyway.
27612749
*/
2762-
27632750
copied = skb->len;
27642751
if (copied > len) {
27652752
copied = len;
@@ -2772,9 +2759,20 @@ static int packet_recvmsg(struct kiocb *iocb, struct socket *sock,
27722759

27732760
sock_recv_ts_and_drops(msg, sk, skb);
27742761

2775-
if (msg->msg_name)
2762+
if (msg->msg_name) {
2763+
/* If the address length field is there to be filled
2764+
* in, we fill it in now.
2765+
*/
2766+
if (sock->type == SOCK_PACKET) {
2767+
msg->msg_namelen = sizeof(struct sockaddr_pkt);
2768+
} else {
2769+
struct sockaddr_ll *sll = &PACKET_SKB_CB(skb)->sa.ll;
2770+
msg->msg_namelen = sll->sll_halen +
2771+
offsetof(struct sockaddr_ll, sll_addr);
2772+
}
27762773
memcpy(msg->msg_name, &PACKET_SKB_CB(skb)->sa,
27772774
msg->msg_namelen);
2775+
}
27782776

27792777
if (pkt_sk(sk)->auxdata) {
27802778
struct tpacket_auxdata aux;

‎net/rds/recv.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -410,8 +410,6 @@ int rds_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg,
410410

411411
rdsdebug("size %zu flags 0x%x timeo %ld\n", size, msg_flags, timeo);
412412

413-
msg->msg_namelen = 0;
414-
415413
if (msg_flags & MSG_OOB)
416414
goto out;
417415

0 commit comments

Comments
 (0)