[PATCH] regulator: core: Protect regulator_supply_alias_list with regulator_list_mutex

[PATCH] regulator: core: Protect regulator_supply_alias_list with regulator_list_mutex

[sparkhuang]

regulator_supply_alias_list was accessed without any locking in
regulator_supply_alias(), regulator_register_supply_alias(), and
regulator_unregister_supply_alias(). Concurrent registration,
unregistration and lookups can race, leading to:

1 use-after-free if an alias entry is removed while being read,
2 duplicate entries when two threads register the same alias,
3 inconsistent alias mappings observed by consumers.

Protect all traversals, insertions and deletions on
regulator_supply_alias_list with the existing regulator_list_mutex.

Fixes: a06ccd9c3785f ("regulator: core: Add ability to create a lookup alias for supply")
Signed-off-by: sparkhuang <huangshaobo3@xiaomi.com>
---
 drivers/regulator/core.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index dd7b10e768c0..e809cbae2261 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1942,6 +1942,7 @@ static void regulator_supply_alias(struct device **dev, const char **supply)
 {
 	struct regulator_supply_alias *map;
 
+	mutex_lock(&regulator_list_mutex);
 	map = regulator_find_supply_alias(*dev, *supply);
 	if (map) {
 		dev_dbg(*dev, "Mapping supply %s to %s,%s\n",
@@ -1950,6 +1951,7 @@ static void regulator_supply_alias(struct device **dev, const char **supply)
 		*dev = map->alias_dev;
 		*supply = map->alias_supply;
 	}
+	mutex_unlock(&regulator_list_mutex);
 }
 
 static int regulator_match(struct device *dev, const void *data)
@@ -2492,22 +2494,26 @@ int regulator_register_supply_alias(struct device *dev, const char *id,
 				    const char *alias_id)
 {
 	struct regulator_supply_alias *map;
+	struct regulator_supply_alias *new_map;
 
-	map = regulator_find_supply_alias(dev, id);
-	if (map)
-		return -EEXIST;
-
-	map = kzalloc(sizeof(struct regulator_supply_alias), GFP_KERNEL);
-	if (!map)
+	new_map = kzalloc(sizeof(struct regulator_supply_alias), GFP_KERNEL);
+	if (!new_map)
 		return -ENOMEM;
 
-	map->src_dev = dev;
-	map->src_supply = id;
-	map->alias_dev = alias_dev;
-	map->alias_supply = alias_id;
+	mutex_lock(&regulator_list_mutex);
+	map = regulator_find_supply_alias(dev, id);
+	if (map) {
+		mutex_unlock(&regulator_list_mutex);
+		kfree(new_map);
+		return -EEXIST;
+	}
 
+	new_map->src_dev = dev;
+	new_map->src_supply = id;
+	new_map->alias_dev = alias_dev;
+	new_map->alias_supply = alias_id;
 	list_add(&map->list, &regulator_supply_alias_list);
-
+	mutex_lock(&regulator_list_mutex);
 	pr_info("Adding alias for supply %s,%s -> %s,%s\n",
 		id, dev_name(dev), alias_id, dev_name(alias_dev));
 
@@ -2527,11 +2533,13 @@ void regulator_unregister_supply_alias(struct device *dev, const char *id)
 {
 	struct regulator_supply_alias *map;
 
+	mutex_lock(&regulator_list_mutex);
 	map = regulator_find_supply_alias(dev, id);
 	if (map) {
 		list_del(&map->list);
 		kfree(map);
 	}
+	mutex_unlock(&regulator_list_mutex);
 }
 EXPORT_SYMBOL_GPL(regulator_unregister_supply_alias);
 
-- 
2.34.1

Re: [PATCH] regulator: core: Protect regulator_supply_alias_list with regulator_list_mutex

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 640 bytes --]

On Wed, Nov 26, 2025 at 02:15:42PM +0800, sparkhuang wrote:

> -	map->alias_supply = alias_id;
> +	mutex_lock(&regulator_list_mutex);
> +	map = regulator_find_supply_alias(dev, id);
> +	if (map) {
> +		mutex_unlock(&regulator_list_mutex);
> +		kfree(new_map);
> +		return -EEXIST;
> +	}
>  
> +	new_map->src_dev = dev;
> +	new_map->src_supply = id;
> +	new_map->alias_dev = alias_dev;
> +	new_map->alias_supply = alias_id;
>  	list_add(&map->list, &regulator_supply_alias_list);
> -
> +	mutex_lock(&regulator_list_mutex);

This is locking regulator_list_mutex which is already locked, I think
you meant mutex_unlock().

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] regulator: core: Protect regulator_supply_alias_list with regulator_list_mutex

[Charles Keepax]

On Wed, Nov 26, 2025 at 02:15:42PM +0800, sparkhuang wrote:
> regulator_supply_alias_list was accessed without any locking in
> regulator_supply_alias(), regulator_register_supply_alias(), and
> regulator_unregister_supply_alias(). Concurrent registration,
> unregistration and lookups can race, leading to:
> 
> 1 use-after-free if an alias entry is removed while being read,
> 2 duplicate entries when two threads register the same alias,
> 3 inconsistent alias mappings observed by consumers.
> 
> Protect all traversals, insertions and deletions on
> regulator_supply_alias_list with the existing regulator_list_mutex.
> 
> Fixes: a06ccd9c3785f ("regulator: core: Add ability to create a lookup alias for supply")
> Signed-off-by: sparkhuang <huangshaobo3@xiaomi.com>
> ---
>  {
>  	struct regulator_supply_alias *map;
> +	struct regulator_supply_alias *new_map;
>  
> -	map = regulator_find_supply_alias(dev, id);
> -	if (map)
> -		return -EEXIST;
> -
> -	map = kzalloc(sizeof(struct regulator_supply_alias), GFP_KERNEL);
> -	if (!map)
> +	new_map = kzalloc(sizeof(struct regulator_supply_alias), GFP_KERNEL);
> +	if (!new_map)
>  		return -ENOMEM;
>  
> -	map->src_dev = dev;
> -	map->src_supply = id;
> -	map->alias_dev = alias_dev;
> -	map->alias_supply = alias_id;
> +	mutex_lock(&regulator_list_mutex);
> +	map = regulator_find_supply_alias(dev, id);
> +	if (map) {
> +		mutex_unlock(&regulator_list_mutex);
> +		kfree(new_map);
> +		return -EEXIST;
> +	}
>  
> +	new_map->src_dev = dev;
> +	new_map->src_supply = id;
> +	new_map->alias_dev = alias_dev;
> +	new_map->alias_supply = alias_id;
>  	list_add(&map->list, &regulator_supply_alias_list);

In addition to Mark's comment the first argument here should be
the new item to add. Personally I would be inclined to just leave
the allocation inside the lock and not require the extra variable
but up to you.

Thanks,
Charles

Re: [PATCH] regulator: core: Protect regulator_supply_alias_list with regulator_list_mutex

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 586 bytes --]

On Wed, Nov 26, 2025 at 02:44:06PM +0000, Charles Keepax wrote:
> On Wed, Nov 26, 2025 at 02:15:42PM +0800, sparkhuang wrote:

> > +	new_map->src_supply = id;
> > +	new_map->alias_dev = alias_dev;
> > +	new_map->alias_supply = alias_id;
> >  	list_add(&map->list, &regulator_supply_alias_list);

> In addition to Mark's comment the first argument here should be
> the new item to add. Personally I would be inclined to just leave
> the allocation inside the lock and not require the extra variable
> but up to you.

It is generally nicer to keep the locked region as small as possible.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v2] regulator: core: Protect regulator_supply_alias_list with regulator_list_mutex

[sparkhuang]

regulator_supply_alias_list was accessed without any locking in
regulator_supply_alias(), regulator_register_supply_alias(), and
regulator_unregister_supply_alias(). Concurrent registration,
unregistration and lookups can race, leading to:

1 use-after-free if an alias entry is removed while being read,
2 duplicate entries when two threads register the same alias,
3 inconsistent alias mappings observed by consumers.

Protect all traversals, insertions and deletions on
regulator_supply_alias_list with the existing regulator_list_mutex.

Fixes: a06ccd9c3785f ("regulator: core: Add ability to create a lookup alias for supply")
Signed-off-by: sparkhuang <huangshaobo3@xiaomi.com>
---
v2:
- after list_add, mutex_lock is changed to mutex_unlock.
- the object in list_add has been changed from map to new_map

