From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from aserp1040.oracle.com ([141.146.126.69]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1U8jWN-0007Jv-BF for kexec@lists.infradead.org; Fri, 22 Feb 2013 03:42:04 +0000 Message-ID: <5126E8F2.70502@oracle.com> Date: Thu, 21 Feb 2013 22:41:38 -0500 From: Sasha Levin MIME-Version: 1.0 Subject: Re: [PATCH] kexec: prevent double free on image allocation failure References: <1361496375-30994-1-git-send-email-sasha.levin@oracle.com> <87vc9l5cz4.fsf@xmission.com> <5126DC06.5020005@cn.fujitsu.com> In-Reply-To: <5126DC06.5020005@cn.fujitsu.com> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="gb2312" Content-Transfer-Encoding: base64 Sender: kexec-bounces@lists.infradead.org Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: Zhang Yanfei Cc: Andrew Morton , kexec@lists.infradead.org, "Eric W. Biederman" , linux-kernel@vger.kernel.org T24gMDIvMjEvMjAxMyAwOTo0NiBQTSwgWmhhbmcgWWFuZmVpIHdyb3RlOgo+INPaIDIwMTPE6jAy 1MIyMsjVIDA5OjU1LCBFcmljIFcuIEJpZWRlcm1hbiDQtLXAOgo+PiBTYXNoYSBMZXZpbiA8c2Fz aGEubGV2aW5Ab3JhY2xlLmNvbT4gd3JpdGVzOgo+Pgo+Pj4gSWYga2ltYWdlX25vcm1hbF9hbGxv YygpIGZhaWxzIHRvIGluaXRpYWxpemUgYW4gYWxsb2NhdGVkIGtpbWFnZSwgaXQgd2lsbCBmcmVl Cj4+PiB0aGUgaW1hZ2UgYnV0IHdvdWxkIHN0aWxsIHNldCAncmltYWdlJywgYXMgYSByZXN1bHQg a2V4ZWNfbG9hZCB3aWxsIHRyeQo+Pj4gdG8gZnJlZSBpdCBhZ2Fpbi4KPj4+Cj4+PiBUaGlzIHdv dWxkIGV4cGxvZGUgYXMgcGFydCBvZiB0aGUgZnJlZWluZyBwcm9jZXNzIGlzIGFjY2Vzc2luZyBp bnRlcm5hbAo+Pj4gbWVtYmVycyB3aGljaCBwb2ludCB0byB1bmluaXRpYWxpemVkIG1lbW9yeS4K Pj4KPj4gQWdyZWVkLgo+Pgo+PiBJIGRvbid0IHRoaW5rIHRoYXQgZmFpbHVyZSBwYXRoIGhhcyBl dmVyIGFjdHVhbGx5IGJlZW4gZXhlcmNpc2VkLgo+Pgo+PiBUaGUgY29kZSBpcyB3cm9uZywgYW5k IGl0IGlzIHdvcnRoIGZpeGluZy4KPj4KPj4gQW5kcmV3IEkgZG8geW91IHRoaW5rIHlvdSBjb3Vs ZCBxdWV1ZSB0aGlzIHVwPyAgSSBkb24ndCBoYXZlIGEgaGFuZHkgdHJlZS4KPiAKPiAKPiBJIHN0 aWxsIGZvdW5kIGFub3RoZXIgbWFsbG9jL2ZyZWUgcHJvYmxlbSBpbiB0aGlzIGZ1bmN0aW9uLiBT byBJIHVwZGF0ZSB0aGUgcGF0Y2guCj4gCj4gLS0tLS0tLS0tLS0tLS0tLS0tLS0tCj4gCj4gRnJv bSAxZmI3NmEzNWU0MTA5ZTE0MzVmNTUwNDhjMjBlYTU4NjIyZTdmODdiIE1vbiBTZXAgMTcgMDA6 MDA6MDAgMjAwMQo+IEZyb206IFpoYW5nIFlhbmZlaSA8emhhbmd5YW5mZWlAY24uZnVqaXRzdS5j b20+Cj4gRGF0ZTogRnJpLCAyMiBGZWIgMjAxMyAxMDozNDowMiArMDgwMAo+IFN1YmplY3Q6IFtQ QVRDSF0ga2V4ZWM6IGZpeCBhbGxvY2F0aW9uIHByb2JsZW1zIGluIGZ1bmN0aW9uIGtpbWFnZV9u b3JtYWxfYWxsb2MKPiAKPiBUaGUgZnVuY3Rpb24ga2ltYWdlX25vcm1hbF9hbGxvYygpIGhhcyAy IGFsbG9jYXRpb24gcHJvYmxlbXMgdGhhdCBtYXkgY2F1c2UKPiBmYWlsdXJlczoKPiAKPiAgIDEu IElmIGtpbWFnZV9ub3JtYWxfYWxsb2MoKSBmYWlscyB0byBpbml0aWFsaXplIGFuIGFsbG9jYXRl ZCBraW1hZ2UsIGl0IHdpbGwKPiAgICAgIGZyZWUgdGhlIGltYWdlIGJ1dCB3b3VsZCBzdGlsbCBz ZXQgJ3JpbWFnZScsIGFzIGEgcmVzdWx0IGtleGVjX2xvYWQgd2lsbAo+ICAgICAgdHJ5IHRvIGZy ZWUgaXQgYWdhaW4uCj4gCj4gICAgICBUaGlzIHdvdWxkIGV4cGxvZGUgYXMgcGFydCBvZiB0aGUg ZnJlZWluZyBwcm9jZXNzIGlzIGFjY2Vzc2luZyBpbnRlcm5hbAo+ICAgICAgbWVtYmVycyB3aGlj aCBwb2ludCB0byB1bmluaXRpYWxpemVkIG1lbW9yeS4KPiAKPiAgIDIuIElmIGtpbWFnZV9ub3Jt YWxfYWxsb2MoKSBmYWlscyB0byBhbGxvYyBwYWdlcyBmb3IgaW1hZ2UtPnN3YXBfcGFnZSwgaXQK PiAgICAgIHNob3VsZCBjYWxsIGtpbWFnZV9mcmVlX3BhZ2VfbGlzdCgpIHRvIGZyZWUgYWxsb2Nh dGVkIHBhZ2VzIGluCj4gICAgICBpbWFnZS0+Y29udHJvbF9wYWdlcyBsaXN0IGJlZm9yZSBpdCBm cmVlcyBpbWFnZS4KPiAKPiBTaWduZWQtb2ZmLWJ5OiBTYXNoYSBMZXZpbiA8c2FzaGEubGV2aW5A b3JhY2xlLmNvbT4KPiBTaWduZWQtb2ZmLWJ5OiBaaGFuZyBZYW5mZWkgPHpoYW5neWFuZmVpQGNu LmZ1aml0c3UuY29tPgo+IC0tLQo+ICBrZXJuZWwva2V4ZWMuYyB8ICAgMTAgKysrKysrLS0tLQo+ ICAxIGZpbGVzIGNoYW5nZWQsIDYgaW5zZXJ0aW9ucygrKSwgNCBkZWxldGlvbnMoLSkKPiAKPiBk aWZmIC0tZ2l0IGEva2VybmVsL2tleGVjLmMgYi9rZXJuZWwva2V4ZWMuYwo+IGluZGV4IDVlNGJk NzguLmYyMTkzNTcgMTAwNjQ0Cj4gLS0tIGEva2VybmVsL2tleGVjLmMKPiArKysgYi9rZXJuZWwv a2V4ZWMuYwo+IEBAIC0yMjMsNiArMjIzLDggQEAgb3V0Ogo+ICAKPiAgfQo+ICAKPiArc3RhdGlj IHZvaWQga2ltYWdlX2ZyZWVfcGFnZV9saXN0KHN0cnVjdCBsaXN0X2hlYWQgKmxpc3QpOwo+ICsK PiAgc3RhdGljIGludCBraW1hZ2Vfbm9ybWFsX2FsbG9jKHN0cnVjdCBraW1hZ2UgKipyaW1hZ2Us IHVuc2lnbmVkIGxvbmcgZW50cnksCj4gIAkJCQl1bnNpZ25lZCBsb25nIG5yX3NlZ21lbnRzLAo+ ICAJCQkJc3RydWN0IGtleGVjX3NlZ21lbnQgX191c2VyICpzZWdtZW50cykKPiBAQCAtMjM2LDgg KzIzOCw2IEBAIHN0YXRpYyBpbnQga2ltYWdlX25vcm1hbF9hbGxvYyhzdHJ1Y3Qga2ltYWdlICoq cmltYWdlLCB1bnNpZ25lZCBsb25nIGVudHJ5LAo+ICAJaWYgKHJlc3VsdCkKPiAgCQlnb3RvIG91 dDsKPiAgCj4gLQkqcmltYWdlID0gaW1hZ2U7Cj4gLQo+ICAJLyoKPiAgCSAqIEZpbmQgYSBsb2Nh dGlvbiBmb3IgdGhlIGNvbnRyb2wgY29kZSBidWZmZXIsIGFuZCBhZGQgaXQKPiAgCSAqIHRoZSB2 ZWN0b3Igb2Ygc2VnbWVudHMgc28gdGhhdCBpdCdzIHBhZ2VzIHdpbGwgYWxzbyBiZQo+IEBAIC0y NTksMTAgKzI1OSwxMiBAQCBzdGF0aWMgaW50IGtpbWFnZV9ub3JtYWxfYWxsb2Moc3RydWN0IGtp bWFnZSAqKnJpbWFnZSwgdW5zaWduZWQgbG9uZyBlbnRyeSwKPiAgCj4gIAlyZXN1bHQgPSAwOwo+ ICAgb3V0Ogo+IC0JaWYgKHJlc3VsdCA9PSAwKQo+ICsJaWYgKHJlc3VsdCA9PSAwKSB7Cj4gIAkJ KnJpbWFnZSA9IGltYWdlOwo+IC0JZWxzZQo+ICsJfSBlbHNlIHsKPiArCQlraW1hZ2VfZnJlZV9w YWdlX2xpc3QoJmltYWdlLT5jb250cm9sX3BhZ2VzKTsKPiAgCQlrZnJlZShpbWFnZSk7Cj4gKwl9 Cj4gIAo+ICAJcmV0dXJuIHJlc3VsdDsKPiAgfQoKQW5kIGlmIGRvX2tpbWFnZV9hbGxvYygpIGZh aWxzIGluc3RlYWQgb2Yga2ltYWdlX2FsbG9jX2NvbnRyb2xfcGFnZXMoKQp5b3Ugd2lsbCBOVUxM IGRlcmVmICdpbWFnZScsIHNvIG5vdyBpbnN0ZWFkIG9mIGxlYWtpbmcgcGFnZXMgdGhlIGtlcm5l bAp3aWxsIGV4cGxvZGUuCgpFaXRoZXIgd2F5LCB0aGlzIGlzc3VlIHlvdSd2ZSBwb2ludGVkIG91 dCBzaG91bGQgYmUgZml4ZWQgaW4gYSBzZXBhcmF0ZQpwYXRjaC4KCgpUaGFua3MsClNhc2hhCgoK X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18Ka2V4ZWMgbWFp bGluZyBsaXN0CmtleGVjQGxpc3RzLmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVh ZC5vcmcvbWFpbG1hbi9saXN0aW5mby9rZXhlYwo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755972Ab3BVDmJ (ORCPT ); Thu, 21 Feb 2013 22:42:09 -0500 Received: from aserp1040.oracle.com ([141.146.126.69]:43201 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753894Ab3BVDmF (ORCPT ); Thu, 21 Feb 2013 22:42:05 -0500 Message-ID: <5126E8F2.70502@oracle.com> Date: Thu, 21 Feb 2013 22:41:38 -0500 From: Sasha Levin User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130113 Thunderbird/17.0.2 MIME-Version: 1.0 To: Zhang Yanfei CC: "Eric W. Biederman" , kexec@lists.infradead.org, linux-kernel@vger.kernel.org, Andrew Morton Subject: Re: [PATCH] kexec: prevent double free on image allocation failure References: <1361496375-30994-1-git-send-email-sasha.levin@oracle.com> <87vc9l5cz4.fsf@xmission.com> <5126DC06.5020005@cn.fujitsu.com> In-Reply-To: <5126DC06.5020005@cn.fujitsu.com> Content-Type: text/plain; charset=GB2312 Content-Transfer-Encoding: 8bit X-Source-IP: ucsinet22.oracle.com [156.151.31.94] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 02/21/2013 09:46 PM, Zhang Yanfei wrote: > 于 2013年02月22日 09:55, Eric W. Biederman 写道: >> Sasha Levin 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 > 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 > Signed-off-by: Zhang Yanfei > --- > 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; > } And if do_kimage_alloc() fails instead of kimage_alloc_control_pages() you will NULL deref 'image', so now instead of leaking pages the kernel will explode. Either way, this issue you've pointed out should be fixed in a separate patch. Thanks, Sasha