Skip to content

Commit e6bd18f

Browse files
jgunthorpedledford
authored andcommitted
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>
1 parent 7723d8c commit e6bd18f

File tree

7 files changed

+40
-1
lines changed

7 files changed

+40
-1
lines changed

Diff for: drivers/infiniband/core/ucm.c

+4
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848

4949
#include <asm/uaccess.h>
5050

51+
#include <rdma/ib.h>
5152
#include <rdma/ib_cm.h>
5253
#include <rdma/ib_user_cm.h>
5354
#include <rdma/ib_marshall.h>
@@ -1103,6 +1104,9 @@ static ssize_t ib_ucm_write(struct file *filp, const char __user *buf,
11031104
struct ib_ucm_cmd_hdr hdr;
11041105
ssize_t result;
11051106

1107+
if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
1108+
return -EACCES;
1109+
11061110
if (len < sizeof(hdr))
11071111
return -EINVAL;
11081112

Diff for: drivers/infiniband/core/ucma.c

+3
Original file line numberDiff line numberDiff line change
@@ -1574,6 +1574,9 @@ static ssize_t ucma_write(struct file *filp, const char __user *buf,
15741574
struct rdma_ucm_cmd_hdr hdr;
15751575
ssize_t ret;
15761576

1577+
if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
1578+
return -EACCES;
1579+
15771580
if (len < sizeof(hdr))
15781581
return -EINVAL;
15791582

Diff for: drivers/infiniband/core/uverbs_main.c

+5
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@
4848

4949
#include <asm/uaccess.h>
5050

51+
#include <rdma/ib.h>
52+
5153
#include "uverbs.h"
5254

5355
MODULE_AUTHOR("Roland Dreier");
@@ -709,6 +711,9 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
709711
int srcu_key;
710712
ssize_t ret;
711713

714+
if (WARN_ON_ONCE(!ib_safe_file_access(filp)))
715+
return -EACCES;
716+
712717
if (count < sizeof hdr)
713718
return -EINVAL;
714719

Diff for: drivers/infiniband/hw/qib/qib_file_ops.c

+5
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@
4545
#include <linux/export.h>
4646
#include <linux/uio.h>
4747

48+
#include <rdma/ib.h>
49+
4850
#include "qib.h"
4951
#include "qib_common.h"
5052
#include "qib_user_sdma.h"
@@ -2067,6 +2069,9 @@ static ssize_t qib_write(struct file *fp, const char __user *data,
20672069
ssize_t ret = 0;
20682070
void *dest;
20692071

2072+
if (WARN_ON_ONCE(!ib_safe_file_access(fp)))
2073+
return -EACCES;
2074+
20702075
if (count < sizeof(cmd.type)) {
20712076
ret = -EINVAL;
20722077
goto bail;

Diff for: drivers/staging/rdma/hfi1/TODO

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,4 @@ July, 2015
33
- Remove unneeded file entries in sysfs
44
- Remove software processing of IB protocol and place in library for use
55
by qib, ipath (if still present), hfi1, and eventually soft-roce
6-
6+
- Replace incorrect uAPI

Diff for: drivers/staging/rdma/hfi1/file_ops.c

+6
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@
4949
#include <linux/vmalloc.h>
5050
#include <linux/io.h>
5151

52+
#include <rdma/ib.h>
53+
5254
#include "hfi.h"
5355
#include "pio.h"
5456
#include "device.h"
@@ -190,6 +192,10 @@ static ssize_t hfi1_file_write(struct file *fp, const char __user *data,
190192
int uctxt_required = 1;
191193
int must_be_root = 0;
192194

195+
/* FIXME: This interface cannot continue out of staging */
196+
if (WARN_ON_ONCE(!ib_safe_file_access(fp)))
197+
return -EACCES;
198+
193199
if (count < sizeof(cmd)) {
194200
ret = -EINVAL;
195201
goto bail;

Diff for: include/rdma/ib.h

+16
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#define _RDMA_IB_H
3535

3636
#include <linux/types.h>
37+
#include <linux/sched.h>
3738

3839
struct ib_addr {
3940
union {
@@ -86,4 +87,19 @@ struct sockaddr_ib {
8687
__u64 sib_scope_id;
8788
};
8889

90+
/*
91+
* The IB interfaces that use write() as bi-directional ioctl() are
92+
* fundamentally unsafe, since there are lots of ways to trigger "write()"
93+
* calls from various contexts with elevated privileges. That includes the
94+
* traditional suid executable error message writes, but also various kernel
95+
* interfaces that can write to file descriptors.
96+
*
97+
* This function provides protection for the legacy API by restricting the
98+
* calling context.
99+
*/
100+
static inline bool ib_safe_file_access(struct file *filp)
101+
{
102+
return filp->f_cred == current_cred() && segment_eq(get_fs(), USER_DS);
103+
}
104+
89105
#endif /* _RDMA_IB_H */

0 commit comments

Comments
 (0)