diff mbox

[27/67] dma-direct: add dma address sanity checks

Message ID 20171229081911.2802-28-hch@lst.de (mailing list archive)
State Awaiting Upstream
Headers show

Commit Message

Christoph Hellwig Dec. 29, 2017, 8:18 a.m. UTC
Roughly based on the x86 pci-nommu implementation.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 lib/dma-direct.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

Comments

Geert Uytterhoeven Dec. 29, 2017, 2:12 p.m. UTC | #1
Hi Christoph,

On Fri, Dec 29, 2017 at 9:18 AM, Christoph Hellwig <hch@lst.de> wrote:
> Roughly based on the x86 pci-nommu implementation.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Thanks for your patch!

> --- a/lib/dma-direct.c
> +++ b/lib/dma-direct.c
> @@ -9,6 +9,24 @@
>  #include <linux/scatterlist.h>
>  #include <linux/pfn.h>
>
> +#define DIRECT_MAPPING_ERROR           0
> +
> +static bool
> +check_addr(struct device *dev, dma_addr_t dma_addr, size_t size,
> +               const char *caller)
> +{
> +       if (unlikely(dev && !dma_capable(dev, dma_addr, size))) {
> +               if (*dev->dma_mask >= DMA_BIT_MASK(32)) {
> +                       dev_err(dev,
> +                               "%s: overflow %llx+%zu of device mask %llx\n",

Please use "%pad" to format dma_addr_t ...

> +                               caller, (long long)dma_addr, size,

... and use &dma_addr.

> +                               (long long)*dev->dma_mask);

This cast is not needed, as u64 is unsigned long long in kernelspace on
all architectures.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig Jan. 4, 2018, 8:52 a.m. UTC | #2
On Fri, Dec 29, 2017 at 03:12:25PM +0100, Geert Uytterhoeven wrote:
> > +check_addr(struct device *dev, dma_addr_t dma_addr, size_t size,
> > +               const char *caller)
> > +{
> > +       if (unlikely(dev && !dma_capable(dev, dma_addr, size))) {
> > +               if (*dev->dma_mask >= DMA_BIT_MASK(32)) {
> > +                       dev_err(dev,
> > +                               "%s: overflow %llx+%zu of device mask %llx\n",
> 
> Please use "%pad" to format dma_addr_t ...
> 
> > +                               caller, (long long)dma_addr, size,
> 
> ... and use &dma_addr.
> 
> > +                               (long long)*dev->dma_mask);
> 
> This cast is not needed, as u64 is unsigned long long in kernelspace on
> all architectures.

Thanks, fixed.
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/lib/dma-direct.c b/lib/dma-direct.c
index 0e087650e86b..ddd9dcf4e663 100644
--- a/lib/dma-direct.c
+++ b/lib/dma-direct.c
@@ -9,6 +9,24 @@ 
 #include <linux/scatterlist.h>
 #include <linux/pfn.h>
 
+#define DIRECT_MAPPING_ERROR		0
+
+static bool
+check_addr(struct device *dev, dma_addr_t dma_addr, size_t size,
+		const char *caller)
+{
+	if (unlikely(dev && !dma_capable(dev, dma_addr, size))) {
+		if (*dev->dma_mask >= DMA_BIT_MASK(32)) {
+			dev_err(dev,
+				"%s: overflow %llx+%zu of device mask %llx\n",
+				caller, (long long)dma_addr, size,
+				(long long)*dev->dma_mask);
+		}
+		return false;
+	}
+	return true;
+}
+
 static void *dma_direct_alloc(struct device *dev, size_t size,
 		dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
 {
@@ -31,7 +49,11 @@  static dma_addr_t dma_direct_map_page(struct device *dev, struct page *page,
 		unsigned long offset, size_t size, enum dma_data_direction dir,
 		unsigned long attrs)
 {
-	return phys_to_dma(dev, page_to_phys(page)) + offset;
+	dma_addr_t dma_addr = phys_to_dma(dev, page_to_phys(page)) + offset;
+
+	if (!check_addr(dev, dma_addr, size, __func__))
+		return DIRECT_MAPPING_ERROR;
+	return dma_addr;
 }
 
 static int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl,
@@ -44,17 +66,25 @@  static int dma_direct_map_sg(struct device *dev, struct scatterlist *sgl,
 		BUG_ON(!sg_page(sg));
 
 		sg_dma_address(sg) = phys_to_dma(dev, sg_phys(sg));
+		if (!check_addr(dev, sg_dma_address(sg), sg->length, __func__))
+			return 0;
 		sg_dma_len(sg) = sg->length;
 	}
 
 	return nents;
 }
 
+static int dma_direct_mapping_error(struct device *dev, dma_addr_t dma_addr)
+{
+	return dma_addr == DIRECT_MAPPING_ERROR;
+}
+
 const struct dma_map_ops dma_direct_ops = {
 	.alloc			= dma_direct_alloc,
 	.free			= dma_direct_free,
 	.map_page		= dma_direct_map_page,
 	.map_sg			= dma_direct_map_sg,
+	.mapping_error		= dma_direct_mapping_error,
 	.is_phys		= true,
 };
 EXPORT_SYMBOL(dma_direct_ops);