https://lore.kernel.org/all/20251126061542.3849-1-huangshaobo3@xiaomi.com/
Thanks to Mark Brown and Charles for reviewing
---
---
 drivers/regulator/core.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index dd7b10e768c0..994c3be96f8e 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1942,6 +1942,7 @@ static void regulator_supply_alias(struct device **dev, const char **supply)
 {
 	struct regulator_supply_alias *map;
 
+	mutex_lock(&regulator_list_mutex);
 	map = regulator_find_supply_alias(*dev, *supply);
 	if (map) {
 		dev_dbg(*dev, "Mapping supply %s to %s,%s\n",
@@ -1950,6 +1951,7 @@ static void regulator_supply_alias(struct device **dev, const char **supply)
 		*dev = map->alias_dev;
 		*supply = map->alias_supply;
 	}
+	mutex_unlock(&regulator_list_mutex);
 }
 
 static int regulator_match(struct device *dev, const void *data)
@@ -2492,22 +2494,26 @@ int regulator_register_supply_alias(struct device *dev, const char *id,
 				    const char *alias_id)
 {
 	struct regulator_supply_alias *map;
+	struct regulator_supply_alias *new_map;
 
-	map = regulator_find_supply_alias(dev, id);
-	if (map)
-		return -EEXIST;
-
-	map = kzalloc(sizeof(struct regulator_supply_alias), GFP_KERNEL);
-	if (!map)
+	new_map = kzalloc(sizeof(struct regulator_supply_alias), GFP_KERNEL);
+	if (!new_map)
 		return -ENOMEM;
 
-	map->src_dev = dev;
-	map->src_supply = id;
-	map->alias_dev = alias_dev;
-	map->alias_supply = alias_id;
-
-	list_add(&map->list, &regulator_supply_alias_list);
+	mutex_lock(&regulator_list_mutex);
+	map = regulator_find_supply_alias(dev, id);
+	if (map) {
+		mutex_unlock(&regulator_list_mutex);
+		kfree(new_map);
+		return -EEXIST;
+	}
 
+	new_map->src_dev = dev;
+	new_map->src_supply = id;
+	new_map->alias_dev = alias_dev;
+	new_map->alias_supply = alias_id;
+	list_add(&new_map->list, &regulator_supply_alias_list);
+	mutex_unlock(&regulator_list_mutex);
 	pr_info("Adding alias for supply %s,%s -> %s,%s\n",
 		id, dev_name(dev), alias_id, dev_name(alias_dev));
 
@@ -2527,11 +2533,13 @@ void regulator_unregister_supply_alias(struct device *dev, const char *id)
 {
 	struct regulator_supply_alias *map;
 
+	mutex_lock(&regulator_list_mutex);
 	map = regulator_find_supply_alias(dev, id);
 	if (map) {
 		list_del(&map->list);
 		kfree(map);
 	}
+	mutex_unlock(&regulator_list_mutex);
 }
 EXPORT_SYMBOL_GPL(regulator_unregister_supply_alias);
 
-- 
2.34.1

Re: [PATCH v2] regulator: core: Protect regulator_supply_alias_list with regulator_list_mutex

[Charles Keepax]

On Thu, Nov 27, 2025 at 10:57:16AM +0800, sparkhuang wrote:
> regulator_supply_alias_list was accessed without any locking in
> regulator_supply_alias(), regulator_register_supply_alias(), and
> regulator_unregister_supply_alias(). Concurrent registration,
> unregistration and lookups can race, leading to:
> 
> 1 use-after-free if an alias entry is removed while being read,
> 2 duplicate entries when two threads register the same alias,
> 3 inconsistent alias mappings observed by consumers.
> 
> Protect all traversals, insertions and deletions on
> regulator_supply_alias_list with the existing regulator_list_mutex.
> 
> Fixes: a06ccd9c3785f ("regulator: core: Add ability to create a lookup alias for supply")
> Signed-off-by: sparkhuang <huangshaobo3@xiaomi.com>
> ---

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

Re: [PATCH v2] regulator: core: Protect regulator_supply_alias_list with regulator_list_mutex

[Mark Brown]

On Thu, 27 Nov 2025 10:57:16 +0800, sparkhuang wrote:
> regulator_supply_alias_list was accessed without any locking in
> regulator_supply_alias(), regulator_register_supply_alias(), and
> regulator_unregister_supply_alias(). Concurrent registration,
> unregistration and lookups can race, leading to:
> 
> 1 use-after-free if an alias entry is removed while being read,
> 2 duplicate entries when two threads register the same alias,
> 3 inconsistent alias mappings observed by consumers.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/1] regulator: core: Protect regulator_supply_alias_list with regulator_list_mutex
      commit: 0cc15a10c3b4ab14cd71b779fd5c9ca0cb2bc30d

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark