Re: [PATCH] regulator core: fix double-free in regulator_register() error path
On Tuesday 28 April 2009, Mark Brown wrote: For the record, that incomplete constraints message is bogus. On that board, VAUX3 has a complete set of constraints: it may only emit 2.8V. It's not VAUX3 that it's saying has incomplete constraints, it's the board as a whole - if the constraints for the board were fully specified No; driver support != constraint. Only one of the issues is packaged as a constraint. (and the core had been told about it) then it would power off VAUX3 at that point. Mark and/or Liam ... you might want to fix that diagnostic, to avoid leading more developers astray! Probably shove a board has in there or something I guess. How about: VAUX3 board support is incomplete. That's accurate. A comment there might suggest constraints and driver support as two common issues ... I'd not be surprised to see a few more idioms pop up. - Dave -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] regulator core: fix double-free in regulator_register() error path
On Tue, Apr 28, 2009 at 02:32:56AM -0700, David Brownell wrote: On Tuesday 28 April 2009, Mark Brown wrote: It's not VAUX3 that it's saying has incomplete constraints, it's the board as a whole - if the constraints for the board were fully specified No; driver support != constraint. Only one of the issues is packaged as a constraint. Driver support isn't particularly relevant here. If there are devices that don't have drivers or don't have drivers with regulator support but need a regulator enabling then the board should mark the regulator as always_on when setting full constraints; the always_on flag can be removed when the required consumers are added. Mark and/or Liam ... you might want to fix that diagnostic, to avoid leading more developers astray! Probably shove a board has in there or something I guess. How about: VAUX3 board support is incomplete. That's accurate. No. The constraints being complete is a property of the board as a whole and not the particular regulator. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] regulator core: fix double-free in regulator_register() error path
On Tue, Apr 28, 2009 at 04:47:52AM -0700, David Brownell wrote: On Tuesday 28 April 2009, Mark Brown wrote: On Tue, Apr 28, 2009 at 02:32:56AM -0700, David Brownell wrote: What it lacks is something entirely different: ?driver support for the LCD which uses the regulator framework, It's not VAUX3 that it's saying has incomplete constraints, it's the board as a whole - if the constraints for the board were fully specified No; driver support != constraint. Only one of the issues is packaged as a constraint. Driver support isn't particularly relevant here. It's the *entire* point. The driver is talking directly to the regulator, bypassing this framework. The constraints on that regulator are fully defined ... and then bypassed. Ah, I see - I didn't realise that there was driver support that doesn't use the regulator API, that had been dropped out of context by that point in the discussion. I guess the LCD driver will be converted to use the API in due course, hopefully there aren't too many other such drivers. This is the sort of use case that made me not want to have the API disable unused regulators by default - there's stuff going on that the regulator API and the code using it doesn't know about (since it's going through some other interface to do the job in this case) so the API can't safely do anything to that regulator. How about: VAUX3 board support is incomplete. That's accurate. No. The constraints being complete is a property of the board as a whole and not the particular regulator. Except ... that constraint isn't the issue, it's unexpected driver behavior. And board is exactly This is sufficiently unexpected that it's the first time I've seen anything bypassing the regulator API; in any case, it's just a question of where you see the problem coming from. My terminology comes from the fact that as far as the core is concerned the issue is that the board didn't say it had given the core complete constraints allowing it to fully control the regulators that are visible to it. The reason the board didn't do that is that is that the LCD driver is bypassing the regulator API and the regulator API doesn't have any way to express that possibility. what I said, so I don't know why you're arguing. (For the fun of it?) David, that is really not necessary or constructive. This sort of disparaging remark has been a constant feature of your interactions on regulator API issues and it is not achieving anything useful. Board support includes full driver support, as well as board setup (constraints). That's the common way to factor it, at any rate -- a board support package addresses both, and they need to work together. s/board/machine driver/; in an ideal world for Linux there is no BSP, it's all mainline. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] regulator core: fix double-free in regulator_register() error path
On Sat, 2009-04-25 at 05:28 -0600, Paul Walmsley wrote: During regulator registration, any error after device_register() will cause a double-free on the struct regulator_dev 'rdev'. The bug is in drivers/regulator/core.c:regulator_register(): ... scrub: device_unregister(rdev-dev); clean: kfree(rdev); --- rdev = ERR_PTR(ret); goto out; ... device_unregister() calls regulator_dev_release() which frees rdev. The subsequent kfree corrupts memory and causes some OMAP3 systems to oops on boot in regulator_get(). Applies against 2.6.30-rc3. Signed-off-by: Paul Walmsley p...@pwsan.com --- drivers/regulator/core.c |4 1 files changed, 4 insertions(+), 0 deletions(-) Applied. Thanks Liam -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] regulator core: fix double-free in regulator_register() error path
On Saturday 25 April 2009, Paul Walmsley wrote: device_unregister() calls regulator_dev_release() which frees rdev. The subsequent kfree corrupts memory and causes some OMAP3 systems to oops on boot in regulator_get(). For the 3430SDP users out there, this patch also fixes the boot hang after regulator_init_complete: incomplete constraints, leaving VAUX3 on on that device. For the record, that incomplete constraints message is bogus. On that board, VAUX3 has a complete set of constraints: it may only emit 2.8V. What it lacks is something entirely different: driver support for the LCD which uses the regulator framework, instead of just bypassing it and talking directly to the PMIC. By the time it gets there, the LCD has probably been turned on. Mark and/or Liam ... you might want to fix that diagnostic, to avoid leading more developers astray! -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] regulator core: fix double-free in regulator_register() error path
On Sat, Apr 25, 2009 at 05:28:36AM -0600, Paul Walmsley wrote: During regulator registration, any error after device_register() will cause a double-free on the struct regulator_dev 'rdev'. The bug is in drivers/regulator/core.c:regulator_register(): Looks good: Acked-by: Mark Brown broo...@opensource.wolfsonmicro.com -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] regulator core: fix double-free in regulator_register() error path
On Saturday 25 April 2009, Paul Walmsley wrote: During regulator registration, any error after device_register() will cause a double-free on the struct regulator_dev 'rdev'. The bug is in drivers/regulator/core.c:regulator_register(): ... scrub: device_unregister(rdev-dev); clean: kfree(rdev); --- rdev = ERR_PTR(ret); goto out; ... device_unregister() calls regulator_dev_release() which frees rdev. The subsequent kfree corrupts memory and causes some OMAP3 systems to oops on boot in regulator_get(). Applies against 2.6.30-rc3. Signed-off-by: Paul Walmsley p...@pwsan.com This looks like it would address the oopsing I mentioned a while back, since affects cleanup paths after errors during driver probe(). --- drivers/regulator/core.c |4 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 01f7702..fabd2e0 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -2080,6 +2080,10 @@ out: scrub: device_unregister(rdev-dev); + /* device core frees rdev */ + rdev = ERR_PTR(ret); + goto out; + clean: kfree(rdev); rdev = ERR_PTR(ret); -- 1.6.3.rc1.51.gea0b7 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] regulator core: fix double-free in regulator_register() error path
On Sat, 25 Apr 2009, Paul Walmsley wrote: During regulator registration, any error after device_register() will cause a double-free on the struct regulator_dev 'rdev'. The bug is in drivers/regulator/core.c:regulator_register(): ... device_unregister() calls regulator_dev_release() which frees rdev. The subsequent kfree corrupts memory and causes some OMAP3 systems to oops on boot in regulator_get(). For the 3430SDP users out there, this patch also fixes the boot hang after regulator_init_complete: incomplete constraints, leaving VAUX3 on on that device. - Paul -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
