| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a59f771f-3289-62f0-ca50-8f3675d9b166@arm.com>
Date: Tue, 6 Jul 2021 15:01:04 +0100
From: Robin Murphy <robin.murphy@....com>
To: Will Deacon <will@...nel.org>, Christoph Hellwig <hch@....de>
Cc: heikki.krogerus@...ux.intel.com, thomas.hellstrom@...ux.intel.com,
peterz@...radead.org, benh@...nel.crashing.org,
joonas.lahtinen@...ux.intel.com, dri-devel@...ts.freedesktop.org,
chris@...is-wilson.co.uk, grant.likely@....com, paulus@...ba.org,
Frank Rowand <frowand.list@...il.com>, mingo@...nel.org,
Stefano Stabellini <sstabellini@...nel.org>,
Saravana Kannan <saravanak@...gle.com>, mpe@...erman.id.au,
"Rafael J . Wysocki" <rafael.j.wysocki@...el.com>,
Bartosz Golaszewski <bgolaszewski@...libre.com>,
bskeggs@...hat.com, linux-pci@...r.kernel.org,
xen-devel@...ts.xenproject.org,
Thierry Reding <treding@...dia.com>,
intel-gfx@...ts.freedesktop.org, matthew.auld@...el.com,
linux-devicetree <devicetree@...r.kernel.org>,
Jianxiong Gao <jxgao@...gle.com>,
Daniel Vetter <daniel@...ll.ch>,
Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
maarten.lankhorst@...ux.intel.com, airlied@...ux.ie,
Dan Williams <dan.j.williams@...el.com>,
linuxppc-dev@...ts.ozlabs.org, jani.nikula@...ux.intel.com,
Nathan Chancellor <nathan@...nel.org>,
Rob Herring <robh+dt@...nel.org>, rodrigo.vivi@...el.com,
Bjorn Helgaas <bhelgaas@...gle.com>,
Claire Chang <tientzu@...omium.org>,
boris.ostrovsky@...cle.com,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
jgross@...e.com, Nicolas Boichat <drinkcat@...omium.org>,
Greg KH <gregkh@...uxfoundation.org>,
Randy Dunlap <rdunlap@...radead.org>,
Qian Cai <quic_qiancai@...cinc.com>,
lkml <linux-kernel@...r.kernel.org>,
"list@....net:IOMMU DRIVERS" <iommu@...ts.linux-foundation.org>,
Jim Quinlan <james.quinlan@...adcom.com>, xypron.glpk@....de,
Tom Lendacky <thomas.lendacky@....com>, bauerman@...ux.ibm.com
Subject: Re: [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for
swiotlb data bouncing
On 2021-07-06 14:24, Will Deacon wrote:
> On Tue, Jul 06, 2021 at 06:48:48AM +0200, Christoph Hellwig wrote:
>> On Mon, Jul 05, 2021 at 08:03:52PM +0100, Will Deacon wrote:
>>> So at this point, the AMD IOMMU driver does:
>>>
>>> swiotlb = (iommu_default_passthrough() || sme_me_mask) ? 1 : 0;
>>>
>>> where 'swiotlb' is a global variable indicating whether or not swiotlb
>>> is in use. It's picked up a bit later on by pci_swiotlb_late_init(), which
>>> will call swiotlb_exit() if 'swiotlb' is false.
>>>
>>> Now, that used to work fine, because swiotlb_exit() clears
>>> 'io_tlb_default_mem' to NULL, but now with the restricted DMA changes, I
>>> think that all the devices which have successfully probed beforehand will
>>> have stale pointers to the freed structure in their 'dev->dma_io_tlb_mem'
>>> field.
>>
>> Yeah. I don't think we can do that anymore, and I also think it is
>> a bad idea to start with.
>
> I've had a crack at reworking things along the following lines:
>
> - io_tlb_default_mem now lives in the BSS, the flexible array member
> is now a pointer and that part is allocated dynamically (downside of
> this is an extra indirection to get at the slots).
>
> - io_tlb_default_mem.nslabs tells you whether the thing is valid
>
> - swiotlb_exit() frees the slots array and clears the rest of the
> structure to 0. I also extended it to free the actual slabs, but I'm
> not sure why it wasn't doing that before.
>
> So a non-NULL dev->dma_io_tlb_mem should always be valid to follow.
FWIW I was pondering the question of whether to do something along those
lines or just scrap the default assignment entirely, so since I hadn't
got round to saying that I've gone ahead and hacked up the alternative
(similarly untested) for comparison :)
TBH I'm still not sure which one I prefer...
Robin.
----->8-----
diff --git a/drivers/base/core.c b/drivers/base/core.c
index ea5b85354526..394abf184c1a 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2847,9 +2847,6 @@ void device_initialize(struct device *dev)
defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL)
dev->dma_coherent = dma_default_coherent;
#endif
-#ifdef CONFIG_SWIOTLB
- dev->dma_io_tlb_mem = io_tlb_default_mem;
-#endif
}
EXPORT_SYMBOL_GPL(device_initialize);
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 39284ff2a6cd..620f16d89a98 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -107,16 +107,21 @@ struct io_tlb_mem {
};
extern struct io_tlb_mem *io_tlb_default_mem;
+static inline struct io_tlb_mem *dev_iotlb_mem(struct device *dev)
+{
+ return dev->dma_io_tlb_mem ?: io_tlb_default_mem;
+}
+
static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t
paddr)
{
- struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+ struct io_tlb_mem *mem = dev_iotlb_mem(dev);
return mem && paddr >= mem->start && paddr < mem->end;
}
static inline bool is_swiotlb_force_bounce(struct device *dev)
{
- struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+ struct io_tlb_mem *mem = dev_iotlb_mem(dev);
return mem && mem->force_bounce;
}
@@ -167,7 +172,7 @@ bool swiotlb_free(struct device *dev, struct page
*page, size_t size);
static inline bool is_swiotlb_for_alloc(struct device *dev)
{
- return dev->dma_io_tlb_mem->for_alloc;
+ return dev_iotlb_mem(dev)->for_alloc;
}
#else
static inline struct page *swiotlb_alloc(struct device *dev, size_t size)
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index b7f76bca89bf..f4942149f87d 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -359,7 +359,7 @@ static unsigned int swiotlb_align_offset(struct
device *dev, u64 addr)
static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr,
size_t size,
enum dma_data_direction dir)
{
- struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+ struct io_tlb_mem *mem = dev_iotlb_mem(dev);
int index = (tlb_addr - mem->start) >> IO_TLB_SHIFT;
phys_addr_t orig_addr = mem->slots[index].orig_addr;
size_t alloc_size = mem->slots[index].alloc_size;
@@ -440,7 +440,7 @@ static unsigned int wrap_index(struct io_tlb_mem
*mem, unsigned int index)
static int swiotlb_find_slots(struct device *dev, phys_addr_t orig_addr,
size_t alloc_size)
{
- struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+ struct io_tlb_mem *mem = dev_iotlb_mem(dev);
unsigned long boundary_mask = dma_get_seg_boundary(dev);
dma_addr_t tbl_dma_addr =
phys_to_dma_unencrypted(dev, mem->start) & boundary_mask;
@@ -522,7 +522,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device
*dev, phys_addr_t orig_addr,
size_t mapping_size, size_t alloc_size,
enum dma_data_direction dir, unsigned long attrs)
{
- struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+ struct io_tlb_mem *mem = dev_iotlb_mem(dev);
unsigned int offset = swiotlb_align_offset(dev, orig_addr);
unsigned int i;
int index;
@@ -565,7 +565,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device
*dev, phys_addr_t orig_addr,
static void swiotlb_release_slots(struct device *dev, phys_addr_t
tlb_addr)
{
- struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+ struct io_tlb_mem *mem = dev_iotlb_mem(dev);
unsigned long flags;
unsigned int offset = swiotlb_align_offset(dev, tlb_addr);
int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT;
@@ -682,7 +682,7 @@ size_t swiotlb_max_mapping_size(struct device *dev)
bool is_swiotlb_active(struct device *dev)
{
- return dev->dma_io_tlb_mem != NULL;
+ return dev_iotlb_mem(dev) != NULL;
}
EXPORT_SYMBOL_GPL(is_swiotlb_active);
@@ -729,7 +729,7 @@ static void rmem_swiotlb_debugfs_init(struct
reserved_mem *rmem)
struct page *swiotlb_alloc(struct device *dev, size_t size)
{
- struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+ struct io_tlb_mem *mem = dev_iotlb_mem(dev);
phys_addr_t tlb_addr;
int index;
@@ -792,7 +792,7 @@ static int rmem_swiotlb_device_init(struct
reserved_mem *rmem,
static void rmem_swiotlb_device_release(struct reserved_mem *rmem,
struct device *dev)
{
- dev->dma_io_tlb_mem = io_tlb_default_mem;
+ dev->dma_io_tlb_mem = NULL;
}
static const struct reserved_mem_ops rmem_swiotlb_ops = {
Powered by blists - more mailing lists