Unverified Commit 94cc89eb authored by Charles Keepax's avatar Charles Keepax Committed by Mark Brown
Browse files

regmap: debugfs: Fix handling of name string for debugfs init delays 



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: default avatarCharles Keepax <ckeepax@opensource.cirrus.com>
Link: https://lore.kernel.org/r/20200917120828.12987-1-ckeepax@opensource.cirrus.com


Signed-off-by: default avatarMark Brown <broonie@kernel.org>
parent d012a719

drivers/base/regmap/internal.h

+2 −2
Original line number Diff line number Diff line
@@ -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

drivers/base/regmap/regmap-debugfs.c

+3 −4
Original line number Diff line number Diff line
@@ -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(&regmap_debugfs_early_lock);

		list_add(&node->link, &regmap_debugfs_early_list);

		mutex_unlock(&regmap_debugfs_early_lock);
@@ -679,7 +678,7 @@ void regmap_debugfs_initcall(void) 


	mutex_lock(&regmap_debugfs_early_lock);

	list_for_each_entry_safe(node, tmp, &regmap_debugfs_early_list, link) {

		regmap_debugfs_init(node->map, node->name);

		regmap_debugfs_init(node->map);

		list_del(&node->link);

		kfree(node);

	}

drivers/base/regmap/regmap.c

+33 −11
Original line number Diff line number Diff line
@@ -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);
@@ -674,9 +694,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;
@@ -687,13 +707,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;

	ret = regmap_set_name(map, config);

	if (ret)

		goto err_map;

		}

	}



	if (config->disable_locking) {

		map->lock = map->unlock = regmap_lock_unlock_none;
@@ -1137,7 +1153,7 @@ skip_format_initialization: 
		if (ret != 0)

			goto err_regcache;

	} else {

		regmap_debugfs_init(map, config->name);

		regmap_debugfs_init(map);

	}



	return map;
@@ -1297,6 +1313,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);
@@ -1309,7 +1327,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;

CRA Git | Maintained and supported by SUSTech CRA and CCSE