lsm: make security_socket_getpeersec_stream() sockptr_t safe

[ Upstream commit b10b9c342f7571f287fd422be5d5c0beb26ba974 ]

Commit 4ff09db1b79b ("bpf: net: Change sk_getsockopt() to take the
sockptr_t argument") made it possible to call sk_getsockopt()
with both user and kernel address space buffers through the use of
the sockptr_t type.  Unfortunately at the time of conversion the
security_socket_getpeersec_stream() LSM hook was written to only
accept userspace buffers, and in a desire to avoid having to change
the LSM hook the commit author simply passed the sockptr_t's
userspace buffer pointer.  Since the only sk_getsockopt() callers
at the time of conversion which used kernel sockptr_t buffers did
not allow SO_PEERSEC, and hence the
security_socket_getpeersec_stream() hook, this was acceptable but
also very fragile as future changes presented the possibility of
silently passing kernel space pointers to the LSM hook.

There are several ways to protect against this, including careful
code review of future commits, but since relying on code review to
catch bugs is a recipe for disaster and the upstream eBPF maintainer
is "strongly against defensive programming", this patch updates the
LSM hook, and all of the implementations to support sockptr_t and
safely handle both user and kernel space buffers.

Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Stable-dep-of: 5a287d3d2b9d ("lsm: fix default return value of the socket_getpeersec_*() hooks")
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 92a76ce0c382d9f278773aeabaef4f525fa5f091..9f550eab8ebdb9ba052d4de0d5e9f5d2842b4892 100644 (file)
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -294,7 +294,7 @@ LSM_HOOK(int, 0, socket_setsockopt, struct socket *sock, int level, int optname)
 LSM_HOOK(int, 0, socket_shutdown, struct socket *sock, int how)
 LSM_HOOK(int, 0, socket_sock_rcv_skb, struct sock *sk, struct sk_buff *skb)
 LSM_HOOK(int, 0, socket_getpeersec_stream, struct socket *sock,
-        char __user *optval, int __user *optlen, unsigned len)
+        sockptr_t optval, sockptr_t optlen, unsigned int len)
 LSM_HOOK(int, 0, socket_getpeersec_dgram, struct socket *sock,
         struct sk_buff *skb, u32 *secid)
 LSM_HOOK(int, 0, sk_alloc_security, struct sock *sk, int family, gfp_t priority)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 64cdf4d7bfb30b2c6363e56d02ef5c9d6b2a5c53..bbf9c8c7bd9c5bb2e394c2dca18f48be018bc35f 100644 (file)
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -926,8 +926,8 @@
  *     SO_GETPEERSEC.  For tcp sockets this can be meaningful if the
  *     socket is associated with an ipsec SA.
  *     @sock is the local socket.
- *     @optval userspace memory where the security state is to be copied.
- *     @optlen userspace int where the module should copy the actual length
+ *     @optval memory where the security state is to be copied.
+ *     @optlen memory where the module should copy the actual length
  *     of the security state.
  *     @len as input is the maximum length to copy to userspace provided
  *     by the caller.

diff --git a/include/linux/security.h b/include/linux/security.h
index e388b1666bcfc345c70c43baee7ec23344ab09cc..5b61aa19fac660f9679d00c7631f54ab63043f9e 100644 (file)
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -31,6 +31,7 @@
 #include <linux/err.h>
 #include <linux/string.h>
 #include <linux/mm.h>
+#include <linux/sockptr.h>
 
 struct linux_binprm;
 struct cred;
@@ -1366,8 +1367,8 @@ int security_socket_getsockopt(struct socket *sock, int level, int optname);
 int security_socket_setsockopt(struct socket *sock, int level, int optname);
 int security_socket_shutdown(struct socket *sock, int how);
 int security_sock_rcv_skb(struct sock *sk, struct sk_buff *skb);
-int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
-                                     int __user *optlen, unsigned len);
+int security_socket_getpeersec_stream(struct socket *sock, sockptr_t optval,
+                                     sockptr_t optlen, unsigned int len);
 int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid);
 int security_sk_alloc(struct sock *sk, int family, gfp_t priority);
 void security_sk_free(struct sock *sk);
@@ -1501,8 +1502,10 @@ static inline int security_sock_rcv_skb(struct sock *sk,
        return 0;
 }
 
-static inline int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
-                                                   int __user *optlen, unsigned len)
+static inline int security_socket_getpeersec_stream(struct socket *sock,
+                                                   sockptr_t optval,
+                                                   sockptr_t optlen,
+                                                   unsigned int len)
 {
        return -ENOPROTOOPT;
 }

