* [PATCH] UBIFS: fix memory leak on error path
@ 2012-05-17 9:03 Sidney Amani
2012-05-18 11:32 ` [PATCH 1/2] UBIFS: make ubifs_lpt_init clean-up in case of failure Artem Bityutskiy
2012-05-18 11:38 ` [PATCH] UBIFS: fix memory leak on error path Artem Bityutskiy
0 siblings, 2 replies; 6+ messages in thread
From: Sidney Amani @ 2012-05-17 9:03 UTC (permalink / raw)
To: linux-mtd; +Cc: Ben Gardiner, Sidney Amani
UBIFS leaks memory on error path in 'mount_ubifs()'. In case of failure in
'ubifs_lpt_init()' or 'ubifs_fixup_free_space()', it does not call
'ubifs_lpt_free()' whereas LPT data structures can potentially be allocated.
The amount of memory leaked can be quite high -- see 'ubifs_lpt_init()'.
The bug was introduced when moving the LPT initialisation earlier in the
mount process (commit '781c5717a95a74b294beb38b8276943b0f8b5bb4').
CC: Ben Gardiner <bengardiner@nanometrics.ca>
Signed-off-by: Sidney Amani <seed95@gmail.com>
---
fs/ubifs/super.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 76e4e05..50216ec 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1296,12 +1296,12 @@ static int mount_ubifs(struct ubifs_info *c)
err = ubifs_lpt_init(c, 1, !c->ro_mount);
if (err)
- goto out_master;
+ goto out_lpt;
if (!c->ro_mount && c->space_fixup) {
err = ubifs_fixup_free_space(c);
if (err)
- goto out_master;
+ goto out_lpt;
}
if (!c->ro_mount) {
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 1/2] UBIFS: make ubifs_lpt_init clean-up in case of failure
2012-05-17 9:03 [PATCH] UBIFS: fix memory leak on error path Sidney Amani
@ 2012-05-18 11:32 ` Artem Bityutskiy
2012-05-18 11:32 ` [PATCH 2/2] UBIFS: fix memory leak on error path Artem Bityutskiy
2012-05-20 5:49 ` [PATCH 1/2] UBIFS: make ubifs_lpt_init clean-up in case of failure Sidney Amani
2012-05-18 11:38 ` [PATCH] UBIFS: fix memory leak on error path Artem Bityutskiy
1 sibling, 2 replies; 6+ messages in thread
From: Artem Bityutskiy @ 2012-05-18 11:32 UTC (permalink / raw)
To: Sidney Amani; +Cc: Ben Gardiner, MTD Maling List
From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Most functions in UBIFS follow the following designn pattern: if the function
allocates multiple resources, and failss at some point, it frees what it has
allocated and returns an error. So the caller can rely on the fact that the
callee has cleaned up everything after own failure.
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
fs/ubifs/lpt.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/fs/ubifs/lpt.c b/fs/ubifs/lpt.c
index 2054e81..b4280c4 100644
--- a/fs/ubifs/lpt.c
+++ b/fs/ubifs/lpt.c
@@ -1740,16 +1740,20 @@ int ubifs_lpt_init(struct ubifs_info *c, int rd, int wr)
if (rd) {
err = lpt_init_rd(c);
if (err)
- return err;
+ goto out_err;
}
if (wr) {
err = lpt_init_wr(c);
if (err)
- return err;
+ goto out_err;
}
return 0;
+
+out_err:
+ ubifs_lpt_free(c, 0);
+ return err;
}
/**
--
1.7.10
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] UBIFS: fix memory leak on error path
2012-05-18 11:32 ` [PATCH 1/2] UBIFS: make ubifs_lpt_init clean-up in case of failure Artem Bityutskiy
@ 2012-05-18 11:32 ` Artem Bityutskiy
2012-05-20 5:49 ` [PATCH 1/2] UBIFS: make ubifs_lpt_init clean-up in case of failure Sidney Amani
1 sibling, 0 replies; 6+ messages in thread
From: Artem Bityutskiy @ 2012-05-18 11:32 UTC (permalink / raw)
To: Sidney Amani; +Cc: Ben Gardiner, MTD Maling List
From: Sidney Amani <seed95@gmail.com>
UBIFS leaks memory on error path in 'mount_ubifs()'. In case of failure in
'ubifs_fixup_free_space()', it does not call 'ubifs_lpt_free()' whereas LPT
data structures can potentially be allocated. The amount of memory leaked can
be quite high -- see 'ubifs_lpt_init()'.
The bug was introduced when moving the LPT initialisation earlier in the
mount process (commit '781c5717a95a74b294beb38b8276943b0f8b5bb4').
Signed-off-by: Sidney Amani <seed95@gmail.com>
Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
---
fs/ubifs/super.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 5b30c4d..675b781 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -1301,7 +1301,7 @@ static int mount_ubifs(struct ubifs_info *c)
if (!c->ro_mount && c->space_fixup) {
err = ubifs_fixup_free_space(c);
if (err)
- goto out_master;
+ goto out_lpt;
}
if (!c->ro_mount) {
--
1.7.10
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] UBIFS: make ubifs_lpt_init clean-up in case of failure
2012-05-18 11:32 ` [PATCH 1/2] UBIFS: make ubifs_lpt_init clean-up in case of failure Artem Bityutskiy
2012-05-18 11:32 ` [PATCH 2/2] UBIFS: fix memory leak on error path Artem Bityutskiy
@ 2012-05-20 5:49 ` Sidney Amani
1 sibling, 0 replies; 6+ messages in thread
From: Sidney Amani @ 2012-05-20 5:49 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: Ben Gardiner, MTD Maling List
On Fri, May 18, 2012 at 9:32 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> From: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
>
> Most functions in UBIFS follow the following designn pattern: if the function
> allocates multiple resources, and failss at some point, it frees what it has
> allocated and returns an error. So the caller can rely on the fact that the
> callee has cleaned up everything after own failure.
>
> Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>
Acked-by: Sidney Amani <seed95@gmail.com>
--
Sidney
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] UBIFS: fix memory leak on error path
2012-05-17 9:03 [PATCH] UBIFS: fix memory leak on error path Sidney Amani
2012-05-18 11:32 ` [PATCH 1/2] UBIFS: make ubifs_lpt_init clean-up in case of failure Artem Bityutskiy
@ 2012-05-18 11:38 ` Artem Bityutskiy
2012-05-20 5:49 ` Sidney Amani
1 sibling, 1 reply; 6+ messages in thread
From: Artem Bityutskiy @ 2012-05-18 11:38 UTC (permalink / raw)
To: Sidney Amani; +Cc: Ben Gardiner, linux-mtd
[-- Attachment #1: Type: text/plain, Size: 833 bytes --]
On Thu, 2012-05-17 at 19:03 +1000, Sidney Amani wrote:
> UBIFS leaks memory on error path in 'mount_ubifs()'. In case of failure in
> 'ubifs_lpt_init()' or 'ubifs_fixup_free_space()', it does not call
> 'ubifs_lpt_free()' whereas LPT data structures can potentially be allocated.
> The amount of memory leaked can be quite high -- see 'ubifs_lpt_init()'.
>
> The bug was introduced when moving the LPT initialisation earlier in the
> mount process (commit '781c5717a95a74b294beb38b8276943b0f8b5bb4').
>
> CC: Ben Gardiner <bengardiner@nanometrics.ca>
> Signed-off-by: Sidney Amani <seed95@gmail.com>
I've replied to you with the counter-proposal patches which I think
should fix this issue in a bit better way. Please, take a look - If you
are fine with that, I can push it.
--
Best Regards,
Artem Bityutskiy
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-05-20 5:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-17 9:03 [PATCH] UBIFS: fix memory leak on error path Sidney Amani
2012-05-18 11:32 ` [PATCH 1/2] UBIFS: make ubifs_lpt_init clean-up in case of failure Artem Bityutskiy
2012-05-18 11:32 ` [PATCH 2/2] UBIFS: fix memory leak on error path Artem Bityutskiy
2012-05-20 5:49 ` [PATCH 1/2] UBIFS: make ubifs_lpt_init clean-up in case of failure Sidney Amani
2012-05-18 11:38 ` [PATCH] UBIFS: fix memory leak on error path Artem Bityutskiy
2012-05-20 5:49 ` Sidney Amani
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).