driver code: clarify and fix platform device DMA mask allocation

This does three inter-related things to clarify the usage of the
platform device dma_mask field. In the process, fix the bug introduced
by 2dfdfdad7cab ("driver core: initialize a default DMA mask for
platform device") that caused Artem Tashkinov's laptop to not boot with
newer Fedora kernels.

This does:

 - First off, rename the field to "platform_dma_mask" to make it
   greppable.

   We have way too many different random fields called "dma_mask" in
   various data structures, where some of them are actual masks, and
   some of them are just pointers to the mask. And the structures all
   have pointers to each other, or embed each other inside themselves,
   and "pdev" sometimes means "platform device" and sometimes it means
   "PCI device".

   So to make it clear in the code when you actually use this new field,
   give it a unique name (it really should be something even more unique
   like "platform_device_dma_mask", since it's per platform device, not
   per platform, but that gets old really fast, and this is unique
   enough in context).

   To further clarify when the field gets used, initialize it when we
   actually start using it with the default value.

 - Then, use this field instead of the random one-off allocation in
   platform_device_register_full() that is now unnecessary since we now
   already have a perfectly fine allocation for it in the platform
   device structure.

 - The above then allows us to fix the actual bug, where the error path
   of platform_device_register_full() would unconditionally free the
   platform device DMA allocation with 'kfree()'.

   That kfree() was dont regardless of whether the allocation had been
   done earlier with the (now removed) kmalloc, or whether
   setup_pdev_dma_masks() had already been used and the dma_mask pointer
   pointed to the mask that was part of the platform device.

It seems most people never triggered the error path, or only triggered
it from a call chain that set an explicit pdevinfo->dma_mask value (and
thus caused the unnecessary allocation that was "cleaned up" in the
error path) before calling platform_device_register_full().

Robin Murphy points out that in Artem's case the wdat_wdt driver failed
in platform_device_add(), and that was the one that had called
platform_device_register_full() with pdevinfo.dma_mask = 0, and would
have caused that kfree() of pdev.dma_mask corrupting the heap.

A later unrelated kmalloc() then oopsed due to the heap corruption.

Fixes: 2dfdfdad7cab ("driver core: initialize a default DMA mask for platform device")
Reported-bisected-and-tested-by:  Artem S. Tashkinov <aros@gmx.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 7fa654f1288b80ffa73e11c49f7d340695d5de9e..b5ce7b0857953e32c90743c39b920a9f61a89f2a 100644 (file)
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -363,10 +363,10 @@ static void setup_pdev_dma_masks(struct platform_device *pdev)
 {
        if (!pdev->dev.coherent_dma_mask)
                pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
-       if (!pdev->dma_mask)
-               pdev->dma_mask = DMA_BIT_MASK(32);
-       if (!pdev->dev.dma_mask)
-               pdev->dev.dma_mask = &pdev->dma_mask;
+       if (!pdev->dev.dma_mask) {
+               pdev->platform_dma_mask = DMA_BIT_MASK(32);
+               pdev->dev.dma_mask = &pdev->platform_dma_mask;
+       }
 };
 
 /**
@@ -662,20 +662,8 @@ struct platform_device *platform_device_register_full(
        pdev->dev.of_node_reused = pdevinfo->of_node_reused;
 
        if (pdevinfo->dma_mask) {
-               /*
-                * This memory isn't freed when the device is put,
-                * I don't have a nice idea for that though.  Conceptually
-                * dma_mask in struct device should not be a pointer.
-                * See http://thread.gmane.org/gmane.linux.kernel.pci/9081
-                */
-               pdev->dev.dma_mask =
-                       kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
-               if (!pdev->dev.dma_mask)
-                       goto err;
-
-               kmemleak_ignore(pdev->dev.dma_mask);
-
-               *pdev->dev.dma_mask = pdevinfo->dma_mask;
+               pdev->platform_dma_mask = pdevinfo->dma_mask;
+               pdev->dev.dma_mask = &pdev->platform_dma_mask;
                pdev->dev.coherent_dma_mask = pdevinfo->dma_mask;
        }
 
@@ -700,7 +688,6 @@ struct platform_device *platform_device_register_full(
        if (ret) {
 err:
                ACPI_COMPANION_SET(&pdev->dev, NULL);
-               kfree(pdev->dev.dma_mask);
                platform_device_put(pdev);
                return ERR_PTR(ret);
        }

diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 276a03c246919ee57be5495808a4043c7abdcaac..041bfa412aa09521813c057fbfc3f273dfda062b 100644 (file)
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -24,7 +24,7 @@ struct platform_device {
        int             id;
        bool            id_auto;
        struct device   dev;
-       u64             dma_mask;
+       u64             platform_dma_mask;
        u32             num_resources;
        struct resource *resource;