From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoph Hellwig Subject: Re: [PATCH 02/21] arm64/iommu: improve mmap bounds checking Date: Tue, 9 Apr 2019 19:09:13 +0200 Message-ID: <20190409170913.GA14679@lst.de> References: <20190327080448.5500-1-hch@lst.de> <20190327080448.5500-3-hch@lst.de> <3629087c-a8cb-d66e-840b-cfee125bdf4c@arm.com> <20190407065923.GA9086@lst.de> <45672f18-c741-601d-6bb2-92f50b77e981@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <45672f18-c741-601d-6bb2-92f50b77e981@arm.com> Sender: linux-kernel-owner@vger.kernel.org To: Robin Murphy Cc: Christoph Hellwig , Joerg Roedel , Catalin Marinas , Will Deacon , Tom Lendacky , iommu@lists.linux-foundation.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org List-Id: iommu@lists.linux-foundation.org On Tue, Apr 09, 2019 at 04:12:51PM +0100, Robin Murphy wrote: > On 07/04/2019 07:59, Christoph Hellwig wrote: >> On Fri, Apr 05, 2019 at 06:30:52PM +0100, Robin Murphy wrote: >>> On 27/03/2019 08:04, Christoph Hellwig wrote: >>>> The nr_pages checks should be done for all mmap requests, not just those >>>> using remap_pfn_range. >>> >>> Hmm, the logic in iommu_dma_mmap() inherently returns an error for the "off >>>> = nr_pages" case already. It's also supposed to be robust against the >>> "vma_pages(vma) > nr_pages - off" condition, although by making the partial >>> mapping and treating it as a success, rather than doing nothing and >>> returning an error. What's the exact motivation here? >> >> Have one error check at the front of the function that is identical >> to the mmap checks in the other dma_map_ops instances so that: >> >> a) we get the same error behavior for partial requests everywhere >> b) we can lift these checks into common code in the next round. >> > > Fair enough, but in that case why isn't the dma_mmap_from_coherent() path > also covered? dma_mmap_from_coherent currently duplicates those checks itself, and because of that the other callers also don't include it in their checks. I don't actually like that situation and have patches to refactor and clean up that whole mess by also moving the dma coherent mmap to common code, and share the checks that I plan to also lift. But for now I'm holding these back as they would conflict with this series and I'm not sure if it will go in and if yes if that is through the dma-mapping or iommu tree. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B6849C282CE for ; Tue, 9 Apr 2019 17:09:29 +0000 (UTC) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 8F0E420850 for ; Tue, 9 Apr 2019 17:09:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8F0E420850 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=lst.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=iommu-bounces@lists.linux-foundation.org Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 8EFB6DC2; Tue, 9 Apr 2019 17:09:28 +0000 (UTC) Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 4DB53ACC for ; Tue, 9 Apr 2019 17:09:27 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.7.6 Received: from newverein.lst.de (verein.lst.de [213.95.11.211]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id ADC227A6 for ; Tue, 9 Apr 2019 17:09:26 +0000 (UTC) Received: by newverein.lst.de (Postfix, from userid 2407) id B96E768B02; Tue, 9 Apr 2019 19:09:13 +0200 (CEST) Date: Tue, 9 Apr 2019 19:09:13 +0200 From: Christoph Hellwig To: Robin Murphy Subject: Re: [PATCH 02/21] arm64/iommu: improve mmap bounds checking Message-ID: <20190409170913.GA14679@lst.de> References: <20190327080448.5500-1-hch@lst.de> <20190327080448.5500-3-hch@lst.de> <3629087c-a8cb-d66e-840b-cfee125bdf4c@arm.com> <20190407065923.GA9086@lst.de> <45672f18-c741-601d-6bb2-92f50b77e981@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <45672f18-c741-601d-6bb2-92f50b77e981@arm.com> User-Agent: Mutt/1.5.17 (2007-11-01) Cc: Tom Lendacky , Catalin Marinas , Will Deacon , linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, Christoph Hellwig , linux-arm-kernel@lists.infradead.org X-BeenThere: iommu@lists.linux-foundation.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Development issues for Linux IOMMU support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Sender: iommu-bounces@lists.linux-foundation.org Errors-To: iommu-bounces@lists.linux-foundation.org Message-ID: <20190409170913.u8jV1G8CG6aRoBEJtsoXiml8HHsQx3lYOxwBxecMQ-4@z> On Tue, Apr 09, 2019 at 04:12:51PM +0100, Robin Murphy wrote: > On 07/04/2019 07:59, Christoph Hellwig wrote: >> On Fri, Apr 05, 2019 at 06:30:52PM +0100, Robin Murphy wrote: >>> On 27/03/2019 08:04, Christoph Hellwig wrote: >>>> The nr_pages checks should be done for all mmap requests, not just those >>>> using remap_pfn_range. >>> >>> Hmm, the logic in iommu_dma_mmap() inherently returns an error for the "off >>>> = nr_pages" case already. It's also supposed to be robust against the >>> "vma_pages(vma) > nr_pages - off" condition, although by making the partial >>> mapping and treating it as a success, rather than doing nothing and >>> returning an error. What's the exact motivation here? >> >> Have one error check at the front of the function that is identical >> to the mmap checks in the other dma_map_ops instances so that: >> >> a) we get the same error behavior for partial requests everywhere >> b) we can lift these checks into common code in the next round. >> > > Fair enough, but in that case why isn't the dma_mmap_from_coherent() path > also covered? dma_mmap_from_coherent currently duplicates those checks itself, and because of that the other callers also don't include it in their checks. I don't actually like that situation and have patches to refactor and clean up that whole mess by also moving the dma coherent mmap to common code, and share the checks that I plan to also lift. But for now I'm holding these back as they would conflict with this series and I'm not sure if it will go in and if yes if that is through the dma-mapping or iommu tree. _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.5 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A8286C282CE for ; Tue, 9 Apr 2019 17:09:33 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7A39720850 for ; Tue, 9 Apr 2019 17:09:33 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="XrkwtzrA" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7A39720850 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=lst.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=AyGy1LF+F+VZT25iuo0jYV+5yu0scGd6LcfhOTPqwEY=; b=XrkwtzrA4BFMl8 1joDTD5HfFzCTSoRm1pTu7rnB5eEYW0Z2Sif5nIIOgXWAkbjJSuIz1GYOXfOhrTUC6UwgYH7BInbU QNQI8aOShmPJ0SA/rFRiFmZJ2A3r2l9oYMz+p/I0CBsHhSd4Li5ERdht2E887VL4sdEv5MA8Yrhjv 7Ez9wuQlq6FegQften6RYVqvlyqaLx16Kv00QV6xAfjNUbFKLoioZLtTjUjgZv7kO/bid7617w4rp iN/vsN1qwIhTcjqiO8Gy4qCezjLFEGiLh4873v2luTBnq89M9V2EGSsdmLPzgUe0eZwCAmyO/BQSN lSqdBJcCEKhjeMFKgyEQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hDuFY-0005Z7-8D; Tue, 09 Apr 2019 17:09:32 +0000 Received: from verein.lst.de ([213.95.11.211] helo=newverein.lst.de) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1hDuFV-0005YA-1D for linux-arm-kernel@lists.infradead.org; Tue, 09 Apr 2019 17:09:30 +0000 Received: by newverein.lst.de (Postfix, from userid 2407) id B96E768B02; Tue, 9 Apr 2019 19:09:13 +0200 (CEST) Date: Tue, 9 Apr 2019 19:09:13 +0200 From: Christoph Hellwig To: Robin Murphy Subject: Re: [PATCH 02/21] arm64/iommu: improve mmap bounds checking Message-ID: <20190409170913.GA14679@lst.de> References: <20190327080448.5500-1-hch@lst.de> <20190327080448.5500-3-hch@lst.de> <3629087c-a8cb-d66e-840b-cfee125bdf4c@arm.com> <20190407065923.GA9086@lst.de> <45672f18-c741-601d-6bb2-92f50b77e981@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <45672f18-c741-601d-6bb2-92f50b77e981@arm.com> User-Agent: Mutt/1.5.17 (2007-11-01) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190409_100929_226891_583064C9 X-CRM114-Status: GOOD ( 14.79 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Tom Lendacky , Catalin Marinas , Joerg Roedel , Will Deacon , linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, Christoph Hellwig , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Apr 09, 2019 at 04:12:51PM +0100, Robin Murphy wrote: > On 07/04/2019 07:59, Christoph Hellwig wrote: >> On Fri, Apr 05, 2019 at 06:30:52PM +0100, Robin Murphy wrote: >>> On 27/03/2019 08:04, Christoph Hellwig wrote: >>>> The nr_pages checks should be done for all mmap requests, not just those >>>> using remap_pfn_range. >>> >>> Hmm, the logic in iommu_dma_mmap() inherently returns an error for the "off >>>> = nr_pages" case already. It's also supposed to be robust against the >>> "vma_pages(vma) > nr_pages - off" condition, although by making the partial >>> mapping and treating it as a success, rather than doing nothing and >>> returning an error. What's the exact motivation here? >> >> Have one error check at the front of the function that is identical >> to the mmap checks in the other dma_map_ops instances so that: >> >> a) we get the same error behavior for partial requests everywhere >> b) we can lift these checks into common code in the next round. >> > > Fair enough, but in that case why isn't the dma_mmap_from_coherent() path > also covered? dma_mmap_from_coherent currently duplicates those checks itself, and because of that the other callers also don't include it in their checks. I don't actually like that situation and have patches to refactor and clean up that whole mess by also moving the dma coherent mmap to common code, and share the checks that I plan to also lift. But for now I'm holding these back as they would conflict with this series and I'm not sure if it will go in and if yes if that is through the dma-mapping or iommu tree. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel