[<prev] [next>] [day] [month] [year] [list]
Message-ID: <1363268178.4853.46.camel@i7.infradead.org>
Date: Thu, 14 Mar 2013 13:36:18 +0000
From: David Woodhouse <dwmw2@...radead.org>
To: Rusty Russell <rusty@...tcorp.com.au>
Cc: linux-kernel@...r.kernel.org
Subject: [PATCH] params: Fix potential memory leak in add_sysfs_param()
On allocation failure, it would fail to free the old attrs array which
was no longer referenced by anything (since it would free the old
module_param_attrs struct on the way out).
Comment the suspicious-looking krealloc() usage to explain why it *isn't*
actually buggy, despite looking like a classic realloc() usage bug.
Signed-off-by: David Woodhouse <David.Woodhouse@...el.com>
---
kernel/params.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/params.c b/kernel/params.c
index ed35345..53b958f 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -613,10 +613,13 @@ static __modinit int add_sysfs_param(struct module_kobject *mk,
sizeof(*mk->mp) + sizeof(mk->mp->attrs[0]) * (num+1),
GFP_KERNEL);
if (!new) {
- kfree(mk->mp);
+ kfree(attrs);
err = -ENOMEM;
goto fail;
}
+ /* Despite looking like the typical realloc() bug, this is safe.
+ * We *want* the old 'attrs' to be freed either way, and we'll store
+ * the new one in the success case. */
attrs = krealloc(attrs, sizeof(new->grp.attrs[0])*(num+2), GFP_KERNEL);
if (!attrs) {
err = -ENOMEM;
--
1.8.1.4
--
David Woodhouse Open Source Technology Centre
David.Woodhouse@...el.com Intel Corporation
Download attachment "smime.p7s" of type "application/x-pkcs7-signature" (6171 bytes)
Powered by blists - more mailing lists