CVE-2016-4565: Permissions, Privileges, and Access Controls (CWE-264)
value: The InfiniBand (aka IB) stack in the Linux kernel before 4.5.3 incorrectly relies on the write system call, which allows local users to cause a denial of service (kernel memory write operation) or possibly have unspecified other impact via a uAPI interface.
CWE description: Weaknesses in this category are related to the management of permissions, privileges, and other security features that are used to perform access control.
CVSS v3 base score: 7.8
Published: 2016-05-23T10:59Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: drivers/infiniband/core/ucm.c, drivers/infiniband/core/ucma.c, drivers/infiniband/core/uverbs_main.c, drivers/infiniband/hw/qib/qib_file_ops.c, drivers/staging/rdma/hfi1/TODO, drivers/staging/rdma/hfi1/file_ops.c, include/rdma/ib.h
Commit date: 2016-04-28 12:03:16 -0400
Commit message: IB/security: Restrict use of the write() interface

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 ]
Cc: stable@vger.kernel.org
Signed-off-by: Doug Ledford <dledford@redhat.com>

Diff: diff --git a/drivers/infiniband/core/ucm.c b/drivers/infiniband/core/ucm.c
index 4a9aa0433b07f..7713ef089c3cc 100644
--- a/drivers/infiniband/core/ucm.c
+++ b/drivers/infiniband/core/ucm.c
@@ -46,10 +46,11 @@
 #include <linux/mutex.h>
 #include <linux/slab.h>
 
 #include <asm/uaccess.h>
 
+#include <rdma/ib.h>
 #include <rdma/ib_cm.h>
 #include <rdma/ib_user_cm.h>
 #include <rdma/ib_marshall.h>
 
 MODULE_AUTHOR("Libor Michalek");
@@ -1101,10 +1102,13 @@ static ssize_t ib_ucm_write(struct file *filp, const char __user *buf,
 {
 	struct ib_ucm_file *file = filp->private_data;
 	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;
 
 	if (copy_from_user(&hdr, buf, sizeof(hdr)))
 		return -EFAULT;
diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index dd3bcceadfdef..c0f3826abb30a 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -1572,10 +1572,13 @@ static ssize_t ucma_write(struct file *filp, const char __user *buf,
 {
 	struct ucma_file *file = filp->private_data;
 	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;
 
 	if (copy_from_user(&hdr, buf, sizeof(hdr)))
 		return -EFAULT;
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 28ba2cc815355..31f422a70623a 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ b/drivers/infiniband/core/uverbs_main.c
@@ -46,10 +46,12 @@
 #include <linux/anon_inodes.h>
 #include <linux/slab.h>
 
 #include <asm/uaccess.h>
 
+#include <rdma/ib.h>
+
 #include "uverbs.h"
 
 MODULE_AUTHOR("Roland Dreier");
 MODULE_DESCRIPTION("InfiniBand userspace verbs access");
 MODULE_LICENSE("Dual BSD/GPL");
@@ -707,10 +709,13 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
 	__u32 command;
 	__u32 flags;
 	int srcu_key;
 	ssize_t ret;
 
+	if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
+		return -EACCES;
+
 	if (count < sizeof hdr)
 		return -EINVAL;
 
 	if (copy_from_user(&hdr, buf, sizeof hdr))
 		return -EFAULT;
diff --git a/drivers/infiniband/hw/qib/qib_file_ops.c b/drivers/infiniband/hw/qib/qib_file_ops.c
index e449e394963f0..24f4a782e0f43 100644
--- a/drivers/infiniband/hw/qib/qib_file_ops.c
+++ b/drivers/infiniband/hw/qib/qib_file_ops.c
@@ -43,10 +43,12 @@
 #include <asm/pgtable.h>
 #include <linux/delay.h>
 #include <linux/export.h>
 #include <linux/uio.h>
 
+#include <rdma/ib.h>
+
 #include "qib.h"
 #include "qib_common.h"
 #include "qib_user_sdma.h"
 
 #undef pr_fmt
@@ -2065,10 +2067,13 @@ static ssize_t qib_write(struct file *fp, const char __user *data,
 	size_t consumed, copy = 0;
 	struct qib_cmd cmd;
 	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 05de0dad8762e..4c6f1d7d2eaf2 100644
--- a/drivers/staging/rdma/hfi1/TODO
+++ b/drivers/staging/rdma/hfi1/TODO
@@ -1,6 +1,6 @@
 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 5415295897366..c1c5bf82addb0 100644
--- a/drivers/staging/rdma/hfi1/file_ops.c
+++ b/drivers/staging/rdma/hfi1/file_ops.c
@@ -47,10 +47,12 @@
 #include <linux/poll.h>
 #include <linux/cdev.h>
 #include <linux/vmalloc.h>
 #include <linux/io.h>
 
+#include <rdma/ib.h>
+
 #include "hfi.h"
 #include "pio.h"
 #include "device.h"
 #include "common.h"
 #include "trace.h"
@@ -188,10 +190,14 @@ static ssize_t hfi1_file_write(struct file *fp, const char __user *data,
 	void *dest = NULL;
 	__u64 user_val = 0;
 	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 cf8f9e700e489..a6b93706b0fc9 100644
--- a/include/rdma/ib.h
+++ b/include/rdma/ib.h
@@ -32,10 +32,11 @@
 
 #if !defined(_RDMA_IB_H)
 #define _RDMA_IB_H
 
 #include <linux/types.h>
+#include <linux/sched.h>
 
 struct ib_addr {
 	union {
 		__u8		uib_addr8[16];
 		__be16		uib_addr16[8];
@@ -84,6 +85,21 @@ struct sockaddr_ib {
 	__be64			sib_sid;
 	__be64			sib_sid_mask;
 	__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 */

Fixed code: #include <rdma/ib.h>
	if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
		return -EACCES;

	if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
		return -EACCES;

#include <rdma/ib.h>

	if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
		return -EACCES;

#include <rdma/ib.h>

	if (WARN_ON_ONCE(!ib_safe_file_access(fp)))
		return -EACCES;

- Replace incorrect uAPI
#include <rdma/ib.h>

	/* FIXME: This interface cannot continue out of staging */
	if (WARN_ON_ONCE(!ib_safe_file_access(fp)))
		return -EACCES;

#include <linux/sched.h>
/*
 * 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);
}