linux-kernel - Re: dma-coherent: fix dma_declare_coherent

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 15 Sep 2017 17:03:33 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Roy Pledge <roy.pledge@....com>
Cc:     Christoph Hellwig <hch@....de>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Robin Murphy <robin.murphy@....com>,
        Vladimir Murzin <vladimir.murzin@....com>,
        Mark Craske <Mark_Craske@...tor.com>,
        "George G. Davis" <george_davis@...tor.com>,
        Bastian Hecht <bhecht@...adit-jv.com>,
        "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: dma-coherent: fix dma_declare_coherent_memory() logic error

>> @@ -338,14 +346,18 @@ static struct reserved_mem *dma_reserved_default_memory __initdata;
>>   static int rmem_dma_device_init(struct reserved_mem *rmem, struct device *dev)
>>   {
>>       struct dma_coherent_mem *mem = rmem->priv;
>> +     int ret;
>> +
>> +     if (!mem)
>> +             return -ENODEV;
>
> When I picked up this change my use of of_reserved_mem_device_init()
> broke. The only place rmem->priv is set is in this function (bottom of
> the patch) so the !mem check above will always fail. Am I missing
> something? It seems to me the intent here was to only call
> dma_init_coherent_memory() once and use rmem->priv as a cache for future
> calls but I'm just looking at the implemation of this for the first time.

Yes, you are right, I misread that code.  I think we need this fixup on top
to restore the previous behavior:

diff --git a/drivers/base/dma-coherent.c b/drivers/base/dma-coherent.c
index a39b2166b145..744f64f43454 100644
--- a/drivers/base/dma-coherent.c
+++ b/drivers/base/dma-coherent.c
@@ -348,16 +348,15 @@ static int rmem_dma_device_init(struct
reserved_mem *rmem, struct device *dev)
        struct dma_coherent_mem *mem = rmem->priv;
        int ret;

-       if (!mem)
-               return -ENODEV;
-
-       ret = dma_init_coherent_memory(rmem->base, rmem->base, rmem->size,
-                                      DMA_MEMORY_EXCLUSIVE, &mem);
-
-       if (ret) {
-               pr_err("Reserved memory: failed to init DMA memory
pool at %pa, size %ld MiB\n",
-                       &rmem->base, (unsigned long)rmem->size / SZ_1M);
-               return ret;
+       if (!mem) {
+               ret = dma_init_coherent_memory(rmem->base, rmem->base,
+                                              rmem->size,
+                                              DMA_MEMORY_EXCLUSIVE, &mem);
+               if (ret) {
+                       pr_err("Reserved memory: failed to init DMA
memory pool at %pa, size %ld MiB\n",
+                               &rmem->base, (unsigned long)rmem->size / SZ_1M);
+                       return ret;
+               }
        }
        mem->use_dev_dma_pfn_offset = true;
        rmem->priv = mem;