[PATCH 1/5] SUNRPC: Separate the TLS control-record receive from its policy

Chuck Lever <[email protected]>
Newsgroups org.kernel.vger.linux-nfs,org.kernel.vger.netdev
Message-ID <[email protected]>
svc_tcp_sock_recv_cmsg() receives the record at the head of the kTLS
receive queue and decides what its content type means for the
transport. The receive and the decision are one step, so a caller
cannot learn a record's type without also acting on it. A later
caller classifies the head of the queue with MSG_PEEK before it
decides whether to consume the record. It needs the receive without
the decision.

Move the receive into svc_tcp_recv_cmsg(), which takes the recvmsg()
flags and reports the octet count, the record type, and the message
flags. The alert policy stays in svc_tcp_sock_recv_cmsg(), the
helper's only caller in this patch.

The zeroed control buffer replaces the msg_controllen check. An
unfilled buffer reports record type zero, so a positive octet count
with no record type now returns -EBADMSG instead of a count. The
alert parse runs on a second msghdr over the alert buffer, so the
copy and the parse share no iterator state. The rewind that commit
bee47cb026e7 ("sunrpc: fix handling of server side tls alerts")
added by hand is no longer needed.

Signed-off-by: Chuck Lever <[email protected]>
---
 net/sunrpc/svcsock.c | 144 ++++++++++++++++++++++-----------------------------
 1 file changed, 63 insertions(+), 81 deletions(-)

diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 5a2d52284d75..b402923c40f1 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -271,97 +271,79 @@ svc_tcp_sock_drain_record(struct socket *sock)
 	}
 }
 
-static int
-svc_tcp_sock_process_cmsg(struct socket *sock, struct msghdr *msg,
-			  struct cmsghdr *cmsg, int ret)
-{
-	u8 content_type = tls_get_record_type(sock->sk, cmsg);
-	u8 level, description;
-
-	switch (content_type) {
-	case 0:
-		break;
-	case TLS_RECORD_TYPE_DATA:
-		/* TLS sets EOR at the end of each application data
-		 * record, even though there might be more frames
-		 * waiting to be decrypted.
-		 */
-		msg->msg_flags &= ~MSG_EOR;
-		break;
-	case TLS_RECORD_TYPE_ALERT:
-		tls_alert_recv(sock->sk, msg, &level, &description);
-		/* RFC 8446 Section 6: every alert but a closure alert is
-		 * an error alert.
-		 */
-		switch (description) {
-		case TLS_ALERT_DESC_CLOSE_NOTIFY:
-		case TLS_ALERT_DESC_USER_CANCELED:
-			ret = -EAGAIN;
-			break;
-		default:
-			ret = -ENOTCONN;
-		}
-		break;
-	default:
-		/* discard this record type */
-		ret = -EAGAIN;
-	}
-	return ret;
-}
-
-static int
-svc_tcp_sock_recv_cmsg(struct socket *sock, unsigned int *msg_flags)
+static int svc_tcp_recv_cmsg(struct socket *sock, int flags,
+			     struct kvec *payload, u8 *type,
+			     unsigned int *msg_flags)
 {
 	union {
 		struct cmsghdr	cmsg;
 		u8		buf[CMSG_SPACE(sizeof(u8))];
-	} u;
-	u8 alert[2];
-	struct kvec alert_kvec = {
-		.iov_base = alert,
-		.iov_len = sizeof(alert),
-	};
+	} u = {};
 	struct msghdr msg = {
-		.msg_flags = *msg_flags,
-		.msg_control = &u,
-		.msg_controllen = sizeof(u),
+		.msg_control	= &u,
+		.msg_controllen	= sizeof(u),
 	};
 	int ret;
 
-	iov_iter_kvec(&msg.msg_iter, ITER_DEST, &alert_kvec, 1,
-		      alert_kvec.iov_len);
-	ret = sock_recvmsg(sock, &msg, MSG_DONTWAIT);
-	/* put_cmsg() shrinks msg_controllen, so a short one means
-	 * kTLS filled in u.cmsg.
-	 */
-	if (ret >= 0 && msg.msg_controllen < sizeof(u)) {
-		u8 content_type = tls_get_record_type(sock->sk, &u.cmsg);
-
-		/* Returning the count would credit the RPC stream with
-		 * octets that never reached the caller's buffer.
-		 */
-		if (content_type != TLS_RECORD_TYPE_ALERT) {
-			/* An application data record carries RPC payload.
-			 * Draining one breaks RPC fragment framing.
-			 */
-			if (content_type != TLS_RECORD_TYPE_DATA &&
-			    !(msg.msg_flags & MSG_EOR))
-				svc_tcp_sock_drain_record(sock);
-			return -EAGAIN;
-		}
-		/* An Alert record carries exactly one two-octet message
-		 * (RFC 8446 Section 5.1). alert_kvec caps the receive at two,
-		 * so a longer record produces the same count. MSG_EOR appears
-		 * only once kTLS has drained the whole record.
-		 */
-		if (ret != sizeof(alert) || !(msg.msg_flags & MSG_EOR))
-			return -EBADMSG;
-		iov_iter_revert(&msg.msg_iter, ret);
-		ret = svc_tcp_sock_process_cmsg(sock, &msg, &u.cmsg, -EAGAIN);
-	}
+	iov_iter_kvec(&msg.msg_iter, ITER_DEST, payload, 1, payload->iov_len);
+	ret = sock_recvmsg(sock, &msg, flags);
+	if (ret < 0)
+		return ret;
+	*msg_flags = msg.msg_flags;
+	*type = tls_get_record_type(sock->sk, &u.cmsg);
+	if (!*type && ret)
+		return -EBADMSG;
 	return ret;
 }
 
+static int
+svc_tcp_sock_recv_cmsg(struct socket *sock)
+{
+	u8 alert[2], type, level, description;
+	struct kvec recv_kvec = {
+		.iov_base	= alert,
+		.iov_len	= sizeof(alert),
+	};
+	unsigned int msg_flags;
+	struct msghdr msg = {};
+	int ret;
+
+	ret = svc_tcp_recv_cmsg(sock, MSG_DONTWAIT, &recv_kvec, &type,
+				&msg_flags);
+	if (ret < 0 || !type)
+		return ret;
+	if (type != TLS_RECORD_TYPE_ALERT) {
+		/* An application data record carries RPC payload.
+		 * Draining one breaks RPC fragment framing.
+		 */
+		if (type != TLS_RECORD_TYPE_DATA && !(msg_flags & MSG_EOR))
+			svc_tcp_sock_drain_record(sock);
+		return -EAGAIN;
+	}
+	/* An Alert record carries exactly one two-octet message (RFC
+	 * 8446 Section 5.1). recv_kvec caps the receive at two, so a
+	 * longer record produces the same count. MSG_EOR appears only
+	 * once kTLS has drained the whole record.
+	 */
+	if (ret != sizeof(alert) || !(msg_flags & MSG_EOR))
+		return -EBADMSG;
+
+	iov_iter_kvec(&msg.msg_iter, ITER_DEST, &recv_kvec, 1,
+		      recv_kvec.iov_len);
+	tls_alert_recv(sock->sk, &msg, &level, &description);
+
+	/* RFC 8446 Section 6: every alert but a closure alert is
+	 * an error alert.
+	 */
+	switch (description) {
+	case TLS_ALERT_DESC_CLOSE_NOTIFY:
+	case TLS_ALERT_DESC_USER_CANCELED:
+		return -EAGAIN;
+	default:
+		return -ENOTCONN;
+	}
+}
+
 static int
 svc_tcp_sock_recvmsg(struct svc_sock *svsk, struct msghdr *msg)
 {
@@ -372,7 +354,7 @@ svc_tcp_sock_recvmsg(struct svc_sock *svsk, struct msghdr *msg)
 	if (msg->msg_flags & MSG_CTRUNC) {
 		msg->msg_flags &= ~(MSG_CTRUNC | MSG_EOR);
 		if (ret == 0 || ret == -EIO) {
-			ret = svc_tcp_sock_recv_cmsg(sock, &msg->msg_flags);
+			ret = svc_tcp_sock_recv_cmsg(sock);
 			/* A control record delivers nothing to the caller,
 			 * and kTLS announces no data_ready for records it
 			 * already holds. Mark the transport ready so that

-- 
2.54.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.