From d6e53ea318ddfa32e54532f447412911709a80e1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic@digikod.net>
Date: Tue, 4 Apr 2023 17:28:34 +0200
Subject: [PATCH] PATCH: Fix multiple issues in net.c

- Store ports in rulesets as __be16 to avoid runtime conversions.
- Constify arguments.
- Check address's length before dereferencing address to read the port.
- Fix and add comments.
- Format with clang-format.
---
 security/landlock/net.c | 55 ++++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 20 deletions(-)

diff --git a/security/landlock/net.c b/security/landlock/net.c
index e19c339906e7..663eee5920d5 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -22,9 +22,10 @@ int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
 {
 	int err;
 	const struct landlock_id id = {
-		.key.data = port,
+		.key.data = (__force uintptr_t)htons(port),
 		.type = LANDLOCK_KEY_NET_PORT,
 	};
+
 	BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data));
 
 	/* Transforms relative access rights to absolute ones. */
@@ -60,20 +61,24 @@ static const struct landlock_ruleset *get_current_net_domain(void)
 	return dom;
 }
 
-static int check_addrlen(const struct sockaddr *const address, int addrlen)
+static int check_addrlen(const struct sockaddr *const address,
+			 const int addrlen)
 {
 	if (addrlen < offsetofend(struct sockaddr, sa_family))
 		return -EINVAL;
+
 	switch (address->sa_family) {
 	case AF_UNSPEC:
 	case AF_INET:
 		if (addrlen < sizeof(struct sockaddr_in))
 			return -EINVAL;
+
 		return 0;
 #if IS_ENABLED(CONFIG_IPV6)
 	case AF_INET6:
 		if (addrlen < SIN6_LEN_RFC2133)
 			return -EINVAL;
+
 		return 0;
 #endif
 	}
@@ -81,7 +86,7 @@ static int check_addrlen(const struct sockaddr *const address, int addrlen)
 	return 0;
 }
 
-static u16 get_port(const struct sockaddr *const address)
+static __be16 get_port(const struct sockaddr *const address)
 {
 	/* Gets port value in host byte order. */
 	switch (address->sa_family) {
@@ -89,13 +94,13 @@ static u16 get_port(const struct sockaddr *const address)
 	case AF_INET: {
 		const struct sockaddr_in *const sockaddr =
 			(struct sockaddr_in *)address;
-		return ntohs(sockaddr->sin_port);
+		return sockaddr->sin_port;
 	}
 #if IS_ENABLED(CONFIG_IPV6)
 	case AF_INET6: {
 		const struct sockaddr_in6 *const sockaddr_ip6 =
 			(struct sockaddr_in6 *)address;
-		return ntohs(sockaddr_ip6->sin6_port);
+		return sockaddr_ip6->sin6_port;
 	}
 #endif
 	}
@@ -103,16 +108,18 @@ static u16 get_port(const struct sockaddr *const address)
 	return 0;
 }
 
-static int check_socket_access(struct socket *sock, struct sockaddr *address, int addrlen, u16 port,
-			       access_mask_t access_request)
+static int check_socket_access(struct socket *const sock,
+			       struct sockaddr *const address,
+			       const int addrlen,
+			       const access_mask_t access_request)
 {
-	int ret;
+	int err;
+	__be16 port;
 	bool allowed = false;
 	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_NET] = {};
 	const struct landlock_rule *rule;
 	access_mask_t handled_access;
-	const struct landlock_id id = {
-		.key.data = port,
+	struct landlock_id id = {
 		.type = LANDLOCK_KEY_NET_PORT,
 	};
 	const struct landlock_ruleset *const domain = get_current_net_domain();
@@ -121,13 +128,20 @@ static int check_socket_access(struct socket *sock, struct sockaddr *address, in
 		return 0;
 	if (WARN_ON_ONCE(domain->num_layers < 1))
 		return -EACCES;
-	/* Check if it's a TCP socket. */
+
+	/* Checks if it's a TCP socket. */
 	if (sock->type != SOCK_STREAM)
 		return 0;
 
-	ret = check_addrlen(address, addrlen);
-	if (ret)
-		return ret;
+	/* Checks for minimal header length. */
+	err = check_addrlen(address, addrlen);
+	if (err)
+		return err;
+
+	/* It is now safe to read the port. */
+	port = get_port(address);
+	id.key.data = (__force uintptr_t)port;
+	BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data));
 
 	switch (address->sa_family) {
 	case AF_UNSPEC:
@@ -172,17 +186,18 @@ static int check_socket_access(struct socket *sock, struct sockaddr *address, in
 	return allowed ? 0 : -EACCES;
 }
 
-static int hook_socket_bind(struct socket *sock, struct sockaddr *address,
-			    int addrlen)
+static int hook_socket_bind(struct socket *const sock,
+			    struct sockaddr *const address, const int addrlen)
 {
-	return check_socket_access(sock, address, addrlen, get_port(address),
+	return check_socket_access(sock, address, addrlen,
 				   LANDLOCK_ACCESS_NET_BIND_TCP);
 }
 
-static int hook_socket_connect(struct socket *sock, struct sockaddr *address,
-			       int addrlen)
+static int hook_socket_connect(struct socket *const sock,
+			       struct sockaddr *const address,
+			       const int addrlen)
 {
-	return check_socket_access(sock, address, addrlen, get_port(address),
+	return check_socket_access(sock, address, addrlen,
 				   LANDLOCK_ACCESS_NET_CONNECT_TCP);
 }
 
-- 
2.39.0