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: <57B415A5.60104@redhat.com>
Date:	Wed, 17 Aug 2016 15:43:33 +0800
From:	Xunlei Pang <xpang@...hat.com>
To:	Dave Young <dyoung@...hat.com>, Xunlei Pang <xlpang@...hat.com>
Cc:	Baoquan He <bhe@...hat.com>, kexec@...ts.infradead.org,
	linux-kernel@...r.kernel.org, ebiederm@...ssion.com,
	akpm@...ux-foundation.org, Vivek Goyal <vgoyal@...hat.com>
Subject: Re: [PATCH v2 2/2] kexec: Consider crashk_low_res in
 sanity_check_segment_list()

On 2016/08/17 at 15:24, Dave Young wrote:
> Hi, Xunlei,
>
> On 08/17/16 at 09:50am, Xunlei Pang wrote:
>> We have crashk_res only in most cases, but sometimes we have
>> crashk_low_res.
>>
>> For example, on 64-bit x86 systems, when "crashkernel=32M,high"
>> combined with "crashkernel=128M,low" is used, so some segments
>> may have the chance to be loaded into crashk_low_res area. We
>> can't fail it as a memory violation in these cases.
>>
>> Thus, we add the case to regard the segment as valid if it is
>> within crashk_low_res.
> crashkernel low is meant for swiotlb, it can be reserved automaticlly
> in case there's only crashkernel high specified in cmdline, I'm not
> sure it is useful to use crashk_res_low for other purpose and
> likely kdump can fail in the case. 
>
> I'm not sure it is really necessary to add this check now, we may
> handle it only when there is an actual use case and bug report in
> the future.

Thanks for the review.
The reason I added this is that crashk_res is allowed to be shrunk, so the segment
will surely fall into crashk_low_res if crashk_res was shrunk to be a small range.

But yes, this should be a corner case, but seems it does no harm adding this check.
Anyway, if you think it's not necessary, let's simply ignore it :-)

Regards,
Xunlei

>
> Thanks
> Dave
>> Signed-off-by: Xunlei Pang <xlpang@...hat.com>
>> ---
>>  kernel/kexec_core.c | 11 ++++++++---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
>> index 707d18e..9012a60 100644
>> --- a/kernel/kexec_core.c
>> +++ b/kernel/kexec_core.c
>> @@ -248,9 +248,14 @@ int sanity_check_segment_list(struct kimage *image)
>>  			mstart = image->segment[i].mem;
>>  			mend = mstart + image->segment[i].memsz - 1;
>>  			/* Ensure we are within the crash kernel limits */
>> -			if ((mstart < phys_to_boot_phys(crashk_res.start)) ||
>> -			    (mend > phys_to_boot_phys(crashk_res.end)))
>> -				return -EADDRNOTAVAIL;
>> +			if ((mstart >= phys_to_boot_phys(crashk_res.start)) &&
>> +			    (mend <= phys_to_boot_phys(crashk_res.end)))
>> +				continue;
>> +			if ((mstart >= phys_to_boot_phys(crashk_low_res.start)) &&
>> +			    (mend <= phys_to_boot_phys(crashk_low_res.end)))
>> +				continue;
>> +
>> +			return -EADDRNOTAVAIL;
>>  		}
>>  	}
>>  
>> -- 
>> 1.8.3.1
>>
> _______________________________________________
> kexec mailing list
> kexec@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