Received: by 10.223.185.82 with SMTP id b18csp10317wrg; Thu, 8 Mar 2018 18:02:35 -0800 (PST) X-Google-Smtp-Source: AG47ELvxedUYKqnzmfqjWOCE9hOCduxJtjdSHoXReucPdeW13yQZN/IOBtEgYT6rPGIZMfuu0eOz X-Received: by 10.101.86.198 with SMTP id w6mr22558508pgs.434.1520560955019; Thu, 08 Mar 2018 18:02:35 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1520560954; cv=none; d=google.com; s=arc-20160816; b=mnHK5jCKLnCfMTo1oNRchFkyRZ9vXcH3xB5Ite3jNdCQdniQBsD2j9geGyg3mKo5WH RIvZvSeA9QQglBjOe01Gm99ukEyNwlud7C45KW+QFf+j0KrxbNbV3M2dyHymnb+S99WW HEZ0dokrhK80qvbmTRbVMHL0PWA2Jv2KbqLnxvpOLODLYt5QYVe5ZkPgbZZ0MhJfc3jI b76FG++IqkLk/PpQgYytbGRNOzs1nNEC/D8xJcOFoceSYjpNZZLIVcpQBbs5BUUMmmyO 35h3zDqpPHOBxp5vzTt4fkSbjAnA2m5gJgXSpOgQbNPpameOetovI2Vh70+2xR9VwNWG OwFg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject:arc-authentication-results; bh=pO4vt+u7fAdFWljqZW8yW2qH2gDwHhCZCA7qmZEBE4c=; b=krTTmm3twI8mZQvCh//jwhzbLYkM5XzodOkKSka2pIW9wUxU6uEFi6k/x1ptdxMjyJ +p9C1SBVX8xDs//eh7CZDgB72k8PVleLDhIM+F7/ZZmX4yJxm3DOopU4ogyzbHaWBXkE W4A8m4y01wcTf43GZ4RbkPKlrNLUeCa+FProDnIfEH9kI0BDBopt44erZ7l/rosSedbs eRh5PZaUf6euFRoi+uWhisnPB5+S0IpLaHMv64o4ctSSgAiY0Ql6YihQFgTGg0Ao099J FP09KS8mnI7IwSD+p4cJ+nb6201fK651gxbBTnQtWJzt84pEs3b0rHQmXCKUDgaEzlhu giow== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id m16-v6si15831115pls.471.2018.03.08.18.02.20; Thu, 08 Mar 2018 18:02:34 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750923AbeCICBP (ORCPT + 99 others); Thu, 8 Mar 2018 21:01:15 -0500 Received: from mail-oi0-f42.google.com ([209.85.218.42]:40705 "EHLO mail-oi0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750787AbeCICBN (ORCPT ); Thu, 8 Mar 2018 21:01:13 -0500 Received: by mail-oi0-f42.google.com with SMTP id c12so5914216oic.7 for ; Thu, 08 Mar 2018 18:01:13 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=pO4vt+u7fAdFWljqZW8yW2qH2gDwHhCZCA7qmZEBE4c=; b=b0Nmid7BTy3lyXREJL7qvMZvsZgtPWeliEU8BnQUJ/G2wlFaFPbBjj7YDcv00Vmb+Z toCi5K7x1I2vJcn5C8yq/Tm4GImXIAHS5QO2+sYJtyGuggWBxTrRiBfPXyjP6ui90l/P TuW6+yS87R2dVrRoF04OFKlzUfxrw8z5vlGcx85tkv43UeNgIG4s096MCm58jVJWXh8W niV6FODTh85/ZHxJfsiw0R1xH30Fkwzb4xhUAU7oT1nT+miqC+xRI6RYQJgHilpd3wQ+ rK01U1kMIkDnm81xDd9FzrAz0dWLRyW/SrjXAUU5yTwFSUatn/OUeomJr7Pf++jCzCcv mHeA== X-Gm-Message-State: AElRT7GyKYfeocrBrREiAhpCTM+IycNXtIxD0uNldh8f8c0dK4mb8vrq E8seAmAEbVC0OZWA+gUdNhHocw== X-Received: by 10.202.17.12 with SMTP id 12mr16846443oir.278.1520560872887; Thu, 08 Mar 2018 18:01:12 -0800 (PST) Received: from ?IPv6:2601:602:9802:a8dc:4eb2:6dae:ab32:e5b0? ([2601:602:9802:a8dc:4eb2:6dae:ab32:e5b0]) by smtp.gmail.com with ESMTPSA id b46sm8498889otd.47.2018.03.08.18.01.10 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 08 Mar 2018 18:01:11 -0800 (PST) Subject: Re: [RFC] android: ion: How to properly clean caches for uncached allocations To: Liam Mark Cc: Sumit Semwal , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org, Martijn Coenen , Todd Kjos , =?UTF-8?Q?Arve_Hj=c3=b8nnev=c3=a5g?= , linaro-mm-sig@lists.linaro.org References: From: Laura Abbott Message-ID: <7812c0ba-8ef3-2291-eef7-cdbf921f400d@redhat.com> Date: Thu, 8 Mar 2018 18:01:10 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/08/2018 04:45 PM, Liam Mark wrote: > On Wed, 7 Mar 2018, Laura Abbott wrote: > >> On 02/28/2018 09:18 PM, Liam Mark wrote: >>> The issue: >>> >>> Currently in ION if you allocate uncached memory it is possible that there >>> are still dirty lines in the cache. And often these dirty lines in the >>> cache are the zeros which were meant to clear out any sensitive kernel >>> data. >>> >>> What this means is that if you allocate uncached memory from ION, and then >>> subsequently write to that buffer (using the uncached mapping you are >>> provided by ION) then the data you have written could be corrupted at some >>> point in the future if a dirty line is evicted from the cache. >>> >>> Also this means there is a potential security issue. If an un-privileged >>> userspace user allocated uncached memory (for example from the system heap) >>> and then if they were to read from that buffer (through the un-cached >>> mapping they are provided by ION), and if some of the zeros which were >>> written to that memory are still in the cache then this un-privileged >>> userspace user could read potentially sensitive kernel data. >> >> For the use case you are describing we don't actually need the >> memory to be non-cached until it comes time to do the dma mapping. >> Here's a proposal to shoot holes in: >> >> - Before any dma_buf attach happens, all mmap mappings are cached >> - At the time attach happens, we shoot down any existing userspace >> mappings, do the dma_map with appropriate flags to clean the pages >> and then allow remapping to userspace as uncached. Really this >> looks like a variation on the old Ion faulting code which I removed >> except it's for uncached buffers instead of cached buffers. >> > > Thanks Laura, I will take a look to see if I can think of any concerns. > > Initial thoughts. > - What about any kernel mappings (kmap/vmap) the client has made? > We could either synchronize with dma_buf_{begin,end}_cpu_access or just disallow the mapping to happen if there's an outstanding kmap or vmap. Is this an actual problem or only theoretical? > - I guess it would be tempting to only do this behavior for memory that > came from buddy (as opposed to the pool since it should be clean), but we > would need to be careful that no pages sneak into the pool without being > cleaned (example: client allocs then frees without ever call > dma_buf_attach). > You're welcome to try that optimization but I think we should focus on the basics first. Honestly it might make sense just to have a single pool at this point since the cost of syncing is not happening on the allocation path. >> Potential problems: >> - I'm not 100% about the behavior here if the attaching device >> is already dma_coherent. I also consider uncached mappings >> enough of a device specific optimization that you shouldn't >> do them unless you know it's needed. > > I don't believe we want to allow uncached memory to be dma mapped by an > io-coherent device and this is something I would like to eventually block. > > Since there is always a kernel cached mapping for ION uncached memory then > speculative access could still be putting lines in the cache, so when an > io-coherent device tries to read this uncached memory its snoop into the > cache could find one of these 'stale' clean cache lines and end up using > stale data. > Agree? > Sounds right. >> - The locking/sequencing with userspace could be tricky >> since userspace may not like us ripping mappings out from >> underneath if it's trying to access. > > Perhaps delay this work to the dma_map_attachment call since when the data > is dma mapped the CPU shouldn't be accessing it? > > Or worst case perhaps fail all map attempts to uncached memory until the > memory has been dma mapped (and cleaned) at least once? > My concern was mostly concurrent userspace access on a buffer that's being dma_mapped but that sounds racy to begin with. I suggested disallowing mmap until dma_mapping before and I thought that was not possible? Thanks, Laura > Thanks, > Liam > > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project >