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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5126DC06.5020005@cn.fujitsu.com>
Date:	Fri, 22 Feb 2013 10:46:30 +0800
From:	Zhang Yanfei <zhangyanfei@...fujitsu.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
CC:	Sasha Levin <sasha.levin@...cle.com>, kexec@...ts.infradead.org,
	linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] kexec: prevent double free on image allocation failure

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/