Patch "IB/security: Restrict use of the write() interface" has been added to the 4.5-stable tree

PATCH [email protected] 1 participant 1 message May 2, 2016

Subsystems (3)

Stable branch Staging InfiniBand

lore

Share & tools

lore

[email protected]

author maintainer Applied

2016-05-02 00:02 UTC #0 lore raw

This is a note to let you know that I've just added the patch titled

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

to the 4.5-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
ib-security-restrict-use-of-the-write-interface.patch
and it can be found in the queue-4.5 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From e6bd18f57aad1a2d1ef40e646d03ed0f2515c9e3 Mon Sep 17 00:00:00 2001

From: Jason Gunthorpe <[email protected]>
Date: Sun, 10 Apr 2016 19:13:13 -0600
Subject: IB/security: Restrict use of the write() interface

From: Jason Gunthorpe <[email protected]>

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 <[email protected]>

Signed-off-by Linus Torvalds <[email protected]>

Signed-off-by Jason Gunthorpe <[email protected]>

[ Expanded check to all known write() entry points ]

Signed-off-by Doug Ledford <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 drivers/infiniband/core/ucm.c            |    4 ++++
 drivers/infiniband/core/ucma.c           |    3 +++
 drivers/infiniband/core/uverbs_main.c    |    5 +++++
 drivers/infiniband/hw/qib/qib_file_ops.c |    5 +++++
 drivers/staging/rdma/hfi1/TODO           |    2 +-
 drivers/staging/rdma/hfi1/file_ops.c     |    6 ++++++
 include/rdma/ib.h                        |   16 ++++++++++++++++
 7 files changed, 40 insertions(+), 1 deletion(-)

drivers/infiniband/core/ucm.c +4 -0

@@ -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
 	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;

drivers/infiniband/core/ucma.c +3 -0

@@ -1574,6 +1574,9 @@ static ssize_t ucma_write(struct file *f
 	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;

drivers/infiniband/core/uverbs_main.c +5 -0

@@ -48,6 +48,8 @@
 
 #include <asm/uaccess.h>
 
+#include <rdma/ib.h>
+
 #include "uverbs.h"
 
 MODULE_AUTHOR("Roland Dreier");

@@ -693,6 +695,9 @@ static ssize_t ib_uverbs_write(struct fi
 	int srcu_key;
 	ssize_t ret;
 
+	if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
+		return -EACCES;
+
 	if (count < sizeof hdr)
 		return -EINVAL;

drivers/infiniband/hw/qib/qib_file_ops.c +5 -0

@@ -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
 	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;

drivers/staging/rdma/hfi1/TODO +1 -1

@@ -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

drivers/staging/rdma/hfi1/file_ops.c +6 -0

@@ -52,6 +52,8 @@
 #include <linux/vmalloc.h>
 #include <linux/io.h>
 
+#include <rdma/ib.h>
+
 #include "hfi.h"
 #include "pio.h"
 #include "device.h"

@@ -194,6 +196,10 @@ static ssize_t hfi1_file_write(struct fi
 	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;

include/rdma/ib.h +16 -0

@@ -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 */

Patches currently in stable-queue which might be from [email protected] are

queue-4.5/ib-security-restrict-use-of-the-write-interface.patch