regulator: fix memory leak on error path of regulator_register()
The change corrects registration and deregistration on error path
of a regulator, the problem was manifested by a reported memory
leak on deferred probe:
    as3722-regulator as3722-regulator: regulator 13 register failed -517
    # cat /sys/kernel/debug/kmemleak
    unreferenced object 0xecc43740 (size 64):
      comm "swapper/0", pid 1, jiffies 4294937640 (age 712.880s)
      hex dump (first 32 bytes):
        72 65 67 75 6c 61 74 6f 72 2e 32 34 00 5a 5a 5a  regulator.24.ZZZ
        5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a  ZZZZZZZZZZZZZZZZ
      backtrace:
        [<0c4c3d1c>] __kmalloc_track_caller+0x15c/0x2c0
        [<40c0ad48>] kvasprintf+0x64/0xd4
        [<109abd29>] kvasprintf_const+0x70/0x84
        [<c4215946>] kobject_set_name_vargs+0x34/0xa8
        [<62282ea2>] dev_set_name+0x40/0x64
        [<a39b6757>] regulator_register+0x3a4/0x1344
        [<16a9543f>] devm_regulator_register+0x4c/0x84
        [<51a4c6a1>] as3722_regulator_probe+0x294/0x754
        ...
The memory leak problem was introduced as a side ef another fix in
regulator_register() error path, I believe that the proper fix is
to decouple device_register() function into its two compounds and
initialize a struct device before assigning any values to its fields
and then using it before actual registration of a device happens.
This lets to call put_device() safely after initialization, and, since
now a release callback is called, kfree(rdev->constraints) shall be
removed to exclude a double free condition.
Fixes: a3cde9534e ("regulator: core: fix regulator_register() error paths to properly release rdev")
Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
Cc: Wen Yang <wenyang@linux.alibaba.com>
Link: https://lore.kernel.org/r/20200724005013.23278-1-vz@mleia.com
Signed-off-by: Mark Brown <broonie@kernel.org>
			
			
This commit is contained in:
		
							parent
							
								
									2ca76b3e49
								
							
						
					
					
						commit
						9177514ce3
					
				| @ -5092,7 +5092,6 @@ regulator_register(const struct regulator_desc *regulator_desc, | |||||||
| 	struct regulator_dev *rdev; | 	struct regulator_dev *rdev; | ||||||
| 	bool dangling_cfg_gpiod = false; | 	bool dangling_cfg_gpiod = false; | ||||||
| 	bool dangling_of_gpiod = false; | 	bool dangling_of_gpiod = false; | ||||||
| 	bool reg_device_fail = false; |  | ||||||
| 	struct device *dev; | 	struct device *dev; | ||||||
| 	int ret, i; | 	int ret, i; | ||||||
| 
 | 
 | ||||||
| @ -5221,10 +5220,12 @@ regulator_register(const struct regulator_desc *regulator_desc, | |||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	/* register with sysfs */ | 	/* register with sysfs */ | ||||||
|  | 	device_initialize(&rdev->dev); | ||||||
| 	rdev->dev.class = ®ulator_class; | 	rdev->dev.class = ®ulator_class; | ||||||
| 	rdev->dev.parent = dev; | 	rdev->dev.parent = dev; | ||||||
| 	dev_set_name(&rdev->dev, "regulator.%lu", | 	dev_set_name(&rdev->dev, "regulator.%lu", | ||||||
| 		    (unsigned long) atomic_inc_return(®ulator_no)); | 		    (unsigned long) atomic_inc_return(®ulator_no)); | ||||||
|  | 	dev_set_drvdata(&rdev->dev, rdev); | ||||||
| 
 | 
 | ||||||
| 	/* set regulator constraints */ | 	/* set regulator constraints */ | ||||||
| 	if (init_data) | 	if (init_data) | ||||||
| @ -5275,12 +5276,9 @@ regulator_register(const struct regulator_desc *regulator_desc, | |||||||
| 	    !rdev->desc->fixed_uV) | 	    !rdev->desc->fixed_uV) | ||||||
| 		rdev->is_switch = true; | 		rdev->is_switch = true; | ||||||
| 
 | 
 | ||||||
| 	dev_set_drvdata(&rdev->dev, rdev); | 	ret = device_add(&rdev->dev); | ||||||
| 	ret = device_register(&rdev->dev); | 	if (ret != 0) | ||||||
| 	if (ret != 0) { |  | ||||||
| 		reg_device_fail = true; |  | ||||||
| 		goto unset_supplies; | 		goto unset_supplies; | ||||||
| 	} |  | ||||||
| 
 | 
 | ||||||
| 	rdev_init_debugfs(rdev); | 	rdev_init_debugfs(rdev); | ||||||
| 
 | 
 | ||||||
| @ -5302,17 +5300,15 @@ unset_supplies: | |||||||
| 	mutex_unlock(®ulator_list_mutex); | 	mutex_unlock(®ulator_list_mutex); | ||||||
| wash: | wash: | ||||||
| 	kfree(rdev->coupling_desc.coupled_rdevs); | 	kfree(rdev->coupling_desc.coupled_rdevs); | ||||||
| 	kfree(rdev->constraints); |  | ||||||
| 	mutex_lock(®ulator_list_mutex); | 	mutex_lock(®ulator_list_mutex); | ||||||
| 	regulator_ena_gpio_free(rdev); | 	regulator_ena_gpio_free(rdev); | ||||||
| 	mutex_unlock(®ulator_list_mutex); | 	mutex_unlock(®ulator_list_mutex); | ||||||
|  | 	put_device(&rdev->dev); | ||||||
|  | 	rdev = NULL; | ||||||
| clean: | clean: | ||||||
| 	if (dangling_of_gpiod) | 	if (dangling_of_gpiod) | ||||||
| 		gpiod_put(config->ena_gpiod); | 		gpiod_put(config->ena_gpiod); | ||||||
| 	if (reg_device_fail) | 	kfree(rdev); | ||||||
| 		put_device(&rdev->dev); |  | ||||||
| 	else |  | ||||||
| 		kfree(rdev); |  | ||||||
| 	kfree(config); | 	kfree(config); | ||||||
| rinse: | rinse: | ||||||
| 	if (dangling_cfg_gpiod) | 	if (dangling_cfg_gpiod) | ||||||
|  | |||||||
		Loading…
	
		Reference in New Issue
	
	Block a user