Re: [PATCH] kexec: prevent double free on image allocation failure

[Zhang Yanfei <zhangyanfei@cn.fujitsu.com>]

于 2013年02月22日 09:55, Eric W. Biederman 写道:
> Sasha Levin <sasha.levin@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@cn.fujitsu.com>
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@oracle.com>
Signed-off-by: Zhang Yanfei <zhangyanfei@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


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec