Commit e6bd18f5 authored by Jason Gunthorpe's avatar Jason Gunthorpe Committed by Doug Ledford

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: default avatarJann Horn <jann@thejh.net>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: default avatarJason Gunthorpe <jgunthorpe@obsidianresearch.com>
[ Expanded check to all known write() entry points ]
Cc: stable@vger.kernel.org
Signed-off-by: default avatarDoug Ledford <dledford@redhat.com>
parent 7723d8c2
...@@ -48,6 +48,7 @@ ...@@ -48,6 +48,7 @@
#include <asm/uaccess.h> #include <asm/uaccess.h>
#include <rdma/ib.h>
#include <rdma/ib_cm.h> #include <rdma/ib_cm.h>
#include <rdma/ib_user_cm.h> #include <rdma/ib_user_cm.h>
#include <rdma/ib_marshall.h> #include <rdma/ib_marshall.h>
...@@ -1103,6 +1104,9 @@ static ssize_t ib_ucm_write(struct file *filp, const char __user *buf, ...@@ -1103,6 +1104,9 @@ static ssize_t ib_ucm_write(struct file *filp, const char __user *buf,
struct ib_ucm_cmd_hdr hdr; struct ib_ucm_cmd_hdr hdr;
ssize_t result; ssize_t result;
if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
return -EACCES;
if (len < sizeof(hdr)) if (len < sizeof(hdr))
return -EINVAL; return -EINVAL;
......
...@@ -1574,6 +1574,9 @@ static ssize_t ucma_write(struct file *filp, const char __user *buf, ...@@ -1574,6 +1574,9 @@ static ssize_t ucma_write(struct file *filp, const char __user *buf,
struct rdma_ucm_cmd_hdr hdr; struct rdma_ucm_cmd_hdr hdr;
ssize_t ret; ssize_t ret;
if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
return -EACCES;
if (len < sizeof(hdr)) if (len < sizeof(hdr))
return -EINVAL; return -EINVAL;
......
...@@ -48,6 +48,8 @@ ...@@ -48,6 +48,8 @@
#include <asm/uaccess.h> #include <asm/uaccess.h>
#include <rdma/ib.h>
#include "uverbs.h" #include "uverbs.h"
MODULE_AUTHOR("Roland Dreier"); MODULE_AUTHOR("Roland Dreier");
...@@ -709,6 +711,9 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf, ...@@ -709,6 +711,9 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
int srcu_key; int srcu_key;
ssize_t ret; ssize_t ret;
if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
return -EACCES;
if (count < sizeof hdr) if (count < sizeof hdr)
return -EINVAL; return -EINVAL;
......
...@@ -45,6 +45,8 @@ ...@@ -45,6 +45,8 @@
#include <linux/export.h> #include <linux/export.h>
#include <linux/uio.h> #include <linux/uio.h>
#include <rdma/ib.h>
#include "qib.h" #include "qib.h"
#include "qib_common.h" #include "qib_common.h"
#include "qib_user_sdma.h" #include "qib_user_sdma.h"
...@@ -2067,6 +2069,9 @@ static ssize_t qib_write(struct file *fp, const char __user *data, ...@@ -2067,6 +2069,9 @@ static ssize_t qib_write(struct file *fp, const char __user *data,
ssize_t ret = 0; ssize_t ret = 0;
void *dest; void *dest;
if (WARN_ON_ONCE(!ib_safe_file_access(fp)))
return -EACCES;
if (count < sizeof(cmd.type)) { if (count < sizeof(cmd.type)) {
ret = -EINVAL; ret = -EINVAL;
goto bail; goto bail;
......
...@@ -3,4 +3,4 @@ July, 2015 ...@@ -3,4 +3,4 @@ July, 2015
- Remove unneeded file entries in sysfs - Remove unneeded file entries in sysfs
- Remove software processing of IB protocol and place in library for use - Remove software processing of IB protocol and place in library for use
by qib, ipath (if still present), hfi1, and eventually soft-roce by qib, ipath (if still present), hfi1, and eventually soft-roce
- Replace incorrect uAPI
...@@ -49,6 +49,8 @@ ...@@ -49,6 +49,8 @@
#include <linux/vmalloc.h> #include <linux/vmalloc.h>
#include <linux/io.h> #include <linux/io.h>
#include <rdma/ib.h>
#include "hfi.h" #include "hfi.h"
#include "pio.h" #include "pio.h"
#include "device.h" #include "device.h"
...@@ -190,6 +192,10 @@ static ssize_t hfi1_file_write(struct file *fp, const char __user *data, ...@@ -190,6 +192,10 @@ static ssize_t hfi1_file_write(struct file *fp, const char __user *data,
int uctxt_required = 1; int uctxt_required = 1;
int must_be_root = 0; 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)) { if (count < sizeof(cmd)) {
ret = -EINVAL; ret = -EINVAL;
goto bail; goto bail;
......
...@@ -34,6 +34,7 @@ ...@@ -34,6 +34,7 @@
#define _RDMA_IB_H #define _RDMA_IB_H
#include <linux/types.h> #include <linux/types.h>
#include <linux/sched.h>
struct ib_addr { struct ib_addr {
union { union {
...@@ -86,4 +87,19 @@ struct sockaddr_ib { ...@@ -86,4 +87,19 @@ struct sockaddr_ib {
__u64 sib_scope_id; __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 */ #endif /* _RDMA_IB_H */
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment