Skip to content

Commit bee47cb

Browse files
Olga Kornievskaiachucklever
authored andcommitted
sunrpc: fix handling of server side tls alerts
Scott Mayhew discovered a security exploit in NFS over TLS in tls_alert_recv() due to its assumption it can read data from the msg iterator's kvec.. kTLS implementation splits TLS non-data record payload between the control message buffer (which includes the type such as TLS aler or TLS cipher change) and the rest of the payload (say TLS alert's level/description) which goes into the msg payload buffer. This patch proposes to rework how control messages are setup and used by sock_recvmsg(). If no control message structure is setup, kTLS layer will read and process TLS data record types. As soon as it encounters a TLS control message, it would return an error. At that point, NFS can setup a kvec backed msg buffer and read in the control message such as a TLS alert. Msg iterator can advance the kvec pointer as a part of the copy process thus we need to revert the iterator before calling into the tls_alert_recv. Reported-by: Scott Mayhew <smayhew@redhat.com> Fixes: 5e052dd ("SUNRPC: Recognize control messages in server-side TCP socket code") Suggested-by: Trond Myklebust <trondmy@hammerspace.com> Cc: stable@vger.kernel.org Signed-off-by: Olga Kornievskaia <okorniev@redhat.com> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
1 parent e5a7315 commit bee47cb

File tree

1 file changed

+35
-8
lines changed

1 file changed

+35
-8
lines changed

net/sunrpc/svcsock.c

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -257,20 +257,47 @@ svc_tcp_sock_process_cmsg(struct socket *sock, struct msghdr *msg,
257257
}
258258

259259
static int
260-
svc_tcp_sock_recv_cmsg(struct svc_sock *svsk, struct msghdr *msg)
260+
svc_tcp_sock_recv_cmsg(struct socket *sock, unsigned int *msg_flags)
261261
{
262262
union {
263263
struct cmsghdr cmsg;
264264
u8 buf[CMSG_SPACE(sizeof(u8))];
265265
} u;
266-
struct socket *sock = svsk->sk_sock;
266+
u8 alert[2];
267+
struct kvec alert_kvec = {
268+
.iov_base = alert,
269+
.iov_len = sizeof(alert),
270+
};
271+
struct msghdr msg = {
272+
.msg_flags = *msg_flags,
273+
.msg_control = &u,
274+
.msg_controllen = sizeof(u),
275+
};
276+
int ret;
277+
278+
iov_iter_kvec(&msg.msg_iter, ITER_DEST, &alert_kvec, 1,
279+
alert_kvec.iov_len);
280+
ret = sock_recvmsg(sock, &msg, MSG_DONTWAIT);
281+
if (ret > 0 &&
282+
tls_get_record_type(sock->sk, &u.cmsg) == TLS_RECORD_TYPE_ALERT) {
283+
iov_iter_revert(&msg.msg_iter, ret);
284+
ret = svc_tcp_sock_process_cmsg(sock, &msg, &u.cmsg, -EAGAIN);
285+
}
286+
return ret;
287+
}
288+
289+
static int
290+
svc_tcp_sock_recvmsg(struct svc_sock *svsk, struct msghdr *msg)
291+
{
267292
int ret;
293+
struct socket *sock = svsk->sk_sock;
268294

269-
msg->msg_control = &u;
270-
msg->msg_controllen = sizeof(u);
271295
ret = sock_recvmsg(sock, msg, MSG_DONTWAIT);
272-
if (unlikely(msg->msg_controllen != sizeof(u)))
273-
ret = svc_tcp_sock_process_cmsg(sock, msg, &u.cmsg, ret);
296+
if (msg->msg_flags & MSG_CTRUNC) {
297+
msg->msg_flags &= ~(MSG_CTRUNC | MSG_EOR);
298+
if (ret == 0 || ret == -EIO)
299+
ret = svc_tcp_sock_recv_cmsg(sock, &msg->msg_flags);
300+
}
274301
return ret;
275302
}
276303

@@ -321,7 +348,7 @@ static ssize_t svc_tcp_read_msg(struct svc_rqst *rqstp, size_t buflen,
321348
iov_iter_advance(&msg.msg_iter, seek);
322349
buflen -= seek;
323350
}
324-
len = svc_tcp_sock_recv_cmsg(svsk, &msg);
351+
len = svc_tcp_sock_recvmsg(svsk, &msg);
325352
if (len > 0)
326353
svc_flush_bvec(bvec, len, seek);
327354

@@ -1018,7 +1045,7 @@ static ssize_t svc_tcp_read_marker(struct svc_sock *svsk,
10181045
iov.iov_base = ((char *)&svsk->sk_marker) + svsk->sk_tcplen;
10191046
iov.iov_len = want;
10201047
iov_iter_kvec(&msg.msg_iter, ITER_DEST, &iov, 1, want);
1021-
len = svc_tcp_sock_recv_cmsg(svsk, &msg);
1048+
len = svc_tcp_sock_recvmsg(svsk, &msg);
10221049
if (len < 0)
10231050
return len;
10241051
svsk->sk_tcplen += len;

0 commit comments

Comments
 (0)