platform: Fix race condition during DMA configure at IOMMU probe time

To avoid a race between the IOMMU probing thread and the device driver
async probing thread during configuration of the platform

platform: Fix race condition during DMA configure at IOMMU probe time

To avoid a race between the IOMMU probing thread and the device driver
async probing thread during configuration of the platform DMA, update
`platform_dma_configure()` to read `dev->driver` once and test if it's
NULL before using it. This ensures that we don't de-reference an invalid
platform driver pointer if the device driver is asynchronously bound
while configuring the DMA.

Fixes: bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper probe path")
Signed-off-by: Will McVicker <[email protected]>
Reviewed-by: Robin Murphy <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

iommu: Get DT/ACPI parsing into the proper probe path

In hindsight, there were some crucial subtleties overlooked when moving
{of,acpi}_dma_configure() to driver probe time to allow waiting for
IOMM

iommu: Get DT/ACPI parsing into the proper probe path

In hindsight, there were some crucial subtleties overlooked when moving
{of,acpi}_dma_configure() to driver probe time to allow waiting for
IOMMU drivers with -EPROBE_DEFER, and these have become an
ever-increasing source of problems. The IOMMU API has some fundamental
assumptions that iommu_probe_device() is called for every device added
to the system, in the order in which they are added. Calling it in a
random order or not at all dependent on driver binding leads to
malformed groups, a potential lack of isolation for devices with no
driver, and all manner of unexpected concurrency and race conditions.
We've attempted to mitigate the latter with point-fix bodges like
iommu_probe_device_lock, but it's a losing battle and the time has come
to bite the bullet and address the true source of the problem instead.

The crux of the matter is that the firmware parsing actually serves two
distinct purposes; one is identifying the IOMMU instance associated with
a device so we can check its availability, the second is actually
telling that instance about the relevant firmware-provided data for the
device. However the latter also depends on the former, and at the time
there was no good place to defer and retry that separately from the
availability check we also wanted for client driver probe.

Nowadays, though, we have a proper notion of multiple IOMMU instances in
the core API itself, and each one gets a chance to probe its own devices
upon registration, so we can finally make that work as intended for
DT/IORT/VIOT platforms too. All we need is for iommu_probe_device() to
be able to run the iommu_fwspec machinery currently buried deep in the
wrong end of {of,acpi}_dma_configure(). Luckily it turns out to be
surprisingly straightforward to bootstrap this transformation by pretty
much just calling the same path twice. At client driver probe time,
dev->driver is obviously set; conversely at device_add(), or a
subsequent bus_iommu_probe(), any device waiting for an IOMMU really
should *not* have a driver already, so we can use that as a condition to
disambiguate the two cases, and avoid recursing back into the IOMMU core
at the wrong times.

Obviously this isn't the nicest thing, but for now it gives us a
functional baseline to then unpick the layers in between without many
more awkward cross-subsystem patches. There are some minor side-effects
like dma_range_map potentially being created earlier, and some debug
prints being repeated, but these aren't significantly detrimental. Let's
make things work first, then deal with making them nice.

With the basic flow finally in the right order again, the next step is
probably turning the bus->dma_configure paths inside-out, since all we
really need from bus code is its notion of which device and input ID(s)
to parse the common firmware properties with...

Acked-by: Bjorn Helgaas <[email protected]> # pci-driver.c
Acked-by: Rob Herring (Arm) <[email protected]> # of/device.c
Signed-off-by: Robin Murphy <[email protected]>
Reviewed-by: Lorenzo Pieralisi <[email protected]>
Reviewed-by: Jason Gunthorpe <[email protected]>
Link: https://lore.kernel.org/r/e3b191e6fd6ca9a1e84c5e5e40044faf97abb874.1740753261.git.robin.murphy@arm.com
Signed-off-by: Joerg Roedel <[email protected]>

show more ...

platform: Make platform_bus_type constant

Since commit d492cc2573a0 ("driver core: device.h: make struct
bus_type a const *"), the driver core can properly handle constant
struct bus_type, move the

platform: Make platform_bus_type constant

Since commit d492cc2573a0 ("driver core: device.h: make struct
bus_type a const *"), the driver core can properly handle constant
struct bus_type, move the platform_bus_type variable to be a constant
structure as well, placing it into read-only memory which can not be
modified at runtime.

Cc: Greg Kroah-Hartman <[email protected]>
Suggested-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Kunwu Chan <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

driver core: platform: Switch to use kmemdup_array()

Let the kememdup_array() take care about multiplication and possible
overflows.

Signed-off-by: Andy Shevchenko <[email protected]

driver core: platform: Switch to use kmemdup_array()

Let the kememdup_array() take care about multiplication and possible
overflows.

Signed-off-by: Andy Shevchenko <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

driver core: have match() callback in struct bus_type take a const *

In the match() callback, the struct device_driver * should not be
changed, so change the function callback to be a const *. This

driver core: have match() callback in struct bus_type take a const *

In the match() callback, the struct device_driver * should not be
changed, so change the function callback to be a const *. This is one
step of many towards making the driver core safe to have struct
device_driver in read-only memory.

Because the match() callback is in all busses, all busses are modified
to handle this properly. This does entail switching some container_of()
calls to container_of_const() to properly handle the constant *.

For some busses, like PCI and USB and HV, the const * is cast away in
the match callback as those busses do want to modify those structures at
this point in time (they have a local lock in the driver structure.)
That will have to be changed in the future if they wish to have their
struct device * in read-only-memory.

Cc: Rafael J. Wysocki <[email protected]>
Reviewed-by: Alex Elder <[email protected]>
Acked-by: Sumit Garg <[email protected]>
Link: https://lore.kernel.org/r/2024070136-wrongdoer-busily-01e8@gregkh
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

driver core: platform: fix ups for constant struct device_driver

Fix up a few places in the platform core code that can easily handle
struct device_driver being constant. This is part of the work t

driver core: platform: fix ups for constant struct device_driver

Fix up a few places in the platform core code that can easily handle
struct device_driver being constant. This is part of the work to make
all struct device_driver pointers be constant.

Cc: Rafael J. Wysocki <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

platform: Make platform_driver::remove() return void

struct platform_driver::remove returning an integer made driver authors
expect that returning an error code was proper error handling. However
th

platform: Make platform_driver::remove() return void

struct platform_driver::remove returning an integer made driver authors
expect that returning an error code was proper error handling. However
the driver core ignores the error and continues to remove the device
because there is nothing the core could do anyhow and reentering the
remove callback again is only calling for trouble.

To prevent such wrong assumptions, change the return type of the remove
callback to void. This was prepared by introducing an alternative remove
callback returning void and converting all drivers to that. So .remove()
can be changed without further changes in drivers.

This corresponds to step b) of the plan outlined in commit
5c5a7680e67b ("platform: Provide a remove callback that returns no value").

Signed-off-by: Uwe Kleine-König <[email protected]>

show more ...

driver core: platform: Annotate struct irq_affinity_devres with __counted_by

Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated wi

driver core: platform: Annotate struct irq_affinity_devres with __counted_by

Prepare for the coming implementation by GCC and Clang of the __counted_by
attribute. Flexible array members annotated with __counted_by can have
their accesses bounds-checked at run-time via CONFIG_UBSAN_BOUNDS (for
array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family
functions).

As found with Coccinelle[1], add __counted_by for struct
irq_affinity_devres.

Cc: Greg Kroah-Hartman <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: "Gustavo A. R. Silva" <[email protected]>
Cc: [email protected]
Link: https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci [1]
Signed-off-by: Kees Cook <[email protected]>
Reviewed-by: "Gustavo A. R. Silva" <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

driver core: platform: Unify the firmware node type check

OF and ACPI currently are using asymmetrical APIs to check
for the firmware node type. Unify them by using is_*_node()
against struct fwnode

driver core: platform: Unify the firmware node type check

OF and ACPI currently are using asymmetrical APIs to check
for the firmware node type. Unify them by using is_*_node()
against struct fwnode_handle pointer.

Signed-off-by: Andy Shevchenko <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

driver core: platform: Use temporary variable in platform_device_add()

With the temporary variable for the struct device pointer the code
looks better and slightly easier to read and parse by human

driver core: platform: Use temporary variable in platform_device_add()

With the temporary variable for the struct device pointer the code
looks better and slightly easier to read and parse by human being.

Signed-off-by: Andy Shevchenko <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

driver core: platform: Refactor error path in a couple places

The usual pattern is to bail out on the error case. Besides that
one of the labels is redundant as we may return directly. Refactor
plat

driver core: platform: Refactor error path in a couple places

The usual pattern is to bail out on the error case. Besides that
one of the labels is redundant as we may return directly. Refactor
platform_device_add() and platform_dma_configure() accordingly.

Signed-off-by: Andy Shevchenko <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

driver core: platform: Drop redundant check in platform_device_add()

Starting from the commit 37c12e7497b6 ("[DRIVER MODEL] Improved
dynamically allocated platform_device interface") the pdev expect

driver core: platform: Drop redundant check in platform_device_add()

Starting from the commit 37c12e7497b6 ("[DRIVER MODEL] Improved
dynamically allocated platform_device interface") the pdev expects
to be allocated beforehand or guaranteed to be non-NULL.

Hence the leftover check is now redundant (as we have no combined
calls like platform_device_add(platform_device_alloc(...)) in the
entire kernel source code.

Signed-off-by: Andy Shevchenko <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

of: Move of_platform_register_reconfig_notifier() into DT core

There's no reason the generic platform bus code needs to call
of_platform_register_reconfig_notifier(). The notifier can be setup
befor

of: Move of_platform_register_reconfig_notifier() into DT core

There's no reason the generic platform bus code needs to call
of_platform_register_reconfig_notifier(). The notifier can be setup
before the platform bus is. Let's move it into of_core_init() which is
called just before platform_bus_init() instead to keep more of the DT
bits in the DT code.

Reviewed-by: Greg Kroah-Hartman <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Rob Herring <[email protected]>

show more ...

driver core: platform: simplify __platform_driver_probe()

__platform_driver_probe() pokes around in some bus and driver private
lists and locks in a way that is not needed at all. The code only wan

driver core: platform: simplify __platform_driver_probe()

__platform_driver_probe() pokes around in some bus and driver private
lists and locks in a way that is not needed at all. The code only wants
to know if a device was bound to the driver that was registered, so walk
all devices on the bus to see if there was a match. If there is not a
match, return an error. This is the same logic as was originally
present, but just done in a simpler and more obvious way that is not a
layering violation.

Cc: "Rafael J. Wysocki" <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

driver core: platform: removed unneeded variable from __platform_driver_probe()

In the reworking of the function __platform_driver_probe() over the
years, it turns out that the variable 'code' does

driver core: platform: removed unneeded variable from __platform_driver_probe()

In the reworking of the function __platform_driver_probe() over the
years, it turns out that the variable 'code' does not actually do
anything or mean anything anymore and can be removed to simplify the
logic when trying to read and understand what this function is actually
doing.

Cc: "Rafael J. Wysocki" <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

driver core: make struct bus_type.uevent() take a const *

The uevent() callback in struct bus_type should not be modifying the
device that is passed into it, so mark it as a const * and propagate th

driver core: make struct bus_type.uevent() take a const *

The uevent() callback in struct bus_type should not be modifying the
device that is passed into it, so mark it as a const * and propagate the
function signature changes out into all relevant subsystems that use
this callback.

Acked-by: Rafael J. Wysocki <[email protected]>
Acked-by: Hans de Goede <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

platform: remove useless if-branch in __platform_get_irq_byname()

When CONFIG_OF_IRQ is not enabled, there will be a stub method that always
returns 0 when getting IRQ. Thus, the if-branch can be re

platform: remove useless if-branch in __platform_get_irq_byname()

When CONFIG_OF_IRQ is not enabled, there will be a stub method that always
returns 0 when getting IRQ. Thus, the if-branch can be removed safely.

Signed-off-by: Soha Jin <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

platform: Document platform_add_devices() return value

platform_add_devices() returns 0 on success and negative
errno on failure. Document it.

Signed-off-by: Umang Jain <[email protected]

platform: Document platform_add_devices() return value

platform_add_devices() returns 0 on success and negative
errno on failure. Document it.

Signed-off-by: Umang Jain <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

platform: Provide a remove callback that returns no value

struct platform_driver::remove returning an integer made driver authors
expect that returning an error code was proper error handling. Howev

platform: Provide a remove callback that returns no value

struct platform_driver::remove returning an integer made driver authors
expect that returning an error code was proper error handling. However
the driver core ignores the error and continues to remove the device
because there is nothing the core could do anyhow and reentering the
remove callback again is only calling for trouble.

So this is an source for errors typically yielding resource leaks in the
error path.

As there are too many platform drivers to neatly convert them all to
return void in a single go, do it in several steps after this patch:

a) Convert all drivers to implement .remove_new() returning void instead
of .remove() returning int;
b) Change struct platform_driver::remove() to return void and so make
it identical to .remove_new();
c) Change all drivers back to .remove() now with the better prototype;
d) drop struct platform_driver::remove_new().

