Skip to content

Commit 6b3b697

Browse files
jtlaytonchucklever
authored andcommitted
sunrpc: allocate a separate bvec array for socket sends
svc_tcp_sendmsg() calls xdr_buf_to_bvec() with the second slot of rq_bvec as the start, but doesn't reduce the array length by one, which could lead to an array overrun. Also, rq_bvec is always rq_maxpages in length, which can be too short in some cases, since the TCP record marker consumes a slot. Fix both problems by adding a separate bvec array to the svc_sock that is specifically for sending. For TCP, make this array one slot longer than rq_maxpages, to account for the record marker. For UDP, only allocate as large an array as we need since it's limited to 64k of payload. Signed-off-by: Jeff Layton <jlayton@kernel.org> Reviewed-by: NeilBrown <neil@brown.name> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
1 parent ebd3330 commit 6b3b697

2 files changed

Lines changed: 51 additions & 7 deletions

File tree

include/linux/sunrpc/svcsock.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,9 @@ struct svc_sock {
2626
void (*sk_odata)(struct sock *);
2727
void (*sk_owspace)(struct sock *);
2828

29+
/* For sends (protected by xpt_mutex) */
30+
struct bio_vec *sk_bvec;
31+
2932
/* private TCP part */
3033
/* On-the-wire fragment header: */
3134
__be32 sk_marker;

net/sunrpc/svcsock.c

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,17 @@
6868

6969
#define RPCDBG_FACILITY RPCDBG_SVCXPRT
7070

71+
/*
72+
* For UDP:
73+
* 1 for header page
74+
* enough pages for RPCSVC_MAXPAYLOAD_UDP
75+
* 1 in case payload is not aligned
76+
* 1 for tail page
77+
*/
78+
enum {
79+
SUNRPC_MAX_UDP_SENDPAGES = 1 + RPCSVC_MAXPAYLOAD_UDP / PAGE_SIZE + 1 + 1
80+
};
81+
7182
/* To-do: to avoid tying up an nfsd thread while waiting for a
7283
* handshake request, the request could instead be deferred.
7384
*/
@@ -740,14 +751,14 @@ static int svc_udp_sendto(struct svc_rqst *rqstp)
740751
if (svc_xprt_is_dead(xprt))
741752
goto out_notconn;
742753

743-
count = xdr_buf_to_bvec(rqstp->rq_bvec, rqstp->rq_maxpages, xdr);
754+
count = xdr_buf_to_bvec(svsk->sk_bvec, SUNRPC_MAX_UDP_SENDPAGES, xdr);
744755

745-
iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, rqstp->rq_bvec,
756+
iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, svsk->sk_bvec,
746757
count, rqstp->rq_res.len);
747758
err = sock_sendmsg(svsk->sk_sock, &msg);
748759
if (err == -ECONNREFUSED) {
749760
/* ICMP error on earlier request. */
750-
iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, rqstp->rq_bvec,
761+
iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, svsk->sk_bvec,
751762
count, rqstp->rq_res.len);
752763
err = sock_sendmsg(svsk->sk_sock, &msg);
753764
}
@@ -1236,19 +1247,19 @@ static int svc_tcp_sendmsg(struct svc_sock *svsk, struct svc_rqst *rqstp,
12361247
int ret;
12371248

12381249
/* The stream record marker is copied into a temporary page
1239-
* fragment buffer so that it can be included in rq_bvec.
1250+
* fragment buffer so that it can be included in sk_bvec.
12401251
*/
12411252
buf = page_frag_alloc(&svsk->sk_frag_cache, sizeof(marker),
12421253
GFP_KERNEL);
12431254
if (!buf)
12441255
return -ENOMEM;
12451256
memcpy(buf, &marker, sizeof(marker));
1246-
bvec_set_virt(rqstp->rq_bvec, buf, sizeof(marker));
1257+
bvec_set_virt(svsk->sk_bvec, buf, sizeof(marker));
12471258

1248-
count = xdr_buf_to_bvec(rqstp->rq_bvec + 1, rqstp->rq_maxpages,
1259+
count = xdr_buf_to_bvec(svsk->sk_bvec + 1, rqstp->rq_maxpages,
12491260
&rqstp->rq_res);
12501261

1251-
iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, rqstp->rq_bvec,
1262+
iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, svsk->sk_bvec,
12521263
1 + count, sizeof(marker) + rqstp->rq_res.len);
12531264
ret = sock_sendmsg(svsk->sk_sock, &msg);
12541265
page_frag_free(buf);
@@ -1393,6 +1404,20 @@ void svc_sock_update_bufs(struct svc_serv *serv)
13931404
spin_unlock_bh(&serv->sv_lock);
13941405
}
13951406

1407+
static int svc_sock_sendpages(struct svc_serv *serv, struct socket *sock, int flags)
1408+
{
1409+
switch (sock->type) {
1410+
case SOCK_STREAM:
1411+
/* +1 for TCP record marker */
1412+
if (flags & SVC_SOCK_TEMPORARY)
1413+
return svc_serv_maxpages(serv) + 1;
1414+
return 0;
1415+
case SOCK_DGRAM:
1416+
return SUNRPC_MAX_UDP_SENDPAGES;
1417+
}
1418+
return -EINVAL;
1419+
}
1420+
13961421
/*
13971422
* Initialize socket for RPC use and create svc_sock struct
13981423
*/
@@ -1403,12 +1428,26 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv,
14031428
struct svc_sock *svsk;
14041429
struct sock *inet;
14051430
int pmap_register = !(flags & SVC_SOCK_ANONYMOUS);
1431+
int sendpages;
14061432
unsigned long pages;
14071433

1434+
sendpages = svc_sock_sendpages(serv, sock, flags);
1435+
if (sendpages < 0)
1436+
return ERR_PTR(sendpages);
1437+
14081438
pages = svc_serv_maxpages(serv);
14091439
svsk = kzalloc(struct_size(svsk, sk_pages, pages), GFP_KERNEL);
14101440
if (!svsk)
14111441
return ERR_PTR(-ENOMEM);
1442+
1443+
if (sendpages) {
1444+
svsk->sk_bvec = kcalloc(sendpages, sizeof(*svsk->sk_bvec), GFP_KERNEL);
1445+
if (!svsk->sk_bvec) {
1446+
kfree(svsk);
1447+
return ERR_PTR(-ENOMEM);
1448+
}
1449+
}
1450+
14121451
svsk->sk_maxpages = pages;
14131452

14141453
inet = sock->sk;
@@ -1420,6 +1459,7 @@ static struct svc_sock *svc_setup_socket(struct svc_serv *serv,
14201459
inet->sk_protocol,
14211460
ntohs(inet_sk(inet)->inet_sport));
14221461
if (err < 0) {
1462+
kfree(svsk->sk_bvec);
14231463
kfree(svsk);
14241464
return ERR_PTR(err);
14251465
}
@@ -1637,5 +1677,6 @@ static void svc_sock_free(struct svc_xprt *xprt)
16371677
sock_release(sock);
16381678

16391679
page_frag_cache_drain(&svsk->sk_frag_cache);
1680+
kfree(svsk->sk_bvec);
16401681
kfree(svsk);
16411682
}

0 commit comments

Comments
 (0)