? 2013?02?22? 09:55, Eric W. Biederman ??:
> Sasha Levin <sasha.levin at oracle.com> writes:
>
>> If kimage_normal_alloc() fails to initialize an allocated kimage, it will free
>> the image but would still set 'rimage', as a result kexec_load will try
>> to free it again.
>>
>> This would explode as part of the freeing process is accessing internal
>> members which point to uninitialized memory.
>
> Agreed.
>
> I don't think that failure path has ever actually been exercised.
>
> The code is wrong, and it is worth fixing.
>
> Andrew I do you think you could queue this up? I don't have a handy tree.
I still found another malloc/free problem in this function. So I update the patch.
---------------------
>From 1fb76a35e4109e1435f55048c20ea58622e7f87b Mon Sep 17 00:00:00 2001
From: Zhang Yanfei <zhangyanfei@xxxxxxxxxxxxxx>
Date: Fri, 22 Feb 2013 10:34:02 +0800
Subject: [PATCH] kexec: fix allocation problems in function kimage_normal_alloc
The function kimage_normal_alloc() has 2 allocation problems that may cause
failures:
1. If kimage_normal_alloc() fails to initialize an allocated kimage, it will
free the image but would still set 'rimage', as a result kexec_load will
try to free it again.
This would explode as part of the freeing process is accessing internal
members which point to uninitialized memory.
2. If kimage_normal_alloc() fails to alloc pages for image->swap_page, it
should call kimage_free_page_list() to free allocated pages in
image->control_pages list before it frees image.
Signed-off-by: Sasha Levin <sasha.levin at oracle.com>
Signed-off-by: Zhang Yanfei <zhangyanfei at cn.fujitsu.com>
---
kernel/kexec.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 5e4bd78..f219357 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -223,6 +223,8 @@ out:
}
+static void kimage_free_page_list(struct list_head *list);
+
static int kimage_normal_alloc(struct kimage **rimage, unsigned long entry,
unsigned long nr_segments,
struct kexec_segment __user *segments)
@@ -236,8 +238,6 @@ static int kimage_normal_alloc(struct kimage **rimage, unsigned long entry,
if (result)
goto out;
- *rimage = image;
-
/*
* Find a location for the control code buffer, and add it
* the vector of segments so that it's pages will also be
@@ -259,10 +259,12 @@ static int kimage_normal_alloc(struct kimage **rimage, unsigned long entry,
result = 0;
out:
- if (result == 0)
+ if (result == 0) {
*rimage = image;
- else
+ } else {
+ kimage_free_page_list(&image->control_pages);
kfree(image);
+ }
return result;
}
--
1.7.1
