Skip to contentSkip to conversation

regulator: Fix memory leak in set_machine_constraints() error paths

PATCH Superseded Axel Lin 2 participants 5 messages Jul 14, 2011 – Jul 15, 2011 Voltage and current regulator framework

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Axel Lin

author
#0 lore raw

PATCH 1/1 regulator: Fix memory leak in set_machine_constraints() error paths

Signed-off-by Axel Lin <[email protected]>
---
 drivers/regulator/core.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
drivers/regulator/core.c +4 -4
index f59821f..e7655a5 100644
@@ -787,7 +787,6 @@ static int machine_constraints_voltage(struct regulator_dev *rdev,
 		if (ret < 0) {
 			rdev_err(rdev, "failed to apply %duV constraint\n",
 				 rdev->constraints->min_uV);
-			rdev->constraints = NULL;
 			return ret;
 		}
 	}

@@ -873,7 +872,7 @@ static int machine_constraints_voltage(struct regulator_dev *rdev,
 static int set_machine_constraints(struct regulator_dev *rdev,
 	const struct regulation_constraints *constraints)
 {
-	int ret = 0;
+	int ret;
 	struct regulator_ops *ops = rdev->desc->ops;
 
 	rdev->constraints = kmemdup(constraints, sizeof(*constraints),

@@ -890,7 +889,6 @@ static int set_machine_constraints(struct regulator_dev *rdev,
 		ret = suspend_prepare(rdev, rdev->constraints->initial_state);
 		if (ret < 0) {
 			rdev_err(rdev, "failed to set suspend state\n");
-			rdev->constraints = NULL;
 			goto out;
 		}
 	}

@@ -917,13 +915,15 @@ static int set_machine_constraints(struct regulator_dev *rdev,
 		ret = ops->enable(rdev);
 		if (ret < 0) {
 			rdev_err(rdev, "failed to enable\n");
-			rdev->constraints = NULL;
 			goto out;
 		}
 	}
 
 	print_constraints(rdev);
+	return 0;
 out:
+	kfree(rdev->constraints);
+	rdev->constraints = NULL;
 	return ret;
 }
Signature · 1 line
1.7.4.1
#1 lore raw

drivers/regulator/core.c static int machine_constraints_voltage(struct regulator_dev *rdev, · line 876

Quoted 7 lines · Axel Lin

static int set_machine_constraints(struct regulator_dev *rdev,

drivers/regulator/core.c +1 -1
 	const struct regulation_constraints *constraints)
 {
-	int ret = 0;
+	int ret;
 	struct regulator_ops *ops = rdev->desc->ops;

This doesn't look like a leak fix? There's nothing that checks for other errors here (like null pointers)?

author
#2 lore raw

2011/7/15 Mark Brown <[email protected]>:

Quoted 9 lines · Axel Lin
Quoted 7 lines

 static int set_machine_constraints(struct regulator_dev *rdev, const struct regulation_constraints *constraints) {
-     int ret = 0;
+     int ret;
      struct regulator_ops *ops = rdev->desc->ops;

Quoted 3 lines

This doesn't look like a leak fix?  There's nothing that checks for other errors here (like null pointers)?

I think it does checking null pointer for rdev->constraints in current implementation:

static int set_machine_constraints(struct regulator_dev *rdev,
const struct regulation_constraints *constraints)

{
        int ret;
        struct regulator_ops *ops = rdev->desc->ops;

        rdev->constraints = kmemdup(constraints, sizeof(*constraints),
                                    GFP_KERNEL);
        if (!rdev->constraints)
                return -ENOMEM;

Regards,
Axel

#3 lore raw
Quoted 2 lines · Axel Lin

I think it does checking null pointer for rdev->constraints in current implementation:

So, then does this perhaps open paths where ret might be used uninitialized? In general this seems like an unrelated stylistic change which is unfortunately hard to review from a context diff alone.

author
#4 lore raw
Quoted 4 lines

So, then does this perhaps open paths where ret might be used uninitialized?  In general this seems like an unrelated stylistic change which is unfortunately hard to review from a context diff alone.

I got your point. will send a new version of the patch.

Thanks,
Axel