linux-kernel - Re: [PATCH v4] mm/memblock: fix overlapping allocation when doubling reserved array

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: <4FE0F675.3050201@hp.com>
Date:	Tue, 19 Jun 2012 22:00:22 +0000
From:	"Pearson, Greg" <greg.pearson@...com>
To:	Tejun Heo <tj@...nel.org>
CC:	"hpa@...ux.intel.com" <hpa@...ux.intel.com>,
	"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
	"shangw@...ux.vnet.ibm.com" <shangw@...ux.vnet.ibm.com>,
	"mingo@...e.hu" <mingo@...e.hu>,
	"yinghai@...nel.org" <yinghai@...nel.org>,
	"benh@...nel.crashing.org" <benh@...nel.crashing.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4] mm/memblock: fix overlapping allocation when
 doubling reserved array

On 06/19/2012 03:33 PM, Tejun Heo wrote:
> On Mon, Jun 18, 2012 at 05:47:58PM -0600, Greg Pearson wrote:
>> The __alloc_memory_core_early() routine will ask memblock for a range
>> of memory then try to reserve it. If the reserved region array lacks
>> space for the new range, memblock_double_array() is called to allocate
>> more space for the array. If memblock is used to allocate memory for
>> the new array it can end up using a range that overlaps with the range
>> originally allocated in __alloc_memory_core_early(), leading to possible
>> data corruption.
>>
>> With this patch memblock_double_array() now calls memblock_find_in_range()
>> with a narrowed candidate range (in cases where the reserved.regions array
>> is being doubled) so any memory allocated will not overlap with the original
>> range that was being reserved. The range is narrowed by passing in the
>> starting address and size of the previously allocated range. Then the
>> range above the ending address is searched and if a candidate is not
>> found, the range below the starting address is searched.
>>
>> Changes from v1 to v2 (based on comments from Yinghai Lu):
>> - use obase instead of base in memblock_add_region() for excluding start address
>> - pass in both the starting and ending address of the exclude range to
>>    memblock_double_array()
>> - have memblock_double_array() search above the exclude ending address
>>    and below the exclude starting address for a free range
>>
>> Changes from v2 to v3 (based on comments from Yinghai Lu):
>> - pass in exclude_start and exclude_size to memblock_double_array()
>> - only exclude a range if doubling the reserved.regions array
>> - make sure narrowed range passed to memblock_find_in_range() is accessible
>> - to make the code less confusing, change memblock_isolate_range() to
>>    pass in exclude_start and exclude_size
>> - remove unneeded comment in memblock_add_region() between while and
>>    one line loop body
>>
>> Changes from v3 to v4 (based on comments from Tejun Heo):
>> - change parameter names passed to memblock_double_array() so they
>>    are not misleading and better signify the reason why the array is
>>    being doubled
>> - add function comment block to memblock_double_arry() to ensure
>>    the details of the possible overlap are explained
>>
>> Signed-off-by: Greg Pearson <greg.pearson@...com>
>> Cc: Tejun Heo <tj@...nel.org>
>> Cc: Benjamin Herrenschmidt <benh@...nel.crashing.org>
>> Cc: Andrew Morton <akpm@...ux-foundation.org>
>> Signed-off-by: Yinghai Lu <yinghai@...nel.org>
> Acked-by: Tejun Heo <tj@...nel.org>
>
> The SOB tag from Yinghai is a bit weird tho.  SOB indicates the chain
> of custody for the patch, so the above SOBs indicate that the patch is
> originally from Greg and then routed (rolled into series or branch) by
> Yinghai which isn't the case here.  I suppose it's either Reviewed-by:
> or Acked-by:?
>
> Thanks.
>

Tejun,

I wasn't quite sure what to do about that at first either, I read 
"Documentation/SubmittingPatches" and it says:

"The Signed-off-by: tag indicates that the signer was involved in the
development of the patch, or that he/she was in the patch's delivery path."

Since Yinghai contributed some code that is in the current version of 
the patch I thought the "Signed-off-by" tag would be ok, but if 
something else is more appropriate I have no problem re-cutting the 
patch to make the chain of custody more clear.

Thanks

--
Greg
--
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/