Re: [PATCH v3 04/12] ppc64/kexec_file: avoid stomping memory used by special regions

From: Thiago Jung Bauermann
Date: Thu Jul 16 2020 - 18:00:10 EST



Hari Bathini <hbathini@xxxxxxxxxxxxx> writes:

> On 15/07/20 8:09 am, Thiago Jung Bauermann wrote:
>>
>> Hari Bathini <hbathini@xxxxxxxxxxxxx> writes:
>>
>
> <snip>
>
>>> +/**
>>> + * __locate_mem_hole_top_down - Looks top down for a large enough memory hole
>>> + * in the memory regions between buf_min & buf_max
>>> + * for the buffer. If found, sets kbuf->mem.
>>> + * @kbuf: Buffer contents and memory parameters.
>>> + * @buf_min: Minimum address for the buffer.
>>> + * @buf_max: Maximum address for the buffer.
>>> + *
>>> + * Returns 0 on success, negative errno on error.
>>> + */
>>> +static int __locate_mem_hole_top_down(struct kexec_buf *kbuf,
>>> + u64 buf_min, u64 buf_max)
>>> +{
>>> + int ret = -EADDRNOTAVAIL;
>>> + phys_addr_t start, end;
>>> + u64 i;
>>> +
>>> + for_each_mem_range_rev(i, &memblock.memory, NULL, NUMA_NO_NODE,
>>> + MEMBLOCK_NONE, &start, &end, NULL) {
>>> + if (start > buf_max)
>>> + continue;
>>> +
>>> + /* Memory hole not found */
>>> + if (end < buf_min)
>>> + break;
>>> +
>>> + /* Adjust memory region based on the given range */
>>> + if (start < buf_min)
>>> + start = buf_min;
>>> + if (end > buf_max)
>>> + end = buf_max;
>>> +
>>> + start = ALIGN(start, kbuf->buf_align);
>>> + if (start < end && (end - start + 1) >= kbuf->memsz) {
>>
>> This is why I dislike using start and end to express address ranges:
>>
>> While struct resource seems to use the [address, end] convention, my
>
> struct crash_mem also uses [address, end] convention.
> This off-by-one error did not cause any issues as the hole start and size we try to find
> are at least page aligned.
>
> Nonetheless, I think fixing 'end' early in the loop with "end -= 1" would ensure
> correctness while continuing to use the same convention for structs crash_mem & resource.

Sounds good.

--
Thiago Jung Bauermann
IBM Linux Technology Center