From patchwork Thu Apr 2 15:15:52 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Yann Droneaud X-Patchwork-Id: 6149111 Return-Path: X-Original-To: patchwork-linux-rdma@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 6586FBF4A6 for ; Thu, 2 Apr 2015 15:16:12 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 6F72D203AA for ; Thu, 2 Apr 2015 15:16:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4B74A203A4 for ; Thu, 2 Apr 2015 15:16:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753386AbbDBPQI (ORCPT ); Thu, 2 Apr 2015 11:16:08 -0400 Received: from ou.quest-ce.net ([195.154.187.82]:37421 "EHLO ou.quest-ce.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753128AbbDBPQH (ORCPT ); Thu, 2 Apr 2015 11:16:07 -0400 Received: from [37.163.136.85] (helo=test.quest-ce.net) by ou.quest-ce.net with esmtpsa (TLS1.1:RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1Ydgqh-0002mq-QB; Thu, 02 Apr 2015 17:16:04 +0200 Message-ID: <1427987752.22575.65.camel@opteya.com> From: Yann Droneaud To: Shachar Raindel Cc: "oss-security@lists.openwall.com" , " (linux-rdma@vger.kernel.org)" , "linux-kernel@vger.kernel.org" , "stable@vger.kernel.org" Date: Thu, 02 Apr 2015 17:15:52 +0200 In-Reply-To: References: <1427969085.17020.5.camel@opteya.com> Organization: OPTEYA X-Mailer: Evolution 3.12.11 (3.12.11-1.fc21) Mime-Version: 1.0 X-SA-Exim-Connect-IP: 37.163.136.85 X-SA-Exim-Mail-From: ydroneaud@opteya.com X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Subject: Re: CVE-2014-8159 kernel: infiniband: uverbs: unprotected physical memory access X-SA-Exim-Version: 4.2.1 (built Mon, 26 Dec 2011 16:24:06 +0000) X-SA-Exim-Scanned: Yes (on ou.quest-ce.net) Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi, Le jeudi 02 avril 2015 à 10:52 +0000, Shachar Raindel a écrit : > > -----Original Message----- > > From: Yann Droneaud [mailto:ydroneaud@opteya.com] > > Sent: Thursday, April 02, 2015 1:05 PM > > Le mercredi 18 mars 2015 à 17:39 +0000, Shachar Raindel a écrit : ... > > > + /* > > > + * If the combination of the addr and size requested for this > > memory > > > + * region causes an integer overflow, return error. > > > + */ > > > + if ((PAGE_ALIGN(addr + size) <= size) || > > > + (PAGE_ALIGN(addr + size) <= addr)) > > > + return ERR_PTR(-EINVAL); > > > + > > > > Can access_ok() be used here ? > > > > if (!access_ok(writable ? VERIFY_WRITE : VERIFY_READ, > > addr, size)) > > return ERR_PTR(-EINVAL); > > > > No, this will break the current ODP semantics. > > ODP allows the user to register memory that is not accessible yet. > This is a critical design feature, as it allows avoiding holding > a registration cache. Adding this check will break the behavior, > forcing memory to be all accessible when registering an ODP MR. > Failed to notice previously, but since this would break ODP, and ODP is only available starting v3.19-rc1, my proposed fix might be applicable for older kernel (if not better). From 2a3beaeb4b35d968f127cb59cfda2f12dbd87e4b Mon Sep 17 00:00:00 2001 From: Yann Droneaud Date: Thu, 2 Apr 2015 17:01:05 +0200 Subject: [RFC PATCH] IB/core: reject invalid userspace memory range registration Signed-off-by: Yann Droneaud --- drivers/infiniband/core/umem.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c index df0c4f605a21..6758b4ac56eb 100644 --- a/drivers/infiniband/core/umem.c +++ b/drivers/infiniband/core/umem.c @@ -90,6 +90,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr, DEFINE_DMA_ATTRS(attrs); struct scatterlist *sg, *sg_list_start; int need_release = 0; + bool writable; if (dmasync) dma_set_attr(DMA_ATTR_WRITE_BARRIER, &attrs); @@ -97,6 +98,19 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr, if (!can_do_mlock()) return ERR_PTR(-EPERM); + /* + * We ask for writable memory if any access flags other than + * "remote read" are set. "Local write" and "remote write" + * obviously require write access. "Remote atomic" can do + * things like fetch and add, which will modify memory, and + * "MW bind" can change permissions by binding a window. + */ + writable = !!(access & ~IB_ACCESS_REMOTE_READ); + + if (!access_ok(writable ? VERIFY_WRITE : VERIFY_READ, + (void __user *)addr, size)) + return ERR_PTR(-EFAULT); + umem = kzalloc(sizeof *umem, GFP_KERNEL); if (!umem) return ERR_PTR(-ENOMEM); @@ -106,14 +120,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, unsigned long addr, umem->offset = addr & ~PAGE_MASK; umem->page_size = PAGE_SIZE; umem->pid = get_task_pid(current, PIDTYPE_PID); - /* - * We ask for writable memory if any access flags other than - * "remote read" are set. "Local write" and "remote write" - * obviously require write access. "Remote atomic" can do - * things like fetch and add, which will modify memory, and - * "MW bind" can change permissions by binding a window. - */ - umem->writable = !!(access & ~IB_ACCESS_REMOTE_READ); + umem->writable = writable; /* We assume the memory is from hugetlb until proved otherwise */ umem->hugetlb = 1;