From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mx1.redhat.com ([209.132.183.28]) by bombadil.infradead.org with esmtps (Exim 4.89 #1 (Red Hat Linux)) id 1edXL3-0006cg-BX for kexec@lists.infradead.org; Mon, 22 Jan 2018 08:20:24 +0000 Date: Mon, 22 Jan 2018 16:20:06 +0800 From: Baoquan He Subject: Re: [makedumpfile PATCH 1/2] Fix off-by-one errors in exclude_segment() Message-ID: <20180122082006.GA6814@localhost.localdomain> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "kexec" Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: Petr Tesarik Cc: Atsushi Kumagai , kexec@lists.infradead.org On 01/19/18 at 12:45pm, Petr Tesarik wrote: > The crashed reserved memory end offset is the last address within > range, whereas the end offset in the pt_loads[] denotes the first > address past the range. This has caused a number of off-by-one > errors in exclude_segment(). > > First, let's unify the meaning of "end" to be the first out-of-range > address, i.e. start + size. Thanks to that, no +1 or -1 adjustments > are needed in exclude_segment(). > > Second, since the value read from /proc/iomem is the last address > within range, add one when passing it as an argument to > exclude_segment(). This is now the only adjustment by one. > > Signed-off-by: Petr Tesarik > --- > elf_info.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/elf_info.c b/elf_info.c > index 69b1719..1eaddd9 100644 > --- a/elf_info.c > +++ b/elf_info.c > @@ -820,26 +820,26 @@ static int exclude_segment(struct pt_load_segment **pt_loads, > if (kvstart < vend && kvend > vstart) { > if (kvstart != vstart && kvend != vend) { > /* Split load segment */ > - temp_seg.phys_start = end + 1; > + temp_seg.phys_start = end; > temp_seg.phys_end = (*pt_loads)[i].phys_end; > - temp_seg.virt_start = kvend + 1; > + temp_seg.virt_start = kvend; Does the old code cause error? I remember I thought about this, forget why I still took the old way. Looks a good clean up. > temp_seg.virt_end = vend; > temp_seg.file_offset = (*pt_loads)[i].file_offset > + temp_seg.virt_start - (*pt_loads)[i].virt_start; > temp_seg.file_size = temp_seg.phys_end > - temp_seg.phys_start; > > - (*pt_loads)[i].virt_end = kvstart - 1; > - (*pt_loads)[i].phys_end = start - 1; > + (*pt_loads)[i].virt_end = kvstart; > + (*pt_loads)[i].phys_end = start; > (*pt_loads)[i].file_size -= temp_seg.file_size; > > tidx = i+1; > } else if (kvstart != vstart) { > - (*pt_loads)[i].phys_end = start - 1; > - (*pt_loads)[i].virt_end = kvstart - 1; > + (*pt_loads)[i].phys_end = start; > + (*pt_loads)[i].virt_end = kvstart; > } else { > - (*pt_loads)[i].phys_start = end + 1; > - (*pt_loads)[i].virt_start = kvend + 1; > + (*pt_loads)[i].phys_start = end; > + (*pt_loads)[i].virt_start = kvend; > } > (*pt_loads)[i].file_size -= (end -start); > } > @@ -917,7 +917,7 @@ int get_kcore_dump_loads(void) > > for (i = 0; i < crash_reserved_mem_nr; i++) { > exclude_segment(&pt_loads, &num_pt_loads, > - crash_reserved_mem[i].start, crash_reserved_mem[i].end); > + crash_reserved_mem[i].start, crash_reserved_mem[i].end + 1); > } > > max_file_offset = 0; > -- > 2.13.6 > > _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec