| 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: <20260105073621-mutt-send-email-mst@kernel.org>
Date: Mon, 5 Jan 2026 07:37:31 -0500
From: "Michael S. Tsirkin" <mst@...hat.com>
To: Petr Tesarik <ptesarik@...e.com>
Cc: linux-kernel@...r.kernel.org, Cong Wang <xiyou.wangcong@...il.com>,
Jonathan Corbet <corbet@....net>,
Olivia Mackall <olivia@...enic.com>,
Herbert Xu <herbert@...dor.apana.org.au>,
Jason Wang <jasowang@...hat.com>,
Paolo Bonzini <pbonzini@...hat.com>,
Stefan Hajnoczi <stefanha@...hat.com>,
Eugenio Pérez <eperezma@...hat.com>,
"James E.J. Bottomley" <James.Bottomley@...senpartnership.com>,
"Martin K. Petersen" <martin.petersen@...cle.com>,
Gerd Hoffmann <kraxel@...hat.com>,
Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
Marek Szyprowski <m.szyprowski@...sung.com>,
Robin Murphy <robin.murphy@....com>,
Stefano Garzarella <sgarzare@...hat.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
Simon Horman <horms@...nel.org>, Leon Romanovsky <leon@...nel.org>,
Jason Gunthorpe <jgg@...pe.ca>,
Bartosz Golaszewski <brgl@...nel.org>, linux-doc@...r.kernel.org,
linux-crypto@...r.kernel.org, virtualization@...ts.linux.dev,
linux-scsi@...r.kernel.org, iommu@...ts.linux.dev,
kvm@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH v2 05/15] dma-debug: track cache clean flag in entries
On Mon, Jan 05, 2026 at 10:54:33AM +0100, Petr Tesarik wrote:
> On Mon, 5 Jan 2026 03:23:10 -0500
> "Michael S. Tsirkin" <mst@...hat.com> wrote:
>
> > If a driver is buggy and has 2 overlapping mappings but only
> > sets cache clean flag on the 1st one of them, we warn.
> > But if it only does it for the 2nd one, we don't.
> >
> > Fix by tracking cache clean flag in the entry.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@...hat.com>
> > ---
> > kernel/dma/debug.c | 27 ++++++++++++++++++++++-----
> > 1 file changed, 22 insertions(+), 5 deletions(-)
> >
> > diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
> > index 7e66d863d573..43d6a996d7a7 100644
> > --- a/kernel/dma/debug.c
> > +++ b/kernel/dma/debug.c
> > @@ -63,6 +63,7 @@ enum map_err_types {
> > * @sg_mapped_ents: 'mapped_ents' from dma_map_sg
> > * @paddr: physical start address of the mapping
> > * @map_err_type: track whether dma_mapping_error() was checked
> > + * @is_cache_clean: driver promises not to write to buffer while mapped
> > * @stack_len: number of backtrace entries in @stack_entries
> > * @stack_entries: stack of backtrace history
> > */
> > @@ -76,7 +77,8 @@ struct dma_debug_entry {
> > int sg_call_ents;
> > int sg_mapped_ents;
> > phys_addr_t paddr;
> > - enum map_err_types map_err_type;
> > + enum map_err_types map_err_type;
>
> *nitpick* unnecessary change in white space (breaks git-blame).
>
> Other than that, LGTM. I'm not formally a reviewer, but FWIW:
>
> Reviewed-by: Petr Tesarik <ptesarik@...e.com>
>
> Petr T
I mean, yes it's not really required here, but the padding we had before
was broken (two spaces not aligning to anything).
> > + bool is_cache_clean;
> > #ifdef CONFIG_STACKTRACE
> > unsigned int stack_len;
> > unsigned long stack_entries[DMA_DEBUG_STACKTRACE_ENTRIES];
> > @@ -472,12 +474,15 @@ static int active_cacheline_dec_overlap(phys_addr_t cln)
> > return active_cacheline_set_overlap(cln, --overlap);
> > }
> >
> > -static int active_cacheline_insert(struct dma_debug_entry *entry)
> > +static int active_cacheline_insert(struct dma_debug_entry *entry,
> > + bool *overlap_cache_clean)
> > {
> > phys_addr_t cln = to_cacheline_number(entry);
> > unsigned long flags;
> > int rc;
> >
> > + *overlap_cache_clean = false;
> > +
> > /* If the device is not writing memory then we don't have any
> > * concerns about the cpu consuming stale data. This mitigates
> > * legitimate usages of overlapping mappings.
> > @@ -487,8 +492,16 @@ static int active_cacheline_insert(struct dma_debug_entry *entry)
> >
> > spin_lock_irqsave(&radix_lock, flags);
> > rc = radix_tree_insert(&dma_active_cacheline, cln, entry);
> > - if (rc == -EEXIST)
> > + if (rc == -EEXIST) {
> > + struct dma_debug_entry *existing;
> > +
> > active_cacheline_inc_overlap(cln);
> > + existing = radix_tree_lookup(&dma_active_cacheline, cln);
> > + /* A lookup failure here after we got -EEXIST is unexpected. */
> > + WARN_ON(!existing);
> > + if (existing)
> > + *overlap_cache_clean = existing->is_cache_clean;
> > + }
> > spin_unlock_irqrestore(&radix_lock, flags);
> >
> > return rc;
> > @@ -583,20 +596,24 @@ DEFINE_SHOW_ATTRIBUTE(dump);
> > */
> > static void add_dma_entry(struct dma_debug_entry *entry, unsigned long attrs)
> > {
> > + bool overlap_cache_clean;
> > struct hash_bucket *bucket;
> > unsigned long flags;
> > int rc;
> >
> > + entry->is_cache_clean = !!(attrs & DMA_ATTR_CPU_CACHE_CLEAN);
> > +
> > bucket = get_hash_bucket(entry, &flags);
> > hash_bucket_add(bucket, entry);
> > put_hash_bucket(bucket, flags);
> >
> > - rc = active_cacheline_insert(entry);
> > + rc = active_cacheline_insert(entry, &overlap_cache_clean);
> > if (rc == -ENOMEM) {
> > pr_err_once("cacheline tracking ENOMEM, dma-debug disabled\n");
> > global_disable = true;
> > } else if (rc == -EEXIST &&
> > - !(attrs & (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_CPU_CACHE_CLEAN)) &&
> > + !(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
> > + !(entry->is_cache_clean && overlap_cache_clean) &&
> > !(IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) &&
> > is_swiotlb_active(entry->dev))) {
> > err_printk(entry->dev, entry,
Powered by blists - more mailing lists