Re: [PATCH 1/1] cred: fix memory leak

[David Howells]

Jiri Slaby <jsl...@suse.cz> wrote:

> Stanse found a memory leak in prepare_exec_creds. tgcred is not
> freed/assigned on all paths. Fix that.
>
> I.e. unifdef tgcred and add kfree(tgcred); as it is initialized to
> NULL already.
>
> Signed-off-by: Jiri Slaby <jsl...@suse.cz>
> Cc: David Howells <dhow...@redhat.com>
> Cc: James Morris <jmo...@namei.org>
> Cc: Serge Hallyn <se...@us.ibm.com>

Acked-by: David Howells <dhow...@redhat.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Jiri Slaby]

Stanse found a memory leak in prepare_exec_creds. tgcred is not
freed/assigned on all paths. Fix that.

I.e. unifdef tgcred and add kfree(tgcred); as it is initialized to
NULL already.

Signed-off-by: Jiri Slaby <jsl...@suse.cz>
Cc: David Howells <dhow...@redhat.com>
Cc: James Morris <jmo...@namei.org>
Cc: Serge Hallyn <se...@us.ibm.com>

---
kernel/cred.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/cred.c b/kernel/cred.c
index dd76cfe..0e10f73 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -351,9 +351,7 @@ struct cred *prepare_exec_creds(void)
*/
struct cred *prepare_usermodehelper_creds(void)
{
-#ifdef CONFIG_KEYS
struct thread_group_cred *tgcred = NULL;
-#endif
struct cred *new;

#ifdef CONFIG_KEYS
@@ -363,8 +361,10 @@ struct cred *prepare_usermodehelper_creds(void)
#endif

new = kmem_cache_alloc(cred_jar, GFP_ATOMIC);
- if (!new)
+ if (!new) {
+ kfree(tgcred);
return NULL;
+ }

kdebug("prepare_usermodehelper_creds() alloc %p", new);

--
1.6.5.7

[Serge E. Hallyn]

Quoting Jiri Slaby (jsl...@suse.cz):
> Stanse found a memory leak in prepare_exec_creds. tgcred is not
> freed/assigned on all paths. Fix that.
>
> I.e. unifdef tgcred and add kfree(tgcred); as it is initialized to
> NULL already.

Does this compile with CONFIG_KEYS=n, , though? I don't see a dummy
define for struct thread_group_cred in cred.h. Should this patch add
one?

[Jiri Slaby]

On 01/06/2010 06:25 PM, Serge E. Hallyn wrote:
> Quoting Jiri Slaby (jsl...@suse.cz):
>> Stanse found a memory leak in prepare_exec_creds. tgcred is not
>> freed/assigned on all paths. Fix that.
>>
>> I.e. unifdef tgcred and add kfree(tgcred); as it is initialized to
>> NULL already.
>
> Does this compile with CONFIG_KEYS=n, , though?

Yes and I guess it's due to no dereference of the pointer.

> Should this patch add one?

Hmm, I don't think so. The patch is ugly in the light of not having the
struct defined. I should come up with something where the CONFIG_KEYS is
left there.

thanks,
--
js