From patchwork Thu May 11 16:50:04 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Ursula Braun X-Patchwork-Id: 761250 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3wNzcT29phz9s7k for ; Fri, 12 May 2017 02:50:17 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932366AbdEKQuP (ORCPT ); Thu, 11 May 2017 12:50:15 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:37062 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756576AbdEKQuN (ORCPT ); Thu, 11 May 2017 12:50:13 -0400 Received: from pps.filterd (m0098393.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v4BGmbdg043274 for ; Thu, 11 May 2017 12:50:13 -0400 Received: from e06smtp12.uk.ibm.com (e06smtp12.uk.ibm.com [195.75.94.108]) by mx0a-001b2d01.pphosted.com with ESMTP id 2ac3jym60k-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 11 May 2017 12:50:12 -0400 Received: from localhost by e06smtp12.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 11 May 2017 17:50:09 +0100 Received: from b06cxnps3074.portsmouth.uk.ibm.com (9.149.109.194) by e06smtp12.uk.ibm.com (192.168.101.142) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Thu, 11 May 2017 17:50:05 +0100 Received: from d06av25.portsmouth.uk.ibm.com (d06av25.portsmouth.uk.ibm.com [9.149.105.61]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v4BGo4pJ16056646; Thu, 11 May 2017 16:50:04 GMT Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 7340B11C04C; Thu, 11 May 2017 17:48:45 +0100 (BST) Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 2544D11C050; Thu, 11 May 2017 17:48:45 +0100 (BST) Received: from oc0447013845.ibm.com (unknown [9.152.222.226]) by d06av25.portsmouth.uk.ibm.com (Postfix) with ESMTP; Thu, 11 May 2017 17:48:45 +0100 (BST) Subject: Re: net/smc and the RDMA core To: "hch@lst.de" , Sagi Grimberg References: <20170501163311.GA22209@lst.de> <1493750358.2552.13.camel@sandisk.com> <1b79048f-4495-3840-e7a6-d4fa5a8dfb57@grimberg.me> <20170504084825.GA5399@lst.de> Cc: Bart Van Assche , "davem@davemloft.net" , "netdev@vger.kernel.org" , "linux-rdma@vger.kernel.org" From: Ursula Braun Date: Thu, 11 May 2017 18:50:04 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170504084825.GA5399@lst.de> X-TM-AS-GCONF: 00 x-cbid: 17051116-0008-0000-0000-00000444A82E X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17051116-0009-0000-0000-00001DA2F17A Message-Id: <869d9fb6-0d83-5f57-f8e4-5c1ee7477b94@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-05-11_12:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1703280000 definitions=main-1705110086 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 05/04/2017 10:48 AM, hch@lst.de wrote: > On Thu, May 04, 2017 at 11:43:50AM +0300, Sagi Grimberg wrote: >> I would also suggest that you stop exposing the DMA MR for remote >> access (at least by default) and use a proper reg_mr operations with a >> limited lifetime on a properly sized buffer. > > Yes, exposing the default DMA MR is a _major_ security risk. As soon > as SMC is enabled this will mean a remote system has full read/write > access to the local systems memory. > > There іs a reason why I removed the ib_get_dma_mr function and replaced > it with the IB_PD_UNSAFE_GLOBAL_RKEY key that has _UNSAFE_ in the name > and a very long comment explaining why, and I'm really disappointed that > we got a driver merged that instead of asking on the relevant list on > why a change unexpertong a function it needed happened and instead > tried the hard way to keep a security vulnerarbility alive. > Please consider the following patch to make users aware of the security implications through existing mechanisms. Work on proper usage of reg_mr operations is underway, respective patches will follow as soon as possible. --- net/smc/smc_core.c | 16 +++------------- net/smc/smc_ib.c | 21 ++------------------- net/smc/smc_ib.h | 2 -- 3 files changed, 5 insertions(+), 34 deletions(-) diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c index d52a2ee..6ec3d9a 100644 --- a/net/smc/smc_core.c +++ b/net/smc/smc_core.c @@ -613,19 +613,8 @@ int smc_rmb_create(struct smc_sock *smc) rmb_desc = NULL; continue; /* if mapping failed, try smaller one */ } - rc = smc_ib_get_memory_region(lgr->lnk[SMC_SINGLE_LINK].roce_pd, - IB_ACCESS_REMOTE_WRITE | - IB_ACCESS_LOCAL_WRITE, - &rmb_desc->mr_rx[SMC_SINGLE_LINK]); - if (rc) { - smc_ib_buf_unmap(lgr->lnk[SMC_SINGLE_LINK].smcibdev, - tmp_bufsize, rmb_desc, - DMA_FROM_DEVICE); - kfree(rmb_desc->cpu_addr); - kfree(rmb_desc); - rmb_desc = NULL; - continue; - } + rmb_desc->mr_rx[SMC_SINGLE_LINK] = + lgr->lnk[SMC_SINGLE_LINK].roce_pd->__internal_mr; rmb_desc->used = 1; write_lock_bh(&lgr->rmbs_lock); list_add(&rmb_desc->list, @@ -668,6 +657,7 @@ int smc_rmb_rtoken_handling(struct smc_connection *conn, for (i = 0; i < SMC_RMBS_PER_LGR_MAX; i++) { if ((lgr->rtokens[i][SMC_SINGLE_LINK].rkey == rkey) && + (lgr->rtokens[i][SMC_SINGLE_LINK].dma_addr == dma_addr) && test_bit(i, lgr->rtokens_used_mask)) { conn->rtoken_idx = i; return 0; diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c index 3d0d91c..6af9ebf 100644 --- a/net/smc/smc_ib.c +++ b/net/smc/smc_ib.c @@ -37,24 +37,6 @@ u8 local_systemid[SMC_SYSTEMID_LEN] = SMC_LOCAL_SYSTEMID_RESET; /* unique system * identifier */ -int smc_ib_get_memory_region(struct ib_pd *pd, int access_flags, - struct ib_mr **mr) -{ - int rc; - - if (*mr) - return 0; /* already done */ - - /* obtain unique key - - * next invocation of get_dma_mr returns a different key! - */ - *mr = pd->device->get_dma_mr(pd, access_flags); - rc = PTR_ERR_OR_ZERO(*mr); - if (IS_ERR(*mr)) - *mr = NULL; - return rc; -} - static int smc_ib_modify_qp_init(struct smc_link *lnk) { struct ib_qp_attr qp_attr; @@ -211,7 +193,8 @@ int smc_ib_create_protection_domain(struct smc_link *lnk) { int rc; - lnk->roce_pd = ib_alloc_pd(lnk->smcibdev->ibdev, 0); + lnk->roce_pd = ib_alloc_pd(lnk->smcibdev->ibdev, + IB_PD_UNSAFE_GLOBAL_RKEY); rc = PTR_ERR_OR_ZERO(lnk->roce_pd); if (IS_ERR(lnk->roce_pd)) lnk->roce_pd = NULL; diff --git a/net/smc/smc_ib.h b/net/smc/smc_ib.h index e5bb3c9..55c529b 100644 --- a/net/smc/smc_ib.h +++ b/net/smc/smc_ib.h @@ -60,8 +60,6 @@ void smc_ib_dealloc_protection_domain(struct smc_link *lnk); int smc_ib_create_protection_domain(struct smc_link *lnk); void smc_ib_destroy_queue_pair(struct smc_link *lnk); int smc_ib_create_queue_pair(struct smc_link *lnk); -int smc_ib_get_memory_region(struct ib_pd *pd, int access_flags, - struct ib_mr **mr); int smc_ib_ready_link(struct smc_link *lnk); int smc_ib_modify_qp_rts(struct smc_link *lnk); int smc_ib_modify_qp_reset(struct smc_link *lnk);