lkml.org 
[lkml]   [2016]   [Jun]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 3.12 02/56] IB/security: Restrict use of the write() interface
    Date
    From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>

    3.12-stable review patch. If anyone has any objections, please let me know.

    ===============

    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.

    [js] backport to 3.12: hfi1 is not there yet (exclude), ipath is still
    there (include)

    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>
    Signed-off-by: Jiri Slaby <jslaby@suse.cz>
    ---
    drivers/infiniband/core/ucm.c | 4 ++++
    drivers/infiniband/core/ucma.c | 3 +++
    drivers/infiniband/core/uverbs_main.c | 5 +++++
    drivers/infiniband/hw/ipath/ipath_file_ops.c | 5 +++++
    drivers/infiniband/hw/qib/qib_file_ops.c | 5 +++++
    include/rdma/ib.h | 16 ++++++++++++++++
    6 files changed, 38 insertions(+)

    diff --git a/drivers/infiniband/core/ucm.c b/drivers/infiniband/core/ucm.c
    index f2f63933e8a9..5befec118a18 100644
    --- 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 b0f189be543b..da67839fc451 100644
    --- a/drivers/infiniband/core/ucma.c
    +++ b/drivers/infiniband/core/ucma.c
    @@ -1494,6 +1494,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 68e5496c5d58..ee5222168b68 100644
    --- 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");
    @@ -601,6 +603,9 @@ static ssize_t ib_uverbs_write(struct file *filp, const char __user *buf,
    struct ib_uverbs_file *file = filp->private_data;
    struct ib_uverbs_cmd_hdr hdr;

    + 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 6d7f453b4d05..a0626b8c61c5 100644
    --- 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 2023cd61b897..3c089ca85c64 100644
    --- 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 cf8f9e700e48..a6b93706b0fc 100644
    --- 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 */
    --
    2.9.0
    \
     
     \ /
      Last update: 2016-06-15 10:21    [W:4.151 / U:0.188 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site