IB/security: Restrict use of the write() interface

commit e6bd18f57aad1a2d1ef40e646d03ed0f2515c9e3 upstream.

The drivers/infiniband stack uses write() as a replacement for
bi-directional ioctl().  This is not safe. There are ways to
trigger write calls that result in the return structure that
is normally written to user space being shunted off to user
specified kernel memory instead.

For the immediate repair, detect and deny suspicious accesses to
the write API.

For long term, update the user space libraries and the kernel API
to something that doesn't present the same security vulnerabilities
(likely a structured ioctl() interface).

The impacted uAPI interfaces are generally only available if
hardware from drivers/infiniband is installed in the system.

Reported-by: Jann Horn <jann@thejh.net>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
[ Expanded check to all known write() entry points ]
Signed-off-by: Doug Ledford <dledford@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/infiniband/core/ucm.c b/drivers/infiniband/core/ucm.c
index 6b4e8a008bc0418f3ad54728b73e03e6e8d6e1fa..564adf3116e875ba3a599959b1c37db77333e8a1 100644 (file)
--- a/drivers/infiniband/core/ucm.c
+++ b/drivers/infiniband/core/ucm.c
@@ -48,6 +48,7 @@
 
 #include <asm/uaccess.h>
 
+#include <rdma/ib.h>
 #include <rdma/ib_cm.h>
 #include <rdma/ib_user_cm.h>
 #include <rdma/ib_marshall.h>
@@ -1103,6 +1104,9 @@ static ssize_t ib_ucm_write(struct file *filp, const char __user *buf,
        struct ib_ucm_cmd_hdr hdr;
        ssize_t result;
 
+       if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
+               return -EACCES;
+
        if (len < sizeof(hdr))
                return -EINVAL;
 

diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index 8b5a934e1133d80b42e12d1790b3672d73e4a779..886f61ea6cc7d2f58e8d8c739c511ccd7dc78df7 100644 (file)
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -1574,6 +1574,9 @@ static ssize_t ucma_write(struct file *filp, const char __user *buf,
        struct rdma_ucm_cmd_hdr hdr;
        ssize_t ret;
 
+       if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
+               return -EACCES;
+
        if (len < sizeof(hdr))
                return -EINVAL;
 

diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index e3ef28861be63dd453d0fd1b55ec21f939919624..24f3ca2c4ad761bf639eb2933c30387c25377c21 100644 (file)
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -48,6 +48,8 @@
 
 #include <asm/uaccess.h>
 
+#include <rdma/ib.h>
+
 #include "uverbs.h"
 
 MODULE_AUTHOR("Roland Dreier");
@@ -682,6 +684,9 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
        int srcu_key;
        ssize_t ret;
 
+       if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
+               return -EACCES;
+
        if (count < sizeof hdr)
                return -EINVAL;
 

diff --git a/drivers/infiniband/hw/qib/qib_file_ops.c b/drivers/infiniband/hw/qib/qib_file_ops.c
index e449e394963f00d42cd11ecafbca6081f9011bcd..24f4a782e0f431282bb8f79fc496a0b998a7a55a 100644 (file)
--- a/drivers/infiniband/hw/qib/qib_file_ops.c
+++ b/drivers/infiniband/hw/qib/qib_file_ops.c
@@ -45,6 +45,8 @@
 #include <linux/export.h>
 #include <linux/uio.h>
 
+#include <rdma/ib.h>
+
 #include "qib.h"
 #include "qib_common.h"
 #include "qib_user_sdma.h"
@@ -2067,6 +2069,9 @@ static ssize_t qib_write(struct file *fp, const char __user *data,
        ssize_t ret = 0;
        void *dest;
 
+       if (WARN_ON_ONCE(!ib_safe_file_access(fp)))
+               return -EACCES;
+
        if (count < sizeof(cmd.type)) {
                ret = -EINVAL;
                goto bail;

diff --git a/drivers/staging/rdma/hfi1/TODO b/drivers/staging/rdma/hfi1/TODO
index 05de0dad8762eb4332e36ec100c072f4e6ce4ea3..4c6f1d7d2eaf21d17bffa1cc2f0f624052e16f22 100644 (file)
--- a/drivers/staging/rdma/hfi1/TODO
+++ b/drivers/staging/rdma/hfi1/TODO
@@ -3,4 +3,4 @@ July, 2015
 - Remove unneeded file entries in sysfs
 - Remove software processing of IB protocol and place in library for use
   by qib, ipath (if still present), hfi1, and eventually soft-roce
-
+- Replace incorrect uAPI

diff --git a/drivers/staging/rdma/hfi1/file_ops.c b/drivers/staging/rdma/hfi1/file_ops.c
index aae9826ec62bea8e86a6f1f7c4408ff574d72204..c851e51b1dc3b6d306ac4dde117e9f6fe07be73e 100644 (file)
--- a/drivers/staging/rdma/hfi1/file_ops.c
+++ b/drivers/staging/rdma/hfi1/file_ops.c
@@ -62,6 +62,8 @@
 #include <linux/cred.h>
 #include <linux/uio.h>
 
+#include <rdma/ib.h>
+
 #include "hfi.h"
 #include "pio.h"
 #include "device.h"
@@ -214,6 +216,10 @@ static ssize_t hfi1_file_write(struct file *fp, const char __user *data,
        int uctxt_required = 1;
        int must_be_root = 0;
 
+       /* FIXME: This interface cannot continue out of staging */
+       if (WARN_ON_ONCE(!ib_safe_file_access(fp)))
+               return -EACCES;
+
        if (count < sizeof(cmd)) {
                ret = -EINVAL;
                goto bail;

diff --git a/include/rdma/ib.h b/include/rdma/ib.h
index cf8f9e700e48939b7633a97df7f815486e0fb8a8..a6b93706b0fc96494d7de3d7408c26fb57436a02 100644 (file)
--- a/include/rdma/ib.h
+++ b/include/rdma/ib.h
@@ -34,6 +34,7 @@
 #define _RDMA_IB_H
 
 #include <linux/types.h>
+#include <linux/sched.h>
 
 struct ib_addr {
        union {
@@ -86,4 +87,19 @@ struct sockaddr_ib {
        __u64                   sib_scope_id;
 };
 
+/*
+ * The IB interfaces that use write() as bi-directional ioctl() are
+ * fundamentally unsafe, since there are lots of ways to trigger "write()"
+ * calls from various contexts with elevated privileges. That includes the
+ * traditional suid executable error message writes, but also various kernel
+ * interfaces that can write to file descriptors.
+ *
+ * This function provides protection for the legacy API by restricting the
+ * calling context.
+ */
+static inline bool ib_safe_file_access(struct file *filp)
+{
+       return filp->f_cred == current_cred() && segment_eq(get_fs(), USER_DS);
+}
+
 #endif /* _RDMA_IB_H */