Skip to contentSkip to conversation

Smack: IPv6 casting error fix

PATCH In review Casey Schaufler 2 participants 2 messages Jul 25, 2013

Subsystems (2)
Security (LSM) Smack security module
lore
Share & tools
lore

Casey Schaufler

author maintainer
#0 lore raw

PATCH 1/1 Smack: IPv6 casting error fix

Subject: [PATCH] Smack: IPv6 casting error fix

The original implementation of the Smack IPv6 port based local controls works most of the time using a sockaddr as a temporary variable, but not always as it overflows in some circumstances. The correct data is a sockaddr_in6. A struct sockaddr isn't as large as a struct sockaddr_in6. There would need to be casting one way or the other. This patch gets it the right way.

This problem required some effort to make occur in development with 3.10, but hits every time in 3.11. This patch should go into 3.11.

Signed-off-by Casey Schaufler <[email protected]>
---
 security/smack/smack_lsm.c |   24 +++++++++++-------------
 1 files changed, 11 insertions(+), 13 deletions(-)
security/smack/smack_lsm.c +11 -13
index 3f7682a..eefbd10 100644
@@ -1998,12 +1998,11 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address)
  *
  * Create or update the port list entry
  */
-static int smk_ipv6_port_check(struct sock *sk, struct sockaddr *address,
+static int smk_ipv6_port_check(struct sock *sk, struct sockaddr_in6 *address,
 				int act)
 {
 	__be16 *bep;
 	__be32 *be32p;
-	struct sockaddr_in6 *addr6;
 	struct smk_port_label *spp;
 	struct socket_smack *ssp = sk->sk_security;
 	struct smack_known *skp;

@@ -2025,10 +2024,9 @@ static int smk_ipv6_port_check(struct sock *sk, struct sockaddr *address,
 	/*
 	 * Get the IP address and port from the address.
 	 */
-	addr6 = (struct sockaddr_in6 *)address;
-	port = ntohs(addr6->sin6_port);
-	bep = (__be16 *)(&addr6->sin6_addr);
-	be32p = (__be32 *)(&addr6->sin6_addr);
+	port = ntohs(address->sin6_port);
+	bep = (__be16 *)(&address->sin6_addr);
+	be32p = (__be32 *)(&address->sin6_addr);
 
 	/*
 	 * It's remote, so port lookup does no good.

@@ -2060,9 +2058,9 @@ auditout:
 	ad.a.u.net->family = sk->sk_family;
 	ad.a.u.net->dport = port;
 	if (act == SMK_RECEIVING)
-		ad.a.u.net->v6info.saddr = addr6->sin6_addr;
+		ad.a.u.net->v6info.saddr = address->sin6_addr;
 	else
-		ad.a.u.net->v6info.daddr = addr6->sin6_addr;
+		ad.a.u.net->v6info.daddr = address->sin6_addr;
 #endif
 	return smk_access(skp, object, MAY_WRITE, &ad);
 }

@@ -2201,7 +2199,8 @@ static int smack_socket_connect(struct socket *sock, struct sockaddr *sap,
 	case PF_INET6:
 		if (addrlen < sizeof(struct sockaddr_in6))
 			return -EINVAL;
-		rc = smk_ipv6_port_check(sock->sk, sap, SMK_CONNECTING);
+		rc = smk_ipv6_port_check(sock->sk, (struct sockaddr_in6 *)sap,
+						SMK_CONNECTING);
 		break;
 	}
 	return rc;

@@ -3034,7 +3033,7 @@ static int smack_socket_sendmsg(struct socket *sock, struct msghdr *msg,
 				int size)
 {
 	struct sockaddr_in *sip = (struct sockaddr_in *) msg->msg_name;
-	struct sockaddr *sap = (struct sockaddr *) msg->msg_name;
+	struct sockaddr_in6 *sap = (struct sockaddr_in6 *) msg->msg_name;
 	int rc = 0;
 
 	/*

@@ -3121,9 +3120,8 @@ static struct smack_known *smack_from_secattr(struct netlbl_lsm_secattr *sap,
 	return smack_net_ambient;
 }
 
-static int smk_skb_to_addr_ipv6(struct sk_buff *skb, struct sockaddr *sap)
+static int smk_skb_to_addr_ipv6(struct sk_buff *skb, struct sockaddr_in6 *sip)
 {
-	struct sockaddr_in6 *sip = (struct sockaddr_in6 *)sap;
 	u8 nexthdr;
 	int offset;
 	int proto = -EINVAL;

@@ -3181,7 +3179,7 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 	struct netlbl_lsm_secattr secattr;
 	struct socket_smack *ssp = sk->sk_security;
 	struct smack_known *skp;
-	struct sockaddr sadd;
+	struct sockaddr_in6 sadd;
 	int rc = 0;
 	struct smk_audit_info ad;
 #ifdef CONFIG_AUDIT
#1 lore raw
Quoted 65 lines · Casey Schaufler

Subject: [PATCH] Smack: IPv6 casting error fix

The original implementation of the Smack IPv6 port based local controls works most of the time using a sockaddr as a temporary variable, but not always as it overflows in some circumstances. The correct data is a sockaddr_in6. A struct sockaddr isn't as large as a struct sockaddr_in6. There would need to be casting one way or the other. This patch gets it the right way.

This problem required some effort to make occur in development with 3.10, but hits every time in 3.11. This patch should go into 3.11.

Signed-off-by Casey Schaufler <[email protected]>
---
 security/smack/smack_lsm.c |   24 +++++++++++-------------
 1 files changed, 11 insertions(+), 13 deletions(-)
security/smack/smack_lsm.c +6 -8
index 3f7682a..eefbd10 100644
@@ -1998,12 +1998,11 @@ static void smk_ipv6_port_label(struct socket *sock, struct sockaddr *address)
  *
  * Create or update the port list entry
  */
-static int smk_ipv6_port_check(struct sock *sk, struct sockaddr *address,
+static int smk_ipv6_port_check(struct sock *sk, struct sockaddr_in6 *address,
                                int act)
 {
        __be16 *bep;
        __be32 *be32p;
-       struct sockaddr_in6 *addr6;
        struct smk_port_label *spp;
        struct socket_smack *ssp = sk->sk_security;
        struct smack_known *skp;

@@ -2025,10 +2024,9 @@ static int smk_ipv6_port_check(struct sock *sk, struct sockaddr *address,
        /*
         * Get the IP address and port from the address.
         */
-       addr6 = (struct sockaddr_in6 *)address;
-       port = ntohs(addr6->sin6_port);
-       bep = (__be16 *)(&addr6->sin6_addr);
-       be32p = (__be32 *)(&addr6->sin6_addr);
+       port = ntohs(address->sin6_port);
+       bep = (__be16 *)(&address->sin6_addr);
+       be32p = (__be32 *)(&address->sin6_addr);
 
        /*
         * It's remote, so port lookup does no good.

@@ -2060,9 +2058,9 @@ auditout:
        ad.a.u.net->family = sk->sk_family;
        ad.a.u.net->dport = port;
        if (act == SMK_RECEIVING)
-               ad.a.u.net->v6info.saddr = addr6->sin6_addr;
+               ad.a.u.net->v6info.saddr = address->sin6_addr;
        else
-               ad.a.u.net->v6info.daddr = addr6->sin6_addr;
+               ad.a.u.net->v6info.daddr = address->sin6_addr;
 #endif
        return smk_access(skp, object, MAY_WRITE, &ad);
 }

security/smack/smack_lsm.c static int smack_socket_connect(struct socket *sock, struct sockaddr *sap, · line 2202

Quoted 5 lines
security/smack/smack_lsm.c +1 -1
@@ -2201,7 +2199,8 @@ static int smack_socket_connect(struct socket *sock, struct sockaddr *sap,
        case PF_INET6:
                if (addrlen < sizeof(struct sockaddr_in6))
                        return -EINVAL;
-               rc = smk_ipv6_port_check(sock->sk, sap, SMK_CONNECTING);
+               rc = smk_ipv6_port_check(sock->sk, (struct sockaddr_in6 *)sap,

I reviewed ipv4 and ipv6 codes at smack. the variable 'sap' is used for both 'struct sockaddr' and 'struct sockaddr_in6'. It's very confusing to know what's the exact usage. so I suggest to use separate variable for each cases.

as you know, the sizeof(struct sockaddr_in6) is bigger than sizeof(struct sockaddr). now it valid cast it as above. but other case it corrupts the stack.

security/smack/smack_lsm.c static int smack_socket_sendmsg(struct socket *sock, struct msghdr *msg, · line 3036

Quoted 10 lines
security/smack/smack_lsm.c +2 -1
+                                               SMK_CONNECTING);
                break;
        }
        return rc;

@@ -3034,7 +3033,7 @@ static int smack_socket_sendmsg(struct socket *sock, struct msghdr *msg,
                                int size)
 {
        struct sockaddr_in *sip = (struct sockaddr_in *) msg->msg_name;
-       struct sockaddr *sap = (struct sockaddr *) msg->msg_name;
+       struct sockaddr_in6 *sap = (struct sockaddr_in6 *) msg->msg_name;

sap is used for struct sockaddr_in6.

security/smack/smack_lsm.c static struct smack_known *smack_from_secattr(struct netlbl_lsm_secattr *sap, · line 3123

Quoted 9 lines
security/smack/smack_lsm.c +1 -1
        int rc = 0;
 
        /*

@@ -3121,9 +3120,8 @@ static struct smack_known *smack_from_secattr(struct netlbl_lsm_secattr *sap,
        return smack_net_ambient;
 }
 
-static int smk_skb_to_addr_ipv6(struct sk_buff *skb, struct sockaddr *sap)
+static int smk_skb_to_addr_ipv6(struct sk_buff *skb, struct sockaddr_in6 *sip)

'sip' is used for struct sockaddr_in6.

security/smack/smack_lsm.c static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb) · line 3182

Quoted 11 lines
security/smack/smack_lsm.c +1 -2
 {
-       struct sockaddr_in6 *sip = (struct sockaddr_in6 *)sap;
        u8 nexthdr;
        int offset;
        int proto = -EINVAL;

@@ -3181,7 +3179,7 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
        struct netlbl_lsm_secattr secattr;
        struct socket_smack *ssp = sk->sk_security;
        struct smack_known *skp;
-       struct sockaddr sadd;
+       struct sockaddr_in6 sadd;

'sadd' is used for struct sockaddr_in6.

Thank you,
Kyungmin Park

Quoted 9 lines
security/smack/smack_lsm.c +0 -1
        int rc = 0;
        struct smk_audit_info ad;
 #ifdef CONFIG_AUDIT
 
--

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/