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>
[bwh: Backported to 3.2:
 - Drop changes to hfi1
 - ipath_write() has the same problem, so add the same restriction there]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

diff --git a/drivers/infiniband/core/ucm.c b/drivers/infiniband/core/ucm.c
index f2f63933e8a97e889eb8a1cd8b5a5c0b8ec3f475..5befec118a18341960bb358c53be16e22a91e0bf 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>
@@ -1104,6 +1105,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 45d67e9228d75b44d71354d248cf97140dfe37ad..81dd84d0b68b545824788b9d7de99403b1befffe 100644 (file)
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -1487,6 +1487,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 8802d5ccd93d2812b81b87030256fdd6444641db..f3ecfe4b9571a441b5b169235bd7cf0687085dd5 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");
@@ -605,6 +607,9 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
        struct ib_uverbs_cmd_hdr hdr;
        __u32 flags;
 
+       if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
+               return -EACCES;
+
        if (count < sizeof hdr)
                return -EINVAL;
 

diff --git a/drivers/infiniband/hw/ipath/ipath_file_ops.c b/drivers/infiniband/hw/ipath/ipath_file_ops.c
index 6d7f453b4d05ef7da7f74aeafe22608b85dc00fc..a0626b8c61c51888c2c6101654f46fa1d0730ebb 100644 (file)
--- a/drivers/infiniband/hw/ipath/ipath_file_ops.c
+++ b/drivers/infiniband/hw/ipath/ipath_file_ops.c
@@ -45,6 +45,8 @@
 #include <linux/cpu.h>
 #include <asm/pgtable.h>
 
+#include <rdma/ib.h>
+
 #include "ipath_kernel.h"
 #include "ipath_common.h"
 #include "ipath_user_sdma.h"
@@ -2240,6 +2242,9 @@ static ssize_t ipath_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/infiniband/hw/qib/qib_file_ops.c b/drivers/infiniband/hw/qib/qib_file_ops.c
index b15e34eeef685d510c781d25d08433e4b3e7c717..3ab8229b1d8c9346bbd12b8ed604b65a0726ed02 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/delay.h>
 #include <linux/export.h>
 
+#include <rdma/ib.h>
+
 #include "qib.h"
 #include "qib_common.h"
 #include "qib_user_sdma.h"
@@ -2058,6 +2060,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/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 */