diff --git a/net/core/sock.c b/net/core/sock.c
index 42da46965b16f6294a55a0766a30ba1f9d32eda8..016c0b9e01b70f42f7f420dc81c6d2f3de062817 100644 (file)
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1503,7 +1503,8 @@ static int sk_getsockopt(struct sock *sk, int level, int optname,
                break;
 
        case SO_PEERSEC:
-               return security_socket_getpeersec_stream(sock, optval.user, optlen.user, len);
+               return security_socket_getpeersec_stream(sock,
+                                                        optval, optlen, len);
 
        case SO_MARK:
                v.val = sk->sk_mark;

diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 585edcc6814d2fe1674dc9d8cfc6bbfb3f7c2d9a..052f1b920e43fe745f2e06cb757418fe78c9cd9c 100644 (file)
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1070,11 +1070,10 @@ static struct aa_label *sk_peer_label(struct sock *sk)
  * Note: for tcp only valid if using ipsec or cipso on lan
  */
 static int apparmor_socket_getpeersec_stream(struct socket *sock,
-                                            char __user *optval,
-                                            int __user *optlen,
+                                            sockptr_t optval, sockptr_t optlen,
                                             unsigned int len)
 {
-       char *name;
+       char *name = NULL;
        int slen, error = 0;
        struct aa_label *label;
        struct aa_label *peer;
@@ -1091,23 +1090,21 @@ static int apparmor_socket_getpeersec_stream(struct socket *sock,
        /* don't include terminating \0 in slen, it breaks some apps */
        if (slen < 0) {
                error = -ENOMEM;
-       } else {
-               if (slen > len) {
-                       error = -ERANGE;
-               } else if (copy_to_user(optval, name, slen)) {
-                       error = -EFAULT;
-                       goto out;
-               }
-               if (put_user(slen, optlen))
-                       error = -EFAULT;
-out:
-               kfree(name);
-
+               goto done;
+       }
+       if (slen > len) {
+               error = -ERANGE;
+               goto done_len;
        }
 
+       if (copy_to_sockptr(optval, name, slen))
+               error = -EFAULT;
+done_len:
+       if (copy_to_sockptr(optlen, &slen, sizeof(slen)))
+               error = -EFAULT;
 done:
        end_current_label_crit_section(label);
-
+       kfree(name);
        return error;
 }
 

diff --git a/security/security.c b/security/security.c
index 269c3965393f474ea427067105f2b0f24b2fb8e8..e9dcde3c4f14b331ad650b720f5c44bffec9ee73 100644 (file)
--- a/security/security.c
+++ b/security/security.c
@@ -2224,11 +2224,11 @@ int security_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 }
 EXPORT_SYMBOL(security_sock_rcv_skb);
 
-int security_socket_getpeersec_stream(struct socket *sock, char __user *optval,
-                                     int __user *optlen, unsigned len)
+int security_socket_getpeersec_stream(struct socket *sock, sockptr_t optval,
+                                     sockptr_t optlen, unsigned int len)
 {
        return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock,
-                               optval, optlen, len);
+                            optval, optlen, len);
 }
 
 int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 50d3ddfe15fd18f00799444feeb63895784cb7b6..46c00a68bb4bd7f580bac35f032607997641214b 100644 (file)
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5110,11 +5110,12 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
        return err;
 }
 
-static int selinux_socket_getpeersec_stream(struct socket *sock, char __user *optval,
-                                           int __user *optlen, unsigned len)
+static int selinux_socket_getpeersec_stream(struct socket *sock,
+                                           sockptr_t optval, sockptr_t optlen,
+                                           unsigned int len)
 {
        int err = 0;
-       char *scontext;
+       char *scontext = NULL;
        u32 scontext_len;
        struct sk_security_struct *sksec = sock->sk->sk_security;
        u32 peer_sid = SECSID_NULL;
@@ -5130,17 +5131,15 @@ static int selinux_socket_getpeersec_stream(struct socket *sock, char __user *op
                                      &scontext_len);
        if (err)
                return err;
-
        if (scontext_len > len) {
                err = -ERANGE;
                goto out_len;
        }
 
-       if (copy_to_user(optval, scontext, scontext_len))
+       if (copy_to_sockptr(optval, scontext, scontext_len))
                err = -EFAULT;
-
 out_len:
-       if (put_user(scontext_len, optlen))
+       if (copy_to_sockptr(optlen, &scontext_len, sizeof(scontext_len)))
                err = -EFAULT;
        kfree(scontext);
        return err;

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index e1669759403a6f0ddbb16bf1c562b260644019a6..5388f143eecd861f2070b55863421d681aa9de41 100644 (file)
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4022,12 +4022,12 @@ static int smack_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
  * returns zero on success, an error code otherwise
  */
 static int smack_socket_getpeersec_stream(struct socket *sock,
-                                         char __user *optval,
-                                         int __user *optlen, unsigned len)
+                                         sockptr_t optval, sockptr_t optlen,
+                                         unsigned int len)
 {
        struct socket_smack *ssp;
        char *rcp = "";
-       int slen = 1;
+       u32 slen = 1;
        int rc = 0;
 
        ssp = sock->sk->sk_security;
@@ -4035,15 +4035,16 @@ static int smack_socket_getpeersec_stream(struct socket *sock,
                rcp = ssp->smk_packet->smk_known;
                slen = strlen(rcp) + 1;
        }
-
-       if (slen > len)
+       if (slen > len) {
                rc = -ERANGE;
-       else if (copy_to_user(optval, rcp, slen) != 0)
-               rc = -EFAULT;
+               goto out_len;
+       }
 
-       if (put_user(slen, optlen) != 0)
+       if (copy_to_sockptr(optval, rcp, slen))
+               rc = -EFAULT;
+out_len:
+       if (copy_to_sockptr(optlen, &slen, sizeof(slen)))
                rc = -EFAULT;
-
        return rc;
 }