While this touches all drivers eventually twice, steps a) and c) can be
done one driver after another and so reduces coordination efforts
immensely and simplifies review.

Signed-off-by: Uwe Kleine-König <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

platform: use fwnode_irq_get_byname instead of of_irq_get_byname to get irq

Not only platform devices described by OF have named interrupts, but
devices described by ACPI also have named interrupts.

platform: use fwnode_irq_get_byname instead of of_irq_get_byname to get irq

Not only platform devices described by OF have named interrupts, but
devices described by ACPI also have named interrupts. The fwnode is an
abstraction to different standards, and using fwnode_irq_get_byname can
support more devices.

Signed-off-by: Soha Jin <[email protected]>
Tested-by: Wende Tan <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

bus: platform,amba,fsl-mc,PCI: Add device DMA ownership management

The devices on platform/amba/fsl-mc/PCI buses could be bound to drivers
with the device DMA managed by kernel drivers or user-space

bus: platform,amba,fsl-mc,PCI: Add device DMA ownership management

The devices on platform/amba/fsl-mc/PCI buses could be bound to drivers
with the device DMA managed by kernel drivers or user-space applications.
Unfortunately, multiple devices may be placed in the same IOMMU group
because they cannot be isolated from each other. The DMA on these devices
must either be entirely under kernel control or userspace control, never
a mixture. Otherwise the driver integrity is not guaranteed because they
could access each other through the peer-to-peer accesses which by-pass
the IOMMU protection.

