Skip to content

Commit 17b399c

Browse files
diandersgregkh
authored andcommitted
device property: Make modifications of fwnode "flags" thread safe
commit f72e77c upstream. In various places in the kernel, we modify the fwnode "flags" member by doing either: fwnode->flags |= SOME_FLAG; fwnode->flags &= ~SOME_FLAG; This type of modification is not thread-safe. If two threads are both mucking with the flags at the same time then one can clobber the other. While flags are often modified while under the "fwnode_link_lock", this is not universally true. Create some accessor functions for setting, clearing, and testing the FWNODE flags and move all users to these accessor functions. New accessor functions use set_bit() and clear_bit(), which are thread-safe. Cc: stable@vger.kernel.org Fixes: c2c724c ("driver core: Add fw_devlink_parse_fwtree()") Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Acked-by: Mark Brown <broonie@kernel.org> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Signed-off-by: Douglas Anderson <dianders@chromium.org> Reviewed-by: Rafael J. Wysocki (Intel) <rafael@kernel.org> Reviewed-by: Saravana Kannan <saravanak@kernel.org> Link: https://patch.msgid.link/20260317090112.v2.1.I0a4d03104ecd5103df3d76f66c8d21b1d15a2e38@changeid [ Fix fwnode_clear_flag() argument alignment, restore dropped blank line in fwnode_dev_initialized(), and remove unnecessary parentheses around fwnode_test_flag() calls. - Danilo ] Signed-off-by: Danilo Krummrich <dakr@kernel.org> Signed-off-by: Douglas Anderson <dianders@chromium.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent abc6bdc commit 17b399c

9 files changed

Lines changed: 53 additions & 31 deletions

File tree

drivers/base/core.c

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode)
182182
if (fwnode->dev)
183183
return;
184184

185-
fwnode->flags |= FWNODE_FLAG_NOT_DEVICE;
185+
fwnode_set_flag(fwnode, FWNODE_FLAG_NOT_DEVICE);
186186
fwnode_links_purge_consumers(fwnode);
187187

188188
fwnode_for_each_available_child_node(fwnode, child)
@@ -228,7 +228,7 @@ static void __fw_devlink_pickup_dangling_consumers(struct fwnode_handle *fwnode,
228228
if (fwnode->dev && fwnode->dev->bus)
229229
return;
230230

231-
fwnode->flags |= FWNODE_FLAG_NOT_DEVICE;
231+
fwnode_set_flag(fwnode, FWNODE_FLAG_NOT_DEVICE);
232232
__fwnode_links_move_consumers(fwnode, new_sup);
233233

234234
fwnode_for_each_available_child_node(fwnode, child)
@@ -1013,7 +1013,7 @@ static void device_links_missing_supplier(struct device *dev)
10131013
static bool dev_is_best_effort(struct device *dev)
10141014
{
10151015
return (fw_devlink_best_effort && dev->can_match) ||
1016-
(dev->fwnode && (dev->fwnode->flags & FWNODE_FLAG_BEST_EFFORT));
1016+
(dev->fwnode && fwnode_test_flag(dev->fwnode, FWNODE_FLAG_BEST_EFFORT));
10171017
}
10181018

10191019
static struct fwnode_handle *fwnode_links_check_suppliers(
@@ -1729,11 +1729,11 @@ bool fw_devlink_is_strict(void)
17291729

17301730
static void fw_devlink_parse_fwnode(struct fwnode_handle *fwnode)
17311731
{
1732-
if (fwnode->flags & FWNODE_FLAG_LINKS_ADDED)
1732+
if (fwnode_test_flag(fwnode, FWNODE_FLAG_LINKS_ADDED))
17331733
return;
17341734

17351735
fwnode_call_int_op(fwnode, add_links);
1736-
fwnode->flags |= FWNODE_FLAG_LINKS_ADDED;
1736+
fwnode_set_flag(fwnode, FWNODE_FLAG_LINKS_ADDED);
17371737
}
17381738

17391739
static void fw_devlink_parse_fwtree(struct fwnode_handle *fwnode)
@@ -1892,7 +1892,7 @@ static bool fwnode_init_without_drv(struct fwnode_handle *fwnode)
18921892
struct device *dev;
18931893
bool ret;
18941894

1895-
if (!(fwnode->flags & FWNODE_FLAG_INITIALIZED))
1895+
if (!fwnode_test_flag(fwnode, FWNODE_FLAG_INITIALIZED))
18961896
return false;
18971897

18981898
dev = get_dev_from_fwnode(fwnode);
@@ -1951,10 +1951,10 @@ static bool __fw_devlink_relax_cycles(struct fwnode_handle *con_handle,
19511951
* We aren't trying to find all cycles. Just a cycle between con and
19521952
* sup_handle.
19531953
*/
1954-
if (sup_handle->flags & FWNODE_FLAG_VISITED)
1954+
if (fwnode_test_flag(sup_handle, FWNODE_FLAG_VISITED))
19551955
return false;
19561956

1957-
sup_handle->flags |= FWNODE_FLAG_VISITED;
1957+
fwnode_set_flag(sup_handle, FWNODE_FLAG_VISITED);
19581958

19591959
/* Termination condition. */
19601960
if (sup_handle == con_handle) {
@@ -2024,7 +2024,7 @@ static bool __fw_devlink_relax_cycles(struct fwnode_handle *con_handle,
20242024
}
20252025

20262026
out:
2027-
sup_handle->flags &= ~FWNODE_FLAG_VISITED;
2027+
fwnode_clear_flag(sup_handle, FWNODE_FLAG_VISITED);
20282028
put_device(sup_dev);
20292029
put_device(con_dev);
20302030
put_device(par_dev);
@@ -2077,7 +2077,7 @@ static int fw_devlink_create_devlink(struct device *con,
20772077
* When such a flag is set, we can't create device links where P is the
20782078
* supplier of C as that would delay the probe of C.
20792079
*/
2080-
if (sup_handle->flags & FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD &&
2080+
if (fwnode_test_flag(sup_handle, FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD) &&
20812081
fwnode_is_ancestor_of(sup_handle, con->fwnode))
20822082
return -EINVAL;
20832083

@@ -2100,7 +2100,7 @@ static int fw_devlink_create_devlink(struct device *con,
21002100
else
21012101
flags = FW_DEVLINK_FLAGS_PERMISSIVE;
21022102

2103-
if (sup_handle->flags & FWNODE_FLAG_NOT_DEVICE)
2103+
if (fwnode_test_flag(sup_handle, FWNODE_FLAG_NOT_DEVICE))
21042104
sup_dev = fwnode_get_next_parent_dev(sup_handle);
21052105
else
21062106
sup_dev = get_dev_from_fwnode(sup_handle);
@@ -2112,7 +2112,7 @@ static int fw_devlink_create_devlink(struct device *con,
21122112
* supplier device indefinitely.
21132113
*/
21142114
if (sup_dev->links.status == DL_DEV_NO_DRIVER &&
2115-
sup_handle->flags & FWNODE_FLAG_INITIALIZED) {
2115+
fwnode_test_flag(sup_handle, FWNODE_FLAG_INITIALIZED)) {
21162116
dev_dbg(con,
21172117
"Not linking %pfwf - dev might never probe\n",
21182118
sup_handle);

drivers/bus/imx-weim.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ static int of_weim_notify(struct notifier_block *nb, unsigned long action,
335335
* fw_devlink doesn't skip adding consumers to this
336336
* device.
337337
*/
338-
rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
338+
fwnode_clear_flag(&rd->dn->fwnode, FWNODE_FLAG_NOT_DEVICE);
339339
if (!of_platform_device_create(rd->dn, NULL, &pdev->dev)) {
340340
dev_err(&pdev->dev,
341341
"Failed to create child device '%pOF'\n",

drivers/i2c/i2c-core-of.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ static int of_i2c_notify(struct notifier_block *nb, unsigned long action,
182182
* Clear the flag before adding the device so that fw_devlink
183183
* doesn't skip adding consumers to this device.
184184
*/
185-
rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
185+
fwnode_clear_flag(&rd->dn->fwnode, FWNODE_FLAG_NOT_DEVICE);
186186
client = of_i2c_register_device(adap, rd->dn);
187187
if (IS_ERR(client)) {
188188
dev_err(&adap->dev, "failed to create client for '%pOF'\n",

drivers/net/phy/mdio_bus.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -675,8 +675,8 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
675675
return -EINVAL;
676676

677677
if (bus->parent && bus->parent->of_node)
678-
bus->parent->of_node->fwnode.flags |=
679-
FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD;
678+
fwnode_set_flag(&bus->parent->of_node->fwnode,
679+
FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD);
680680

681681
WARN(bus->state != MDIOBUS_ALLOCATED &&
682682
bus->state != MDIOBUS_UNREGISTERED,

drivers/of/base.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1759,7 +1759,7 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
17591759
if (name)
17601760
of_stdout = of_find_node_opts_by_path(name, &of_stdout_options);
17611761
if (of_stdout)
1762-
of_stdout->fwnode.flags |= FWNODE_FLAG_BEST_EFFORT;
1762+
fwnode_set_flag(&of_stdout->fwnode, FWNODE_FLAG_BEST_EFFORT);
17631763
}
17641764

17651765
if (!of_aliases)

drivers/of/dynamic.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ static void __of_attach_node(struct device_node *np)
224224
np->sibling = np->parent->child;
225225
np->parent->child = np;
226226
of_node_clear_flag(np, OF_DETACHED);
227-
np->fwnode.flags |= FWNODE_FLAG_NOT_DEVICE;
227+
fwnode_set_flag(&np->fwnode, FWNODE_FLAG_NOT_DEVICE);
228228

229229
raw_spin_unlock_irqrestore(&devtree_lock, flags);
230230

drivers/of/platform.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -774,7 +774,7 @@ static int of_platform_notify(struct notifier_block *nb,
774774
* Clear the flag before adding the device so that fw_devlink
775775
* doesn't skip adding consumers to this device.
776776
*/
777-
rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
777+
fwnode_clear_flag(&rd->dn->fwnode, FWNODE_FLAG_NOT_DEVICE);
778778
/* pdev_parent may be NULL when no bus platform device */
779779
pdev_parent = of_find_device_by_node(rd->dn->parent);
780780
pdev = of_platform_device_create(rd->dn, NULL,

drivers/spi/spi.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4532,7 +4532,7 @@ static int of_spi_notify(struct notifier_block *nb, unsigned long action,
45324532
* Clear the flag before adding the device so that fw_devlink
45334533
* doesn't skip adding consumers to this device.
45344534
*/
4535-
rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE;
4535+
fwnode_clear_flag(&rd->dn->fwnode, FWNODE_FLAG_NOT_DEVICE);
45364536
spi = of_register_spi_device(ctlr, rd->dn);
45374537
put_device(&ctlr->dev);
45384538

include/linux/fwnode.h

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include <linux/types.h>
1313
#include <linux/list.h>
1414
#include <linux/bits.h>
15+
#include <linux/bitops.h>
1516
#include <linux/err.h>
1617

1718
struct fwnode_operations;
@@ -31,20 +32,20 @@ struct device;
3132
* suppliers. Only enforce ordering with suppliers that have
3233
* drivers.
3334
*/
34-
#define FWNODE_FLAG_LINKS_ADDED BIT(0)
35-
#define FWNODE_FLAG_NOT_DEVICE BIT(1)
36-
#define FWNODE_FLAG_INITIALIZED BIT(2)
37-
#define FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD BIT(3)
38-
#define FWNODE_FLAG_BEST_EFFORT BIT(4)
39-
#define FWNODE_FLAG_VISITED BIT(5)
35+
#define FWNODE_FLAG_LINKS_ADDED 0
36+
#define FWNODE_FLAG_NOT_DEVICE 1
37+
#define FWNODE_FLAG_INITIALIZED 2
38+
#define FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD 3
39+
#define FWNODE_FLAG_BEST_EFFORT 4
40+
#define FWNODE_FLAG_VISITED 5
4041

4142
struct fwnode_handle {
4243
struct fwnode_handle *secondary;
4344
const struct fwnode_operations *ops;
4445
struct device *dev;
4546
struct list_head suppliers;
4647
struct list_head consumers;
47-
u8 flags;
48+
unsigned long flags;
4849
};
4950

5051
/*
@@ -197,16 +198,37 @@ static inline void fwnode_init(struct fwnode_handle *fwnode,
197198
INIT_LIST_HEAD(&fwnode->suppliers);
198199
}
199200

201+
static inline void fwnode_set_flag(struct fwnode_handle *fwnode,
202+
unsigned int bit)
203+
{
204+
set_bit(bit, &fwnode->flags);
205+
}
206+
207+
static inline void fwnode_clear_flag(struct fwnode_handle *fwnode,
208+
unsigned int bit)
209+
{
210+
clear_bit(bit, &fwnode->flags);
211+
}
212+
213+
static inline void fwnode_assign_flag(struct fwnode_handle *fwnode,
214+
unsigned int bit, bool value)
215+
{
216+
assign_bit(bit, &fwnode->flags, value);
217+
}
218+
219+
static inline bool fwnode_test_flag(struct fwnode_handle *fwnode,
220+
unsigned int bit)
221+
{
222+
return test_bit(bit, &fwnode->flags);
223+
}
224+
200225
static inline void fwnode_dev_initialized(struct fwnode_handle *fwnode,
201226
bool initialized)
202227
{
203228
if (IS_ERR_OR_NULL(fwnode))
204229
return;
205230

206-
if (initialized)
207-
fwnode->flags |= FWNODE_FLAG_INITIALIZED;
208-
else
209-
fwnode->flags &= ~FWNODE_FLAG_INITIALIZED;
231+
fwnode_assign_flag(fwnode, FWNODE_FLAG_INITIALIZED, initialized);
210232
}
211233

212234
extern bool fw_devlink_is_strict(void);

0 commit comments

Comments
 (0)