| 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
| ||
|
Date: Tue, 04 Sep 2012 16:57:56 -0600
From: Shuah Khan <shuah.khan@...com>
To: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
Cc: joerg.roedel@....com, paul.gortmaker@...driver.com, kubakici@...pl,
stern@...land.harvard.edu, dan.carpenter@...cle.com,
rob@...dley.net, linux-doc@...r.kernel.org,
LKML <linux-kernel@...r.kernel.org>, shuahkhan@...il.com
Subject: Re: [PATCH] dma-debug: Add dma map/unmap error tracking support
On Tue, 2012-09-04 at 17:05 -0400, Konrad Rzeszutek Wilk wrote:
> On Sun, Sep 02, 2012 at 08:14:17AM -0600, Shuah Khan wrote:
> > A recent dma mapping error analysis effort showed that a large precentage
> > of dma_map_single() and dma_map_page() returns are not checked for mapping
> > errors. Reference: https://lkml.org/lkml/2012/8/10/326
> >
>
> So were you able to catch some naughty drivers with this?
I did compile a complete list of drivers that don't check dma mapping
errors from my analysis. Are you interested in seeing the full analysis?
>
> > Adding support for tracking dma mapping and unmapping errors to help assess
> > the following:
> >
> > When do dma mapping errors get detected?
> > How often do these errors occur?
> > Why don't we see failures related to missing dma mapping error checks?
> > Are they silent failures?
> >
> > Signed-off-by: Shuah Khan <shuah.khan@...com>
> > ---
> > Documentation/DMA-API.txt | 7 +++++++
> > lib/dma-debug.c | 26 +++++++++++++++++++++++++-
> > 2 files changed, 32 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
> > index 66bd97a..ee10a11 100644
> > --- a/Documentation/DMA-API.txt
> > +++ b/Documentation/DMA-API.txt
> > @@ -638,6 +638,13 @@ this directory the following files can currently be found:
> > dma-api/error_count This file is read-only and shows the total
> > numbers of errors found.
> >
> > + dma-api/dma_map_errors This file is read-only and shows the total
> > + number of dma mapping errors detected.
> > +
> > + dma-api/dma_unmap_errors
> > + This file is read-only and shows the total
> > + number of invalid dma unmapping attempts.
> > +
> > dma-api/num_errors The number in this file shows how many
> > warnings will be printed to the kernel log
> > before it stops. This number is initialized to
> > diff --git a/lib/dma-debug.c b/lib/dma-debug.c
> > index 66ce414..8596114 100644
> > --- a/lib/dma-debug.c
> > +++ b/lib/dma-debug.c
> > @@ -83,6 +83,10 @@ static u32 global_disable __read_mostly;
> > /* Global error count */
> > static u32 error_count;
> >
> > +/* dma mapping error counts */
> > +static u32 dma_map_errors;
> > +static u32 dma_unmap_errors;
> > +
> > /* Global error show enable*/
> > static u32 show_all_errors __read_mostly;
> > /* Number of errors to show */
> > @@ -104,6 +108,8 @@ static struct dentry *show_num_errors_dent __read_mostly;
> > static struct dentry *num_free_entries_dent __read_mostly;
> > static struct dentry *min_free_entries_dent __read_mostly;
> > static struct dentry *filter_dent __read_mostly;
> > +static struct dentry *dma_map_errors_dent __read_mostly;
> > +static struct dentry *dma_unmap_errors_dent __read_mostly;
> >
> > /* per-driver filter related state */
> >
> > @@ -695,6 +701,19 @@ static int dma_debug_fs_init(void)
> > if (!filter_dent)
> > goto out_err;
> >
> > + dma_map_errors_dent = debugfs_create_u32("dma_map_errors", 0444,
> > + dma_debug_dent,
> > + &dma_map_errors);
> > +
> > + if (!dma_map_errors_dent)
> > + goto out_err;
> > +
> > + dma_unmap_errors_dent = debugfs_create_u32("dma_unmap_errors", 0444,
> > + dma_debug_dent,
> > + &dma_unmap_errors);
> > + if (!dma_unmap_errors_dent)
> > + goto out_err;
> > +
> > return 0;
> >
> > out_err:
> > @@ -850,6 +869,7 @@ static void check_unmap(struct dma_debug_entry *ref)
> > unsigned long flags;
> >
> > if (dma_mapping_error(ref->dev, ref->dev_addr)) {
> > + dma_unmap_errors += 1;
> > err_printk(ref->dev, NULL, "DMA-API: device driver tries "
> > "to free an invalid DMA memory address\n");
> > return;
> > @@ -1022,8 +1042,12 @@ void debug_dma_map_page(struct device *dev, struct page *page, size_t offset,
> > if (unlikely(global_disable))
> > return;
> >
> > - if (unlikely(dma_mapping_error(dev, dma_addr)))
> > + if (unlikely(dma_mapping_error(dev, dma_addr))) {
> > + dma_map_errors += 1;
> > + err_printk(dev, NULL,
> > + "DMA-API: dma_map_page() returned error\n");
> > return;
> > + }
>
> So this will print if the dma_map_page failed (which can happen).
Correct. This gets printed DMA DEBUG only mode, whenever mapping fails.
>
> I was initially thinking that this patch would contain a state for the driver
> of whether after map it has called dma_mapping_error. So this function would
> increment some internal state, and if dma_mapping_error on that specific dma_addr
> it would decrement it. If it never occured, then we would print on the unmap
> that the device never had called dma_mapping_error on said dma_addr?
That is a good idea. Let me see if I understand what you are saying
correctly. Add a new field to dma_debug_entry structure and keep state
and clear it if dma_mapping_error() is called. This will require adding
a debug interface for dma_mapping_error() which is not hard to do. Is
this close to what you are thinking?
>
> >
> > entry = dma_entry_alloc();
> > if (!entry)
> > --
> > 1.7.9.5
> >
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists