regulator: Avoid potential NULL dereference in reg_fixed_voltage_probe()

PATCH Superseded Axel Lin 2 participants 4 messages Nov 27, 2011 Voltage and current regulator framework

lore

Share & tools

lore

Axel Lin

author

2011-11-27 02:14 UTC #0 lore raw

PATCH 1/1 regulator: Avoid potential NULL dereference in reg_fixed_voltage_probe()

of_get_fixed_voltage_config() may return NULL, return -ENOMEM in this case so we don't dereference NULL pointer.

Signed-off-by Axel Lin <[email protected]>

---
 drivers/regulator/fixed.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

drivers/regulator/fixed.c +4 -1

index 639e19d..2f917a1 100644
@@ -166,8 +166,11 @@ static int __devinit reg_fixed_voltage_probe(struct platform_device *pdev)
 	struct fixed_voltage_data *drvdata;
 	int ret;
 
-	if (pdev->dev.of_node)
+	if (pdev->dev.of_node) {
 		config = of_get_fixed_voltage_config(&pdev->dev);
+		if (!config)
+			return -ENOMEM;
+	}
 
 	drvdata = kzalloc(sizeof(struct fixed_voltage_data), GFP_KERNEL);
 	if (drvdata == NULL) {

Signature · 1 line

1.7.5.4

Mark Brown

2011-11-27 11:35 UTC #1 lore raw

drivers/regulator/fixed.c static int __devinit reg_fixed_voltage_probe(struct platform_device *pdev) · line 173

Quoted 6 lines · Axel Lin

drivers/regulator/fixed.c +4 -1

-	if (pdev->dev.of_node)
+	if (pdev->dev.of_node) {
 		config = of_get_fixed_voltage_config(&pdev->dev);
+		if (!config)
+			return -ENOMEM;
+	}

Looking at this I'm thinking it's better to just drop the check for of_node entirely and just let of_get_fixed_voltage_config() fail to find what it needs.

Axel Lin

author

2011-11-27 12:01 UTC #2 lore raw

2011/11/27 Mark Brown <[email protected]>:

Quoted 9 lines · Axel Lin

Quoted 7 lines
- if (pdev->dev.of_node)
+ if (pdev->dev.of_node) {
config = of_get_fixed_voltage_config(&pdev->dev);
+ if (!config)
+ return -ENOMEM;
+ }

Quoted 4 lines

Looking at this I'm thinking it's better to just drop the check for of_node entirely and just let of_get_fixed_voltage_config() fail to find what it needs.

In the case of pdev->dev.of_node == NULL, config points to pdev->dev.platform_data.

Or do you prefer change it this way:

drivers/regulator/fixed.c +6 -1

@@ -163,12 +163,17 @@ static struct regulator_ops fixed_voltage_ops = {
 
 static int __devinit reg_fixed_voltage_probe(struct platform_device *pdev)
 {
-       struct fixed_voltage_config *config = pdev->dev.platform_data;
+       struct fixed_voltage_config *config;
        struct fixed_voltage_data *drvdata;
        int ret;
 
        if (pdev->dev.of_node)
                config = of_get_fixed_voltage_config(&pdev->dev);
+       else
+               config = pdev->dev.platform_data;
+
+       if (!config)
+               return -ENOMEM;
 
        drvdata = kzalloc(sizeof(struct fixed_voltage_data), GFP_KERNEL);
        if (drvdata == NULL) {

Mark Brown

2011-11-27 12:03 UTC #3 lore raw

Quoted 1 line · Axel Lin

Or do you prefer change it this way:

Yes, that looks sensible.