From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f49.google.com (mail-wr1-f49.google.com [209.85.221.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 30C33213E86 for ; Fri, 7 Mar 2025 10:18:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.49 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741342699; cv=none; b=cLrTRbacOHJKF62mac3w98rit5neM1XqNWbIiFDKR2x3baiQXF2P224yTHxeCByDg6V9IVocVvfnrjRsNKFMqipKxM/VsIDjlaKLs5PBPKZ7NPUNSHYG8u4htgDtbhUUyarR/mW1DhpxU/IN2NHuLHQ0fUNcb9swSSDKMXIERjs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741342699; c=relaxed/simple; bh=A2ngPlxsViHeU++f/T0fHfvoDhTTkba+2oBbb7Nj+nI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=vCk3dRpv2mJmlZ37bqMhcVVVsbTyisGb8vkdZHnDhPCcfYMVklacL8eGeID4d0R8SO/2byPd4ISxTx9Cka+iKMDrmyjR+P70VpUHxfhIsnUoIUko+y6pGINUc2SA1iOwp8R32mqxprtPQdHUR8AavjDy82jluIEST9zzUl8IFjo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch; spf=none smtp.mailfrom=ffwll.ch; dkim=pass (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b=eiLb2h4g; arc=none smtp.client-ip=209.85.221.49 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=ffwll.ch Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="eiLb2h4g" Received: by mail-wr1-f49.google.com with SMTP id ffacd0b85a97d-391342fc0b5so655573f8f.3 for ; Fri, 07 Mar 2025 02:18:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; t=1741342695; x=1741947495; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date:from:to:cc :subject:date:message-id:reply-to; bh=eriVuZyAjeW4XZxzbs/VYx0wmamknYVQrCt+Q1+FUXw=; b=eiLb2h4gjukf5j52+npr3KDym6bccOdn0JdH3it+D9rbhYFyBwGhz5Z9+pASCnjQRA tZVIl5lx2Z+DxbLn2vh/J0Tkmh53BJvEdIXpKYNzoySYWkaGaDPCX5sJtFDs/vll6Q57 KfQ62JeHmX085MjHboJ4HzlUMkb+k8KBhdZ14= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741342695; x=1741947495; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=eriVuZyAjeW4XZxzbs/VYx0wmamknYVQrCt+Q1+FUXw=; b=mzFZt2mewb1KMmThGutdp6Eg9e+Ll4XGV4CWmxZdxuwtyTn0rnjHbuvTV9a8lu5M21 ax26P5GZlxBcjIS56/YvhkkWbG17K5tzKlclMttOXVBTpj47b0ngSfwqq9Lvw+qjFcwm +FCVz1g/I1dhGjmFLNMuNZJMuyYj0lAVowmoPEmVGsJg5CnRWU3i9gjheZNmox+2vJdg zepgDwVgMNm8OU1mJMgFu/lJ/jwZe2u2LWro2mfKzmHkK5V6Pzcr9NKafyZdSqokxckB rZDk2PImmDTNbXl8iSM8h0ZQXKeKSlfaitXpMrFc2THpqxg/mXTrnLilEJFGV4W6KMl2 JZcQ== X-Forwarded-Encrypted: i=1; AJvYcCXvqpfbax8tQ8TrDyeYes07nt/xyp1V5DlgFGbn0tyxwJ3VFjjyPHasLugNDS+AI0t9jTp3YHfXgoVN/m489A==@vger.kernel.org X-Gm-Message-State: AOJu0Yx3VMb/fFyM5oJMjJzDfXNPL2J0QAly5pM6GN0jVQ2EH8UeB1rn aNpj8Ts9TLBW+f68yfWDmXjuPARnzmDeXJ3Wc1wDA4EnWhWLwsiwNhdjPJ96Rng= X-Gm-Gg: ASbGncsM32Tq7Dhz6wLgH6Svpi8znSqTaFDg4Nb4eqLtLdgP6mGomDqUpNY+nMSV3VC btVdeUZ4A/bQ3Nmld1AdVa2X/W7jmVS4tXsVhY8yS0PcMLcOW607CHRa4RFEnPH3pzAExtlDKuV zwAkRObS4mNPKuq1CA6Hb8ezRz1oCRyYFhXpt5f+z2F+TR2cpaDvL2S9iOogA+4YyH8Yu7ZQrEx AnRvWGRO/eQARgu1H3TBQaDNDXJr7FTaZ0a1yUblIQ52RoETsqijKYAhHdB3P/OWvxBYEmMFOQm 8brFVf7KjH428BLXSCKXWKYPY7S4IeKSxKD2J9JMNZb6a9gbbyWCUcWj X-Google-Smtp-Source: AGHT+IEAZISqACsxvEE8pMsJbbIN/kzp1kRyxcTMswdSMukChQqkklpQTvMwVWdtKsOw5sLVVDQLwg== X-Received: by 2002:a5d:584f:0:b0:391:2e7:67ff with SMTP id ffacd0b85a97d-39132d090fbmr2149096f8f.10.1741342695383; Fri, 07 Mar 2025 02:18:15 -0800 (PST) Received: from phenom.ffwll.local ([2a02:168:57f4:0:5485:d4b2:c087:b497]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-3912bfdfcdasm4798966f8f.23.2025.03.07.02.18.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 07 Mar 2025 02:18:14 -0800 (PST) Date: Fri, 7 Mar 2025 11:18:12 +0100 From: Simona Vetter To: Danilo Krummrich Cc: Jason Gunthorpe , Abdiel Janulgue , aliceryhl@google.com, robin.murphy@arm.com, daniel.almeida@collabora.com, rust-for-linux@vger.kernel.org, Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , =?iso-8859-1?Q?Bj=F6rn?= Roy Baron , Benno Lossin , Andreas Hindborg , Trevor Gross , Valentin Obst , open list , Christoph Hellwig , Marek Szyprowski , airlied@redhat.com, "open list:DMA MAPPING HELPERS" Subject: Re: [PATCH v12 2/3] rust: add dma coherent allocator abstraction. Message-ID: Mail-Followup-To: Danilo Krummrich , Jason Gunthorpe , Abdiel Janulgue , aliceryhl@google.com, robin.murphy@arm.com, daniel.almeida@collabora.com, rust-for-linux@vger.kernel.org, Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , =?iso-8859-1?Q?Bj=F6rn?= Roy Baron , Benno Lossin , Andreas Hindborg , Trevor Gross , Valentin Obst , open list , Christoph Hellwig , Marek Szyprowski , airlied@redhat.com, "open list:DMA MAPPING HELPERS" References: <20250224115007.2072043-1-abdiel.janulgue@gmail.com> <20250224115007.2072043-3-abdiel.janulgue@gmail.com> <20250305174118.GA351188@nvidia.com> <20250306160907.GF354511@nvidia.com> Precedence: bulk X-Mailing-List: rust-for-linux@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: Linux phenom 6.12.11-amd64 On Fri, Mar 07, 2025 at 09:50:07AM +0100, Danilo Krummrich wrote: > On Thu, Mar 06, 2025 at 12:09:07PM -0400, Jason Gunthorpe wrote: > > On Thu, Mar 06, 2025 at 04:21:51PM +0100, Simona Vetter wrote: > > > > > > a device with no driver bound should not be passed to the DMA API, > > > > > > much less a dead device that's already been removed from its parent > > > > > > bus. > > > > > > > > Thanks for bringing this up! > > > > > > > > I assume that's because of potential iommu mappings, the memory itself should > > > > not be critical. > > > > There is a lot of state tied to the struct device lifecycle that the > > DMA API and iommu implicitly manages. It is not just iommu mappings. > > > > It is incorrect to view the struct device as a simple refcount object > > where holding the refcount means it is alive and safe to use. There > > are three broad substates (No Driver, Driver Attached, Zombie) that > > the struct device can be in that are relevant. > > > > Technically it is unsafe and oopsable to call the allocation API as > > well on a device that has no driver. This issue is also ignored in > > these bindings and cannot be solved with revoke. > > This is correct, and I am well aware of it. I brought this up once when working > on the initial device / driver, devres and I/O abstractions. > > It's on my list to make the creation of the Devres container fallible in this > aspect, which would prevent this issue. > > For now it's probably not too critical; we never hand out device references > before probe(). The only source of error is when a driver tries to create new > device resources after the device has been unbound. > > > IOW I do not belive you can create bindings here that are truely safe > > without also teaching rust to understand the concept of a scope > > guaranteed to be within a probed driver's lifetime. > > > > > > > Also note that any HW configured to do DMA must be halted before the > > > > > free is allowed otherwise it is a UAF bug. It is worth mentioning that > > > > > in the documentation. > > > > > > > > Agreed, makes sense to document. For embedding the CoherentAllocation into > > > > Devres this shouldn't be an issue, since a driver must stop operating the device > > > > in remove() by definition. > > > > > > I think for basic driver allocations that you just need to run the device > > > stuffing it all into devres is ok. > > > > What exactly will this revokable critical region protect? > > > > The actual critical region extends into the HW itself, it is not > > simple to model this with a pure SW construct of bracketing some > > allocation. You need to bracket the *entire lifecycle* of the > > dma_addr_t that has been returned and passed into HW, until the > > dma_addr_t is removed from HW. > > Devres callbacks run after remove(). It's the drivers job to stop operating the > device latest in remove(). Which means that the design is correct. > > Now, you ask for a step further, i.e. make it that we can enforce that a driver > actually stopped the device in remove(). > > But that's just impossible, because obviously no one else than the driver knows > the semantics of the devicei; it's the whole purpose of the driver. So, this is > one of the exceptions where just have to trust the driver doing the correct > thing. In general it's impossible, but I think for specific cases like pci we can enforce that bus mastering/interrupt generation/whatever else might cause havoc is force-disabled after ->remove finishes. For platform devices this is more annoying, but then it's much harder to physically yank a platform devices. So I'm less worried about that being a practical concern there. > Having that said, it doesn't need to be an "all or nothing", let's catch the > ones we can actually catch. Yeah, agreed. -Sima -- Simona Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch