From: Charles Keepax ckeepax@opensource.cirrus.com
[ Upstream commit 94cc89eb8fa5039fcb6e3e3d50f929ddcccee095 ]
In regmap_debugfs_init the initialisation of the debugfs is delayed if the root node isn't ready yet. Most callers of regmap_debugfs_init pass the name from the regmap_config, which is considered temporary ie. may be unallocated after the regmap_init call returns. This leads to a potential use after free, where config->name has been freed by the time it is used in regmap_debugfs_initcall.
This situation can be seen on Zynq, where the architecture init_irq callback registers a syscon device, using a local variable for the regmap_config. As init_irq is very early in the platform bring up the regmap debugfs root isn't ready yet. Although this doesn't crash it does result in the debugfs entry not having the correct name.
Regmap already sets map->name from config->name on the regmap_init path and the fact that a separate field is used to pass the name to regmap_debugfs_init appears to be an artifact of the debugfs name being added before the map name. As such this patch updates regmap_debugfs_init to use map->name, which is already duplicated from the config avoiding the issue.
This does however leave two lose ends, both regmap_attach_dev and regmap_reinit_cache can be called after a regmap is registered and would have had the effect of applying a new name to the debugfs entries. In both of these cases it was chosen to update the map name. In the case of regmap_attach_dev there are 3 users that currently use this function to update the name, thus doing so avoids changes for those users and it seems reasonable that attaching a device would want to set the name of the map. In the case of regmap_reinit_cache the primary use-case appears to be devices that need some register access to identify the device (for example devices in the same family) and then update the cache to match the exact hardware. Whilst no users do currently update the name here, given the use-case it seemed reasonable the name might want to be updated once the device is better identified.
Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com Link: https://lore.kernel.org/r/20200917120828.12987-1-ckeepax@opensource.cirrus.c... Signed-off-by: Mark Brown broonie@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- drivers/base/regmap/internal.h | 4 +-- drivers/base/regmap/regmap-debugfs.c | 7 ++--- drivers/base/regmap/regmap.c | 44 +++++++++++++++++++++------- 3 files changed, 38 insertions(+), 17 deletions(-)
diff --git a/drivers/base/regmap/internal.h b/drivers/base/regmap/internal.h index 3d80c4b43f720..e0ff8e90ebdcf 100644 --- a/drivers/base/regmap/internal.h +++ b/drivers/base/regmap/internal.h @@ -217,7 +217,7 @@ struct regmap_field {
#ifdef CONFIG_DEBUG_FS extern void regmap_debugfs_initcall(void); -extern void regmap_debugfs_init(struct regmap *map, const char *name); +extern void regmap_debugfs_init(struct regmap *map); extern void regmap_debugfs_exit(struct regmap *map);
static inline void regmap_debugfs_disable(struct regmap *map) @@ -227,7 +227,7 @@ static inline void regmap_debugfs_disable(struct regmap *map)
#else static inline void regmap_debugfs_initcall(void) { } -static inline void regmap_debugfs_init(struct regmap *map, const char *name) { } +static inline void regmap_debugfs_init(struct regmap *map) { } static inline void regmap_debugfs_exit(struct regmap *map) { } static inline void regmap_debugfs_disable(struct regmap *map) { } #endif diff --git a/drivers/base/regmap/regmap-debugfs.c b/drivers/base/regmap/regmap-debugfs.c index f58baff2be0af..b6d63ef16b442 100644 --- a/drivers/base/regmap/regmap-debugfs.c +++ b/drivers/base/regmap/regmap-debugfs.c @@ -17,7 +17,6 @@
struct regmap_debugfs_node { struct regmap *map; - const char *name; struct list_head link; };
@@ -544,11 +543,12 @@ static const struct file_operations regmap_cache_bypass_fops = { .write = regmap_cache_bypass_write_file, };
-void regmap_debugfs_init(struct regmap *map, const char *name) +void regmap_debugfs_init(struct regmap *map) { struct rb_node *next; struct regmap_range_node *range_node; const char *devname = "dummy"; + const char *name = map->name;
/* * Userspace can initiate reads from the hardware over debugfs. @@ -569,7 +569,6 @@ void regmap_debugfs_init(struct regmap *map, const char *name) if (!node) return; node->map = map; - node->name = name; mutex_lock(®map_debugfs_early_lock); list_add(&node->link, ®map_debugfs_early_list); mutex_unlock(®map_debugfs_early_lock); @@ -679,7 +678,7 @@ void regmap_debugfs_initcall(void)
mutex_lock(®map_debugfs_early_lock); list_for_each_entry_safe(node, tmp, ®map_debugfs_early_list, link) { - regmap_debugfs_init(node->map, node->name); + regmap_debugfs_init(node->map); list_del(&node->link); kfree(node); } diff --git a/drivers/base/regmap/regmap.c b/drivers/base/regmap/regmap.c index 927ebde1607be..d9a28f08b5501 100644 --- a/drivers/base/regmap/regmap.c +++ b/drivers/base/regmap/regmap.c @@ -581,14 +581,34 @@ static void regmap_range_exit(struct regmap *map) kfree(map->selector_work_buf); }
+static int regmap_set_name(struct regmap *map, const struct regmap_config *config) +{ + if (config->name) { + const char *name = kstrdup_const(config->name, GFP_KERNEL); + + if (!name) + return -ENOMEM; + + kfree_const(map->name); + map->name = name; + } + + return 0; +} + int regmap_attach_dev(struct device *dev, struct regmap *map, const struct regmap_config *config) { struct regmap **m; + int ret;
map->dev = dev;
- regmap_debugfs_init(map, config->name); + ret = regmap_set_name(map, config); + if (ret) + return ret; + + regmap_debugfs_init(map);
/* Add a devres resource for dev_get_regmap() */ m = devres_alloc(dev_get_regmap_release, sizeof(*m), GFP_KERNEL); @@ -679,9 +699,9 @@ struct regmap *__regmap_init(struct device *dev, const char *lock_name) { struct regmap *map; - int ret = -EINVAL; enum regmap_endian reg_endian, val_endian; int i, j; + int ret;
if (!config) goto err; @@ -692,13 +712,9 @@ struct regmap *__regmap_init(struct device *dev, goto err; }
- if (config->name) { - map->name = kstrdup_const(config->name, GFP_KERNEL); - if (!map->name) { - ret = -ENOMEM; - goto err_map; - } - } + ret = regmap_set_name(map, config); + if (ret) + goto err_map;
if (config->disable_locking) { map->lock = map->unlock = regmap_lock_unlock_none; @@ -1141,7 +1157,7 @@ struct regmap *__regmap_init(struct device *dev, if (ret != 0) goto err_regcache; } else { - regmap_debugfs_init(map, config->name); + regmap_debugfs_init(map); }
return map; @@ -1301,6 +1317,8 @@ EXPORT_SYMBOL_GPL(regmap_field_free); */ int regmap_reinit_cache(struct regmap *map, const struct regmap_config *config) { + int ret; + regcache_exit(map); regmap_debugfs_exit(map);
@@ -1313,7 +1331,11 @@ int regmap_reinit_cache(struct regmap *map, const struct regmap_config *config) map->readable_noinc_reg = config->readable_noinc_reg; map->cache_type = config->cache_type;
- regmap_debugfs_init(map, config->name); + ret = regmap_set_name(map, config); + if (ret) + return ret; + + regmap_debugfs_init(map);
map->cache_bypass = false; map->cache_only = false;