This checks and sets the default DMA mode during driver binding, and
cleanups during driver unbinding. In the default mode, the device DMA is
managed by the device driver which handles DMA operations through the
kernel DMA APIs (see Documentation/core-api/dma-api.rst).

For cases where the devices are assigned for userspace control through the
userspace driver framework(i.e. VFIO), the drivers(for example, vfio_pci/
vfio_platfrom etc.) may set a new flag (driver_managed_dma) to skip this
default setting in the assumption that the drivers know what they are
doing with the device DMA.

Calling iommu_device_use_default_domain() before {of,acpi}_dma_configure
is currently a problem. As things stand, the IOMMU driver ignored the
initial iommu_probe_device() call when the device was added, since at
that point it had no fwspec yet. In this situation,
{of,acpi}_iommu_configure() are retriggering iommu_probe_device() after
the IOMMU driver has seen the firmware data via .of_xlate to learn that
it actually responsible for the given device. As the result, before
that gets fixed, iommu_use_default_domain() goes at the end, and calls
arch_teardown_dma_ops() if it fails.

Cc: Greg Kroah-Hartman <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: Stuart Yoder <[email protected]>
Cc: Laurentiu Tudor <[email protected]>
Signed-off-by: Lu Baolu <[email protected]>
Reviewed-by: Greg Kroah-Hartman <[email protected]>
Reviewed-by: Jason Gunthorpe <[email protected]>
Reviewed-by: Robin Murphy <[email protected]>
Tested-by: Eric Auger <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Joerg Roedel <[email protected]>

show more ...

amba: Stop sharing platform_dma_configure()

Stop sharing platform_dma_configure() helper as they are about to have
their own bus dma_configure callbacks.

Signed-off-by: Lu Baolu <[email protected]

amba: Stop sharing platform_dma_configure()

