* [PATCH] xfs: fix memory leak in xlog_recover_add_to_trans
@ 2013-09-27 14:00 tinguely
2013-09-27 16:55 ` Eric Sandeen
2013-09-30 22:53 ` Ben Myers
0 siblings, 2 replies; 6+ messages in thread
From: tinguely @ 2013-09-27 14:00 UTC (permalink / raw)
To: xfs
[-- Attachment #1: xfs-fix-leak-in-xlog_recover_add_to_trans.patch --]
[-- Type: text/plain, Size: 814 bytes --]
Free the memory in error path of xlog_recover_add_to_trans().
Normally this memory is freed in recovery pass2, but is leaked
in the error path.
Signed-off-by: Mark Tinguely <tinguely@sgi.com>
---
Found by Coverity (134683) in userspace, same patch applies there
also.
fs/xfs/xfs_log_recover.c | 1 +
1 file changed, 1 insertion(+)
Index: b/fs/xfs/xfs_log_recover.c
===================================================================
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -1585,6 +1585,7 @@ xlog_recover_add_to_trans(
"bad number of regions (%d) in inode log format",
in_f->ilf_size);
ASSERT(0);
+ free(ptr);
return XFS_ERROR(EIO);
}
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: fix memory leak in xlog_recover_add_to_trans
2013-09-27 14:00 [PATCH] xfs: fix memory leak in xlog_recover_add_to_trans tinguely
@ 2013-09-27 16:55 ` Eric Sandeen
2013-09-27 17:34 ` Mark Tinguely
2013-09-30 22:53 ` Ben Myers
1 sibling, 1 reply; 6+ messages in thread
From: Eric Sandeen @ 2013-09-27 16:55 UTC (permalink / raw)
To: tinguely; +Cc: xfs
On 9/27/13 9:00 AM, tinguely@sgi.com wrote:
> Free the memory in error path of xlog_recover_add_to_trans().
> Normally this memory is freed in recovery pass2, but is leaked
> in the error path.
>
> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
For this local leak & the fix,
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Since this gets called in a loop from xlog_recover_process_data(),
I'm wondering what happens to previously-allocated items, if we
return an error and never get to pass2?
(I could be off base, haven't really followed it through, but
it seems like they might leak).
Thanks,
-Eric
> ---
> Found by Coverity (134683) in userspace, same patch applies there
> also.
> fs/xfs/xfs_log_recover.c | 1 +
> 1 file changed, 1 insertion(+)
>
> Index: b/fs/xfs/xfs_log_recover.c
> ===================================================================
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -1585,6 +1585,7 @@ xlog_recover_add_to_trans(
> "bad number of regions (%d) in inode log format",
> in_f->ilf_size);
> ASSERT(0);
> + free(ptr);
> return XFS_ERROR(EIO);
> }
>
>
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: fix memory leak in xlog_recover_add_to_trans
2013-09-27 16:55 ` Eric Sandeen
@ 2013-09-27 17:34 ` Mark Tinguely
2013-09-27 17:35 ` Eric Sandeen
0 siblings, 1 reply; 6+ messages in thread
From: Mark Tinguely @ 2013-09-27 17:34 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs
On 09/27/13 11:55, Eric Sandeen wrote:
> On 9/27/13 9:00 AM, tinguely@sgi.com wrote:
>> Free the memory in error path of xlog_recover_add_to_trans().
>> Normally this memory is freed in recovery pass2, but is leaked
>> in the error path.
>>
>> Signed-off-by: Mark Tinguely<tinguely@sgi.com>
>
> For this local leak & the fix,
>
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
>
> Since this gets called in a loop from xlog_recover_process_data(),
> I'm wondering what happens to previously-allocated items, if we
> return an error and never get to pass2?
>
> (I could be off base, haven't really followed it through, but
> it seems like they might leak).
>
> Thanks,
> -Eric
yeah you are correct, it looks like it gets leaked.
I will add it to the list.
Thanks.
--Mark.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: fix memory leak in xlog_recover_add_to_trans
2013-09-27 17:34 ` Mark Tinguely
@ 2013-09-27 17:35 ` Eric Sandeen
0 siblings, 0 replies; 6+ messages in thread
From: Eric Sandeen @ 2013-09-27 17:35 UTC (permalink / raw)
To: Mark Tinguely; +Cc: xfs
On 9/27/13 12:34 PM, Mark Tinguely wrote:
> On 09/27/13 11:55, Eric Sandeen wrote:
>> On 9/27/13 9:00 AM, tinguely@sgi.com wrote:
>>> Free the memory in error path of xlog_recover_add_to_trans().
>>> Normally this memory is freed in recovery pass2, but is leaked
>>> in the error path.
>>>
>>> Signed-off-by: Mark Tinguely<tinguely@sgi.com>
>>
>> For this local leak & the fix,
>>
>> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
>>
>> Since this gets called in a loop from xlog_recover_process_data(),
>> I'm wondering what happens to previously-allocated items, if we
>> return an error and never get to pass2?
>>
>> (I could be off base, haven't really followed it through, but
>> it seems like they might leak).
>>
>> Thanks,
>> -Eric
>
> yeah you are correct, it looks like it gets leaked.
> I will add it to the list.
Thanks. I suppose maybe if it's cleaned up at a higher level,
it doesn't need to be done here, but (*shrug*) I'll let you work
that out. :)
-Eric
> Thanks.
>
> --Mark.
>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: fix memory leak in xlog_recover_add_to_trans
2013-09-27 14:00 [PATCH] xfs: fix memory leak in xlog_recover_add_to_trans tinguely
2013-09-27 16:55 ` Eric Sandeen
@ 2013-09-30 22:53 ` Ben Myers
2013-10-01 13:20 ` Mark Tinguely
1 sibling, 1 reply; 6+ messages in thread
From: Ben Myers @ 2013-09-30 22:53 UTC (permalink / raw)
To: tinguely; +Cc: xfs
On Fri, Sep 27, 2013 at 09:00:55AM -0500, tinguely@sgi.com wrote:
> Free the memory in error path of xlog_recover_add_to_trans().
> Normally this memory is freed in recovery pass2, but is leaked
> in the error path.
>
> Signed-off-by: Mark Tinguely <tinguely@sgi.com>
Applied.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xfs: fix memory leak in xlog_recover_add_to_trans
2013-09-30 22:53 ` Ben Myers
@ 2013-10-01 13:20 ` Mark Tinguely
0 siblings, 0 replies; 6+ messages in thread
From: Mark Tinguely @ 2013-10-01 13:20 UTC (permalink / raw)
To: Ben Myers; +Cc: xfs
On 09/30/13 17:53, Ben Myers wrote:
> On Fri, Sep 27, 2013 at 09:00:55AM -0500, tinguely@sgi.com wrote:
>> Free the memory in error path of xlog_recover_add_to_trans().
>> Normally this memory is freed in recovery pass2, but is leaked
>> in the error path.
>>
>> Signed-off-by: Mark Tinguely<tinguely@sgi.com>
>
> Applied.
Index: b/fs/xfs/xfs_log_recover.c
===================================================================
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -1585,6 +1585,7 @@ xlog_recover_add_to_trans(
"bad number of regions (%d) in inode log format",
in_f->ilf_size);
ASSERT(0);
+ free(ptr);
Sigh. I am a big fat, dunce-hatted, dork. This is the xfsprogs patch.
The kernel needs kmem_free().
Please pull the patch. I will redo the whole thing to also make sure the
list is removed on error.
--Mark.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-10-01 13:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-27 14:00 [PATCH] xfs: fix memory leak in xlog_recover_add_to_trans tinguely
2013-09-27 16:55 ` Eric Sandeen
2013-09-27 17:34 ` Mark Tinguely
2013-09-27 17:35 ` Eric Sandeen
2013-09-30 22:53 ` Ben Myers
2013-10-01 13:20 ` Mark Tinguely
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox