Re: [PATCH] fix memory leak in ramoops_init
In the future, please use scripts/get_maintainer.pl to find your To/Cc list. :) On Mon, Sep 17, 2018 at 2:15 AM, nixiaoming wrote: > 1, memory leak in ramoops_register_dummy. >dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); >but no free when platform_device_register_data return fail Yup, that's a problem. > 2, if kzalloc(sizeof(*dummy_data), GFP_KERNEL) return NULL, > but platform_driver_register(_driver) return 0 >kfree(NULL) in ramoops_exit > so, add return val for ramoops_register_dummy, and check it in ramoops_init The kfree(NULL) isn't a problem, but a NULL platform_data implies device tree data is expected, so it'll just confuse things. However, since the dummy platform data is optional, there's no need to plumb the return value back up. Either it gets set up correctly, or there is a pr_info() about why it has been ignored. > 3, memory leak in ramoops_init. >miss platform_device_unregister(dummy) and kfree(dummy_data) >when platform_driver_register(_driver) return fail Agreed. > Signed-off-by: nixiaoming > --- > fs/pstore/ram.c | 22 +- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c > index bd9812e..331b600 100644 > --- a/fs/pstore/ram.c > +++ b/fs/pstore/ram.c > @@ -604,17 +604,17 @@ static struct platform_driver ramoops_driver = { > }, > }; > > -static void ramoops_register_dummy(void) > +static int ramoops_register_dummy(void) Another bug is that this should actually be __init (it's only called by an __init). > { > if (!mem_size) > - return; > + return -EINVAL; > > pr_info("using module parameters\n"); > > dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); > if (!dummy_data) { > pr_info("could not allocate pdata\n"); > - return; > + return -ENOMEM; > } > > dummy_data->mem_size = mem_size; > @@ -636,13 +636,25 @@ static void ramoops_register_dummy(void) > if (IS_ERR(dummy)) { > pr_info("could not create platform device: %ld\n", > PTR_ERR(dummy)); > + kfree(dummy_data); > + return PTR_ERR(dummy); > } > + return 0; > } > > static int __init ramoops_init(void) > { > - ramoops_register_dummy(); > - return platform_driver_register(_driver); > + int ret = ramoops_register_dummy(); > + > + if (ret != 0) > + return ret; > + > + ret = platform_driver_register(_driver); > + if (ret != 0) { > + platform_device_unregister(dummy); > + kfree(dummy_data); > + } > + return ret; > } > postcore_initcall(ramoops_init); > > -- > 1.9.0 > I'll send an updated patch with a similar fix... -Kees -- Kees Cook Pixel Security
Re: [PATCH] fix memory leak in ramoops_init
In the future, please use scripts/get_maintainer.pl to find your To/Cc list. :) On Mon, Sep 17, 2018 at 2:15 AM, nixiaoming wrote: > 1, memory leak in ramoops_register_dummy. >dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); >but no free when platform_device_register_data return fail Yup, that's a problem. > 2, if kzalloc(sizeof(*dummy_data), GFP_KERNEL) return NULL, > but platform_driver_register(_driver) return 0 >kfree(NULL) in ramoops_exit > so, add return val for ramoops_register_dummy, and check it in ramoops_init The kfree(NULL) isn't a problem, but a NULL platform_data implies device tree data is expected, so it'll just confuse things. However, since the dummy platform data is optional, there's no need to plumb the return value back up. Either it gets set up correctly, or there is a pr_info() about why it has been ignored. > 3, memory leak in ramoops_init. >miss platform_device_unregister(dummy) and kfree(dummy_data) >when platform_driver_register(_driver) return fail Agreed. > Signed-off-by: nixiaoming > --- > fs/pstore/ram.c | 22 +- > 1 file changed, 17 insertions(+), 5 deletions(-) > > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c > index bd9812e..331b600 100644 > --- a/fs/pstore/ram.c > +++ b/fs/pstore/ram.c > @@ -604,17 +604,17 @@ static struct platform_driver ramoops_driver = { > }, > }; > > -static void ramoops_register_dummy(void) > +static int ramoops_register_dummy(void) Another bug is that this should actually be __init (it's only called by an __init). > { > if (!mem_size) > - return; > + return -EINVAL; > > pr_info("using module parameters\n"); > > dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); > if (!dummy_data) { > pr_info("could not allocate pdata\n"); > - return; > + return -ENOMEM; > } > > dummy_data->mem_size = mem_size; > @@ -636,13 +636,25 @@ static void ramoops_register_dummy(void) > if (IS_ERR(dummy)) { > pr_info("could not create platform device: %ld\n", > PTR_ERR(dummy)); > + kfree(dummy_data); > + return PTR_ERR(dummy); > } > + return 0; > } > > static int __init ramoops_init(void) > { > - ramoops_register_dummy(); > - return platform_driver_register(_driver); > + int ret = ramoops_register_dummy(); > + > + if (ret != 0) > + return ret; > + > + ret = platform_driver_register(_driver); > + if (ret != 0) { > + platform_device_unregister(dummy); > + kfree(dummy_data); > + } > + return ret; > } > postcore_initcall(ramoops_init); > > -- > 1.9.0 > I'll send an updated patch with a similar fix... -Kees -- Kees Cook Pixel Security
Re: [PATCH] fix memory leak in ramoops_init
On Fri, Sep 28, 2018 at 2:26 PM, Andrew Morton wrote: > On Mon, 17 Sep 2018 17:15:31 +0800 nixiaoming wrote: > >> 1, memory leak in ramoops_register_dummy. >>dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); >>but no free when platform_device_register_data return fail >> >> 2, if kzalloc(sizeof(*dummy_data), GFP_KERNEL) return NULL, >> but platform_driver_register(_driver) return 0 >>kfree(NULL) in ramoops_exit >> so, add return val for ramoops_register_dummy, and check it in ramoops_init >> >> 3, memory leak in ramoops_init. >>miss platform_device_unregister(dummy) and kfree(dummy_data) >>when platform_driver_register(_driver) return fail > > Looks right. > > It's unclear (to me) who maintains fs/pstore/ram.c. Let's add some > Cc's and see if we can catch a reviewed-by. It's me: PSTORE FILESYSTEM M: Kees Cook M: Anton Vorontsov M: Colin Cross M: Tony Luck S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/pstore F: fs/pstore/ F: include/linux/pstore* F: drivers/firmware/efi/efi-pstore.c F: drivers/acpi/apei/erst.c F: Documentation/admin-guide/ramoops.rst F: Documentation/devicetree/bindings/reserved-memory/ramoops.txt K: \b(pstore|ramoops) I'll take review it and take it via the pstore tree. Thanks! -Kees -- Kees Cook Pixel Security
Re: [PATCH] fix memory leak in ramoops_init
On Fri, Sep 28, 2018 at 2:26 PM, Andrew Morton wrote: > On Mon, 17 Sep 2018 17:15:31 +0800 nixiaoming wrote: > >> 1, memory leak in ramoops_register_dummy. >>dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); >>but no free when platform_device_register_data return fail >> >> 2, if kzalloc(sizeof(*dummy_data), GFP_KERNEL) return NULL, >> but platform_driver_register(_driver) return 0 >>kfree(NULL) in ramoops_exit >> so, add return val for ramoops_register_dummy, and check it in ramoops_init >> >> 3, memory leak in ramoops_init. >>miss platform_device_unregister(dummy) and kfree(dummy_data) >>when platform_driver_register(_driver) return fail > > Looks right. > > It's unclear (to me) who maintains fs/pstore/ram.c. Let's add some > Cc's and see if we can catch a reviewed-by. It's me: PSTORE FILESYSTEM M: Kees Cook M: Anton Vorontsov M: Colin Cross M: Tony Luck S: Maintained T: git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/pstore F: fs/pstore/ F: include/linux/pstore* F: drivers/firmware/efi/efi-pstore.c F: drivers/acpi/apei/erst.c F: Documentation/admin-guide/ramoops.rst F: Documentation/devicetree/bindings/reserved-memory/ramoops.txt K: \b(pstore|ramoops) I'll take review it and take it via the pstore tree. Thanks! -Kees -- Kees Cook Pixel Security
Re: [PATCH] fix memory leak in ramoops_init
On Mon, 17 Sep 2018 17:15:31 +0800 nixiaoming wrote: > 1, memory leak in ramoops_register_dummy. >dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); >but no free when platform_device_register_data return fail > > 2, if kzalloc(sizeof(*dummy_data), GFP_KERNEL) return NULL, > but platform_driver_register(_driver) return 0 >kfree(NULL) in ramoops_exit > so, add return val for ramoops_register_dummy, and check it in ramoops_init > > 3, memory leak in ramoops_init. >miss platform_device_unregister(dummy) and kfree(dummy_data) >when platform_driver_register(_driver) return fail Looks right. It's unclear (to me) who maintains fs/pstore/ram.c. Let's add some Cc's and see if we can catch a reviewed-by. From: nixiaoming Subject: fs/pstore/ram.c: fix memory leak in ramoops_init() 1, memory leak in ramoops_register_dummy. dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); but no free when platform_device_register_data return fail 2, if kzalloc(sizeof(*dummy_data), GFP_KERNEL) return NULL, but platform_driver_register(_driver) return 0 kfree(NULL) in ramoops_exit so, add return val for ramoops_register_dummy, and check it in ramoops_init 3, memory leak in ramoops_init. miss platform_device_unregister(dummy) and kfree(dummy_data) when platform_driver_register(_driver) return fail Link: http://lkml.kernel.org/r/20180917091531.21356-1-nixiaom...@huawei.com Signed-off-by: nixiaoming Cc: Jan Kara Cc: Amir Goldstein Cc: Kees Cook Cc: Joel Fernandes Cc: Geliang Tang Signed-off-by: Andrew Morton --- fs/pstore/ram.c | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) --- a/fs/pstore/ram.c~fix-memory-leak-in-ramoops_init +++ a/fs/pstore/ram.c @@ -898,17 +898,17 @@ static struct platform_driver ramoops_dr }, }; -static void ramoops_register_dummy(void) +static int ramoops_register_dummy(void) { if (!mem_size) - return; + return -EINVAL; pr_info("using module parameters\n"); dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); if (!dummy_data) { pr_info("could not allocate pdata\n"); - return; + return -ENOMEM; } dummy_data->mem_size = mem_size; @@ -932,13 +932,25 @@ static void ramoops_register_dummy(void) if (IS_ERR(dummy)) { pr_info("could not create platform device: %ld\n", PTR_ERR(dummy)); + kfree(dummy_data); + return PTR_ERR(dummy); } + return 0; } static int __init ramoops_init(void) { - ramoops_register_dummy(); - return platform_driver_register(_driver); + int ret = ramoops_register_dummy(); + + if (ret != 0) + return ret; + + ret = platform_driver_register(_driver); + if (ret != 0) { + platform_device_unregister(dummy); + kfree(dummy_data); + } + return ret; } late_initcall(ramoops_init); _
Re: [PATCH] fix memory leak in ramoops_init
On Mon, 17 Sep 2018 17:15:31 +0800 nixiaoming wrote: > 1, memory leak in ramoops_register_dummy. >dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); >but no free when platform_device_register_data return fail > > 2, if kzalloc(sizeof(*dummy_data), GFP_KERNEL) return NULL, > but platform_driver_register(_driver) return 0 >kfree(NULL) in ramoops_exit > so, add return val for ramoops_register_dummy, and check it in ramoops_init > > 3, memory leak in ramoops_init. >miss platform_device_unregister(dummy) and kfree(dummy_data) >when platform_driver_register(_driver) return fail Looks right. It's unclear (to me) who maintains fs/pstore/ram.c. Let's add some Cc's and see if we can catch a reviewed-by. From: nixiaoming Subject: fs/pstore/ram.c: fix memory leak in ramoops_init() 1, memory leak in ramoops_register_dummy. dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); but no free when platform_device_register_data return fail 2, if kzalloc(sizeof(*dummy_data), GFP_KERNEL) return NULL, but platform_driver_register(_driver) return 0 kfree(NULL) in ramoops_exit so, add return val for ramoops_register_dummy, and check it in ramoops_init 3, memory leak in ramoops_init. miss platform_device_unregister(dummy) and kfree(dummy_data) when platform_driver_register(_driver) return fail Link: http://lkml.kernel.org/r/20180917091531.21356-1-nixiaom...@huawei.com Signed-off-by: nixiaoming Cc: Jan Kara Cc: Amir Goldstein Cc: Kees Cook Cc: Joel Fernandes Cc: Geliang Tang Signed-off-by: Andrew Morton --- fs/pstore/ram.c | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) --- a/fs/pstore/ram.c~fix-memory-leak-in-ramoops_init +++ a/fs/pstore/ram.c @@ -898,17 +898,17 @@ static struct platform_driver ramoops_dr }, }; -static void ramoops_register_dummy(void) +static int ramoops_register_dummy(void) { if (!mem_size) - return; + return -EINVAL; pr_info("using module parameters\n"); dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); if (!dummy_data) { pr_info("could not allocate pdata\n"); - return; + return -ENOMEM; } dummy_data->mem_size = mem_size; @@ -932,13 +932,25 @@ static void ramoops_register_dummy(void) if (IS_ERR(dummy)) { pr_info("could not create platform device: %ld\n", PTR_ERR(dummy)); + kfree(dummy_data); + return PTR_ERR(dummy); } + return 0; } static int __init ramoops_init(void) { - ramoops_register_dummy(); - return platform_driver_register(_driver); + int ret = ramoops_register_dummy(); + + if (ret != 0) + return ret; + + ret = platform_driver_register(_driver); + if (ret != 0) { + platform_device_unregister(dummy); + kfree(dummy_data); + } + return ret; } late_initcall(ramoops_init); _
//RE: [PATCH] fix memory leak in ramoops_init
I am very sorry, I sent the wrong email, please ignore the patch just now. -Original Message- From: Nixiaoming Sent: Monday, September 17, 2018 5:16 PM To: j...@suse.cz; amir7...@gmail.com Cc: Nixiaoming ; linux-fsde...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: [PATCH] fix memory leak in ramoops_init 1, memory leak in ramoops_register_dummy. dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); but no free when platform_device_register_data return fail 2, if kzalloc(sizeof(*dummy_data), GFP_KERNEL) return NULL, but platform_driver_register(_driver) return 0 kfree(NULL) in ramoops_exit so, add return val for ramoops_register_dummy, and check it in ramoops_init 3, memory leak in ramoops_init. miss platform_device_unregister(dummy) and kfree(dummy_data) when platform_driver_register(_driver) return fail Signed-off-by: nixiaoming --- fs/pstore/ram.c | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index bd9812e..331b600 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -604,17 +604,17 @@ static struct platform_driver ramoops_driver = { }, }; -static void ramoops_register_dummy(void) +static int ramoops_register_dummy(void) { if (!mem_size) - return; + return -EINVAL; pr_info("using module parameters\n"); dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); if (!dummy_data) { pr_info("could not allocate pdata\n"); - return; + return -ENOMEM; } dummy_data->mem_size = mem_size; @@ -636,13 +636,25 @@ static void ramoops_register_dummy(void) if (IS_ERR(dummy)) { pr_info("could not create platform device: %ld\n", PTR_ERR(dummy)); + kfree(dummy_data); + return PTR_ERR(dummy); } + return 0; } static int __init ramoops_init(void) { - ramoops_register_dummy(); - return platform_driver_register(_driver); + int ret = ramoops_register_dummy(); + + if (ret != 0) + return ret; + + ret = platform_driver_register(_driver); + if (ret != 0) { + platform_device_unregister(dummy); + kfree(dummy_data); + } + return ret; } postcore_initcall(ramoops_init); -- 1.9.0
//RE: [PATCH] fix memory leak in ramoops_init
I am very sorry, I sent the wrong email, please ignore the patch just now. -Original Message- From: Nixiaoming Sent: Monday, September 17, 2018 5:16 PM To: j...@suse.cz; amir7...@gmail.com Cc: Nixiaoming ; linux-fsde...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: [PATCH] fix memory leak in ramoops_init 1, memory leak in ramoops_register_dummy. dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); but no free when platform_device_register_data return fail 2, if kzalloc(sizeof(*dummy_data), GFP_KERNEL) return NULL, but platform_driver_register(_driver) return 0 kfree(NULL) in ramoops_exit so, add return val for ramoops_register_dummy, and check it in ramoops_init 3, memory leak in ramoops_init. miss platform_device_unregister(dummy) and kfree(dummy_data) when platform_driver_register(_driver) return fail Signed-off-by: nixiaoming --- fs/pstore/ram.c | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index bd9812e..331b600 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -604,17 +604,17 @@ static struct platform_driver ramoops_driver = { }, }; -static void ramoops_register_dummy(void) +static int ramoops_register_dummy(void) { if (!mem_size) - return; + return -EINVAL; pr_info("using module parameters\n"); dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); if (!dummy_data) { pr_info("could not allocate pdata\n"); - return; + return -ENOMEM; } dummy_data->mem_size = mem_size; @@ -636,13 +636,25 @@ static void ramoops_register_dummy(void) if (IS_ERR(dummy)) { pr_info("could not create platform device: %ld\n", PTR_ERR(dummy)); + kfree(dummy_data); + return PTR_ERR(dummy); } + return 0; } static int __init ramoops_init(void) { - ramoops_register_dummy(); - return platform_driver_register(_driver); + int ret = ramoops_register_dummy(); + + if (ret != 0) + return ret; + + ret = platform_driver_register(_driver); + if (ret != 0) { + platform_device_unregister(dummy); + kfree(dummy_data); + } + return ret; } postcore_initcall(ramoops_init); -- 1.9.0
[PATCH] fix memory leak in ramoops_init
1, memory leak in ramoops_register_dummy. dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); but no free when platform_device_register_data return fail 2, if kzalloc(sizeof(*dummy_data), GFP_KERNEL) return NULL, but platform_driver_register(_driver) return 0 kfree(NULL) in ramoops_exit so, add return val for ramoops_register_dummy, and check it in ramoops_init 3, memory leak in ramoops_init. miss platform_device_unregister(dummy) and kfree(dummy_data) when platform_driver_register(_driver) return fail Signed-off-by: nixiaoming --- fs/pstore/ram.c | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index bd9812e..331b600 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -604,17 +604,17 @@ static struct platform_driver ramoops_driver = { }, }; -static void ramoops_register_dummy(void) +static int ramoops_register_dummy(void) { if (!mem_size) - return; + return -EINVAL; pr_info("using module parameters\n"); dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); if (!dummy_data) { pr_info("could not allocate pdata\n"); - return; + return -ENOMEM; } dummy_data->mem_size = mem_size; @@ -636,13 +636,25 @@ static void ramoops_register_dummy(void) if (IS_ERR(dummy)) { pr_info("could not create platform device: %ld\n", PTR_ERR(dummy)); + kfree(dummy_data); + return PTR_ERR(dummy); } + return 0; } static int __init ramoops_init(void) { - ramoops_register_dummy(); - return platform_driver_register(_driver); + int ret = ramoops_register_dummy(); + + if (ret != 0) + return ret; + + ret = platform_driver_register(_driver); + if (ret != 0) { + platform_device_unregister(dummy); + kfree(dummy_data); + } + return ret; } postcore_initcall(ramoops_init); -- 1.9.0
[PATCH] fix memory leak in ramoops_init
1, memory leak in ramoops_register_dummy. dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); but no free when platform_device_register_data return fail 2, if kzalloc(sizeof(*dummy_data), GFP_KERNEL) return NULL, but platform_driver_register(_driver) return 0 kfree(NULL) in ramoops_exit so, add return val for ramoops_register_dummy, and check it in ramoops_init 3, memory leak in ramoops_init. miss platform_device_unregister(dummy) and kfree(dummy_data) when platform_driver_register(_driver) return fail Signed-off-by: nixiaoming --- fs/pstore/ram.c | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index bd9812e..331b600 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -604,17 +604,17 @@ static struct platform_driver ramoops_driver = { }, }; -static void ramoops_register_dummy(void) +static int ramoops_register_dummy(void) { if (!mem_size) - return; + return -EINVAL; pr_info("using module parameters\n"); dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); if (!dummy_data) { pr_info("could not allocate pdata\n"); - return; + return -ENOMEM; } dummy_data->mem_size = mem_size; @@ -636,13 +636,25 @@ static void ramoops_register_dummy(void) if (IS_ERR(dummy)) { pr_info("could not create platform device: %ld\n", PTR_ERR(dummy)); + kfree(dummy_data); + return PTR_ERR(dummy); } + return 0; } static int __init ramoops_init(void) { - ramoops_register_dummy(); - return platform_driver_register(_driver); + int ret = ramoops_register_dummy(); + + if (ret != 0) + return ret; + + ret = platform_driver_register(_driver); + if (ret != 0) { + platform_device_unregister(dummy); + kfree(dummy_data); + } + return ret; } postcore_initcall(ramoops_init); -- 1.9.0
[RESEND PATCH] fix memory leak in ramoops_init
1, memory leak in ramoops_register_dummy. dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); but no free when platform_device_register_data return fail 2, if kzalloc(sizeof(*dummy_data), GFP_KERNEL) return NULL, but platform_driver_register(_driver) return 0 kfree(NULL) in ramoops_exit so, add return val for ramoops_register_dummy, and check it in ramoops_init 3, memory leak in ramoops_init. miss platform_device_unregister(dummy) and kfree(dummy_data) when platform_driver_register(_driver) return fail Signed-off-by: nixiaoming--- fs/pstore/ram.c | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index bd9812e..331b600 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -604,17 +604,17 @@ static struct platform_driver ramoops_driver = { }, }; -static void ramoops_register_dummy(void) +static int ramoops_register_dummy(void) { if (!mem_size) - return; + return -EINVAL; pr_info("using module parameters\n"); dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); if (!dummy_data) { pr_info("could not allocate pdata\n"); - return; + return -ENOMEM; } dummy_data->mem_size = mem_size; @@ -636,13 +636,25 @@ static void ramoops_register_dummy(void) if (IS_ERR(dummy)) { pr_info("could not create platform device: %ld\n", PTR_ERR(dummy)); + kfree(dummy_data); + return PTR_ERR(dummy); } + return 0; } static int __init ramoops_init(void) { - ramoops_register_dummy(); - return platform_driver_register(_driver); + int ret = ramoops_register_dummy(); + + if (ret != 0) + return ret; + + ret = platform_driver_register(_driver); + if (ret != 0) { + platform_device_unregister(dummy); + kfree(dummy_data); + } + return ret; } postcore_initcall(ramoops_init); -- 1.9.0
[RESEND PATCH] fix memory leak in ramoops_init
1, memory leak in ramoops_register_dummy. dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); but no free when platform_device_register_data return fail 2, if kzalloc(sizeof(*dummy_data), GFP_KERNEL) return NULL, but platform_driver_register(_driver) return 0 kfree(NULL) in ramoops_exit so, add return val for ramoops_register_dummy, and check it in ramoops_init 3, memory leak in ramoops_init. miss platform_device_unregister(dummy) and kfree(dummy_data) when platform_driver_register(_driver) return fail Signed-off-by: nixiaoming --- fs/pstore/ram.c | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index bd9812e..331b600 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -604,17 +604,17 @@ static struct platform_driver ramoops_driver = { }, }; -static void ramoops_register_dummy(void) +static int ramoops_register_dummy(void) { if (!mem_size) - return; + return -EINVAL; pr_info("using module parameters\n"); dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); if (!dummy_data) { pr_info("could not allocate pdata\n"); - return; + return -ENOMEM; } dummy_data->mem_size = mem_size; @@ -636,13 +636,25 @@ static void ramoops_register_dummy(void) if (IS_ERR(dummy)) { pr_info("could not create platform device: %ld\n", PTR_ERR(dummy)); + kfree(dummy_data); + return PTR_ERR(dummy); } + return 0; } static int __init ramoops_init(void) { - ramoops_register_dummy(); - return platform_driver_register(_driver); + int ret = ramoops_register_dummy(); + + if (ret != 0) + return ret; + + ret = platform_driver_register(_driver); + if (ret != 0) { + platform_device_unregister(dummy); + kfree(dummy_data); + } + return ret; } postcore_initcall(ramoops_init); -- 1.9.0
[PATCH] fix memory leak in ramoops_init
1, memory leak in ramoops_register_dummy. dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); but no free when platform_device_register_data return fail 2, if kzalloc(sizeof(*dummy_data), GFP_KERNEL) return NULL, but platform_driver_register(_driver) return 0 kfree(NULL) in ramoops_exit so, add return val for ramoops_register_dummy, and check it in ramoops_init 3, memory leak in ramoops_init. miss platform_device_unregister(dummy) and kfree(dummy_data) when platform_driver_register(_driver) return fail Signed-off-by: nixiaoming--- fs/pstore/ram.c | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index bd9812e..331b600 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -604,17 +604,17 @@ static struct platform_driver ramoops_driver = { }, }; -static void ramoops_register_dummy(void) +static int ramoops_register_dummy(void) { if (!mem_size) - return; + return -EINVAL; pr_info("using module parameters\n"); dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); if (!dummy_data) { pr_info("could not allocate pdata\n"); - return; + return -ENOMEM; } dummy_data->mem_size = mem_size; @@ -636,13 +636,25 @@ static void ramoops_register_dummy(void) if (IS_ERR(dummy)) { pr_info("could not create platform device: %ld\n", PTR_ERR(dummy)); + kfree(dummy_data); + return PTR_ERR(dummy); } + return 0; } static int __init ramoops_init(void) { - ramoops_register_dummy(); - return platform_driver_register(_driver); + int ret = ramoops_register_dummy(); + + if (ret != 0) + return ret; + + ret = platform_driver_register(_driver); + if (ret != 0) { + platform_device_unregister(dummy); + kfree(dummy_data); + } + return ret; } postcore_initcall(ramoops_init); -- 1.9.0
[PATCH] fix memory leak in ramoops_init
1, memory leak in ramoops_register_dummy. dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); but no free when platform_device_register_data return fail 2, if kzalloc(sizeof(*dummy_data), GFP_KERNEL) return NULL, but platform_driver_register(_driver) return 0 kfree(NULL) in ramoops_exit so, add return val for ramoops_register_dummy, and check it in ramoops_init 3, memory leak in ramoops_init. miss platform_device_unregister(dummy) and kfree(dummy_data) when platform_driver_register(_driver) return fail Signed-off-by: nixiaoming --- fs/pstore/ram.c | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index bd9812e..331b600 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -604,17 +604,17 @@ static struct platform_driver ramoops_driver = { }, }; -static void ramoops_register_dummy(void) +static int ramoops_register_dummy(void) { if (!mem_size) - return; + return -EINVAL; pr_info("using module parameters\n"); dummy_data = kzalloc(sizeof(*dummy_data), GFP_KERNEL); if (!dummy_data) { pr_info("could not allocate pdata\n"); - return; + return -ENOMEM; } dummy_data->mem_size = mem_size; @@ -636,13 +636,25 @@ static void ramoops_register_dummy(void) if (IS_ERR(dummy)) { pr_info("could not create platform device: %ld\n", PTR_ERR(dummy)); + kfree(dummy_data); + return PTR_ERR(dummy); } + return 0; } static int __init ramoops_init(void) { - ramoops_register_dummy(); - return platform_driver_register(_driver); + int ret = ramoops_register_dummy(); + + if (ret != 0) + return ret; + + ret = platform_driver_register(_driver); + if (ret != 0) { + platform_device_unregister(dummy); + kfree(dummy_data); + } + return ret; } postcore_initcall(ramoops_init); -- 1.9.0