Stop sharing platform_dma_configure() helper as they are about to have
their own bus dma_configure callbacks.

Signed-off-by: Lu Baolu <[email protected]>
Reviewed-by: Jason Gunthorpe <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Joerg Roedel <[email protected]>

show more ...

platform: finally disallow IRQ0 in platform_get_irq() and its ilk

The commit a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is
invalid") only calls WARN() when IRQ0 is about to be returned

platform: finally disallow IRQ0 in platform_get_irq() and its ilk

The commit a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is
invalid") only calls WARN() when IRQ0 is about to be returned, however
using IRQ0 is considered invalid (according to Linus) outside the arch/
code where it's used by the i8253 drivers. Many driver subsystems treat
0 specially (e.g. as an indication of the polling mode by libata), so
the users of platform_get_irq[_byname]() in them would have to filter
out IRQ0 explicitly and this (quite obviously) doesn't scale...
Let's finally get this straight and return -EINVAL instead of IRQ0!

Fixes: a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is invalid")
Acked-by: Marc Zyngier <[email protected]>
Signed-off-by: Sergey Shtylyov <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

driver: platform: Add helper for safer setting of driver_override

Several core drivers and buses expect that driver_override is a
dynamically allocated memory thus later they can kfree() it.

Howeve

driver: platform: Add helper for safer setting of driver_override

Several core drivers and buses expect that driver_override is a
dynamically allocated memory thus later they can kfree() it.

However such assumption is not documented, there were in the past and
there are already users setting it to a string literal. This leads to
kfree() of static memory during device release (e.g. in error paths or
during unbind):

kernel BUG at ../mm/slub.c:3960!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP ARM
...
(kfree) from [<c058da50>] (platform_device_release+0x88/0xb4)
(platform_device_release) from [<c0585be0>] (device_release+0x2c/0x90)
(device_release) from [<c0a69050>] (kobject_put+0xec/0x20c)
(kobject_put) from [<c0f2f120>] (exynos5_clk_probe+0x154/0x18c)
(exynos5_clk_probe) from [<c058de70>] (platform_drv_probe+0x6c/0xa4)
(platform_drv_probe) from [<c058b7ac>] (really_probe+0x280/0x414)
(really_probe) from [<c058baf4>] (driver_probe_device+0x78/0x1c4)
(driver_probe_device) from [<c0589854>] (bus_for_each_drv+0x74/0xb8)
(bus_for_each_drv) from [<c058b48c>] (__device_attach+0xd4/0x16c)
(__device_attach) from [<c058a638>] (bus_probe_device+0x88/0x90)
(bus_probe_device) from [<c05871fc>] (device_add+0x3dc/0x62c)
(device_add) from [<c075ff10>] (of_platform_device_create_pdata+0x94/0xbc)
(of_platform_device_create_pdata) from [<c07600ec>] (of_platform_bus_create+0x1a8/0x4fc)
(of_platform_bus_create) from [<c0760150>] (of_platform_bus_create+0x20c/0x4fc)
(of_platform_bus_create) from [<c07605f0>] (of_platform_populate+0x84/0x118)
(of_platform_populate) from [<c0f3c964>] (of_platform_default_populate_init+0xa0/0xb8)
(of_platform_default_populate_init) from [<c01031f8>] (do_one_initcall+0x8c/0x404)

Provide a helper which clearly documents the usage of driver_override.
This will allow later to reuse the helper and reduce the amount of
duplicated code.

Convert the platform driver to use a new helper and make the
driver_override field const char (it is not modified by the core).

Reviewed-by: Rafael J. Wysocki <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
Signed-off-by: Krzysztof Kozlowski <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...

platform: use dev_err_probe() in platform_get_irq_byname()

The commit 2043727c2882 ("driver core: platform: Make use of the helper
function dev_err_probe()") missed to also convert platform_get_irq_

platform: use dev_err_probe() in platform_get_irq_byname()

The commit 2043727c2882 ("driver core: platform: Make use of the helper
function dev_err_probe()") missed to also convert platform_get_irq_byname()
for some strange reason -- do that now.

Signed-off-by: Sergey Shtylyov <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>

show more ...