* [PATCH 0/3] makedumpfile: fix segfault with -X in XEN environment @ 2016-08-10 12:56 Martin Wilck 2016-08-10 12:56 ` [PATCH 1/3] open_dump_bitmap: open bitmap file in non-cyclic case Martin Wilck ` (2 more replies) 0 siblings, 3 replies; 24+ messages in thread From: Martin Wilck @ 2016-08-10 12:56 UTC (permalink / raw) To: ats-kumagai; +Cc: Martin Wilck, ptesarik, kexec exclude_xen4_user_domain()i calls clear_bit_on_2nd_bitmap(pfn, NULL) to exclude domU ranges. This resolves to set_bitmap(info->bitmap2, pfn, 0, NULL) -> set_bitmap_buffer(info->bitmap2, pfn, 0, NULL) (because bitmap2->fd == 0) ==> segfault, set_bitmap_buffer can't handle NULL as cycle pointer. If non-cyclic approach is used (always under XEN AFAICS), makedumpfile needs a bitmap fd to avoid this crash. But info->flag_cyclic can change after open_dump_bitmap() is called. This patch series fixes that by moving the call to open_dump_bitmap() after the call to initial(). Tested successfully on both Linux and XEN, x86_64. Also submitted to https://sourceforge.net/p/makedumpfile/patches/215/ Martin Wilck (3): open_dump_bitmap: open bitmap file in non-cyclic case move call to open_dump_bitmap() to after call to initial() close_dump_bitmap: simplify logic makedumpfile.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) -- 2.9.2 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/3] open_dump_bitmap: open bitmap file in non-cyclic case 2016-08-10 12:56 [PATCH 0/3] makedumpfile: fix segfault with -X in XEN environment Martin Wilck @ 2016-08-10 12:56 ` Martin Wilck 2016-08-10 12:56 ` [PATCH 2/3] move call to open_dump_bitmap() to after call to initial() Martin Wilck 2016-08-10 12:56 ` [PATCH 3/3] close_dump_bitmap: simplify logic Martin Wilck 2 siblings, 0 replies; 24+ messages in thread From: Martin Wilck @ 2016-08-10 12:56 UTC (permalink / raw) To: ats-kumagai; +Cc: Martin Wilck, ptesarik, kexec The logic of set_bitmap() requires that a bitmap fd exists in the non-cyclic case. Signed-off-by: Martin Wilck <mwilck@suse.de> --- makedumpfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/makedumpfile.c b/makedumpfile.c index 853b999..9f05396 100644 --- a/makedumpfile.c +++ b/makedumpfile.c @@ -1365,7 +1365,7 @@ open_dump_bitmap(void) /* Unnecessary to open */ if (!info->working_dir && !info->flag_reassemble && !info->flag_refiltering - && !info->flag_sadump && !info->flag_mem_usage) + && !info->flag_sadump && !info->flag_mem_usage && info->flag_cyclic) return TRUE; tmpname = getenv("TMPDIR"); -- 2.9.2 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/3] move call to open_dump_bitmap() to after call to initial() 2016-08-10 12:56 [PATCH 0/3] makedumpfile: fix segfault with -X in XEN environment Martin Wilck 2016-08-10 12:56 ` [PATCH 1/3] open_dump_bitmap: open bitmap file in non-cyclic case Martin Wilck @ 2016-08-10 12:56 ` Martin Wilck 2016-08-10 12:56 ` [PATCH 3/3] close_dump_bitmap: simplify logic Martin Wilck 2 siblings, 0 replies; 24+ messages in thread From: Martin Wilck @ 2016-08-10 12:56 UTC (permalink / raw) To: ats-kumagai; +Cc: Martin Wilck, ptesarik, kexec When open_files_for_creating_dumpfile() is called, we don't have all necessary information to determine whether a bitmap file is actually needed. In particular, we don't know whether info->flag_cyclic will ultimately be set. This patch moves the call to open_dump_bitmap() to after initialize() when all flags are known. For the dump_dmesg() path, no bitmap file is needed. Signed-off-by: Martin Wilck <mwilck@suse.de> --- makedumpfile.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/makedumpfile.c b/makedumpfile.c index 9f05396..43278f1 100644 --- a/makedumpfile.c +++ b/makedumpfile.c @@ -1480,9 +1480,6 @@ open_files_for_creating_dumpfile(void) if (!open_dump_memory()) return FALSE; - if (!open_dump_bitmap()) - return FALSE; - return TRUE; } @@ -9708,6 +9705,9 @@ create_dumpfile(void) if (!initial()) return FALSE; + if (!open_dump_bitmap()) + return FALSE; + /* create an array of translations from pfn to vmemmap pages */ if (info->flag_excludevm) { if (find_vmemmap() == FAILED) { @@ -10878,6 +10878,9 @@ int show_mem_usage(void) if (!initial()) return FALSE; + if (!open_dump_bitmap()) + return FALSE; + if (!prepare_bitmap_buffer()) return FALSE; -- 2.9.2 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/3] close_dump_bitmap: simplify logic 2016-08-10 12:56 [PATCH 0/3] makedumpfile: fix segfault with -X in XEN environment Martin Wilck 2016-08-10 12:56 ` [PATCH 1/3] open_dump_bitmap: open bitmap file in non-cyclic case Martin Wilck 2016-08-10 12:56 ` [PATCH 2/3] move call to open_dump_bitmap() to after call to initial() Martin Wilck @ 2016-08-10 12:56 ` Martin Wilck 2016-08-10 13:08 ` Petr Tesarik 2 siblings, 1 reply; 24+ messages in thread From: Martin Wilck @ 2016-08-10 12:56 UTC (permalink / raw) To: ats-kumagai; +Cc: Martin Wilck, ptesarik, kexec The boolean expression replicates the logic of open_dump_bitmap(). It's simpler and less error-prone to simply check if fd_bitmap is valid. Signed-off-by: Martin Wilck <mwilck@suse.de> --- makedumpfile.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/makedumpfile.c b/makedumpfile.c index 43278f1..771ab7c 100644 --- a/makedumpfile.c +++ b/makedumpfile.c @@ -8579,8 +8579,7 @@ close_dump_file(void) void close_dump_bitmap(void) { - if (!info->working_dir && !info->flag_reassemble && !info->flag_refiltering - && !info->flag_sadump && !info->flag_mem_usage) + if (!info->fd_bitmap) return; if ((info->fd_bitmap = close(info->fd_bitmap)) < 0) -- 2.9.2 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] close_dump_bitmap: simplify logic 2016-08-10 12:56 ` [PATCH 3/3] close_dump_bitmap: simplify logic Martin Wilck @ 2016-08-10 13:08 ` Petr Tesarik 2016-08-10 13:35 ` Martin Wilck 0 siblings, 1 reply; 24+ messages in thread From: Petr Tesarik @ 2016-08-10 13:08 UTC (permalink / raw) To: Martin Wilck; +Cc: ats-kumagai, kexec On Wed, 10 Aug 2016 14:56:58 +0200 Martin Wilck <mwilck@suse.de> wrote: > The boolean expression replicates the logic of open_dump_bitmap(). > It's simpler and less error-prone to simply check if fd_bitmap is > valid. > > Signed-off-by: Martin Wilck <mwilck@suse.de> > --- > makedumpfile.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/makedumpfile.c b/makedumpfile.c > index 43278f1..771ab7c 100644 > --- a/makedumpfile.c > +++ b/makedumpfile.c > @@ -8579,8 +8579,7 @@ close_dump_file(void) > void > close_dump_bitmap(void) > { > - if (!info->working_dir && !info->flag_reassemble && !info->flag_refiltering > - && !info->flag_sadump && !info->flag_mem_usage) > + if (!info->fd_bitmap) Strictly speaking, zero is a valid FD. I can see that it is highly unlikely to be the bitmap FD, but it would be a nice cleanup to initialize fd_bitmap to a negative number and check info->fd_bitmap < 0. I'm just not sure where to put the initializition... OTOH I know I'm asking you to fix something that you didn't break. Petr T _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] close_dump_bitmap: simplify logic 2016-08-10 13:08 ` Petr Tesarik @ 2016-08-10 13:35 ` Martin Wilck 2016-08-16 0:37 ` Atsushi Kumagai 0 siblings, 1 reply; 24+ messages in thread From: Martin Wilck @ 2016-08-10 13:35 UTC (permalink / raw) To: Petr Tesarik; +Cc: ats-kumagai, kexec On Wed, 2016-08-10 at 15:08 +0200, Petr Tesarik wrote: > On Wed, 10 Aug 2016 14:56:58 +0200 > Martin Wilck <mwilck@suse.de> wrote: > > > The boolean expression replicates the logic of open_dump_bitmap(). > > It's simpler and less error-prone to simply check if fd_bitmap is > > valid. > > > > Signed-off-by: Martin Wilck <mwilck@suse.de> > > --- > > makedumpfile.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/makedumpfile.c b/makedumpfile.c > > index 43278f1..771ab7c 100644 > > --- a/makedumpfile.c > > +++ b/makedumpfile.c > > @@ -8579,8 +8579,7 @@ close_dump_file(void) > > void > > close_dump_bitmap(void) > > { > > - if (!info->working_dir && !info->flag_reassemble && !info- > > >flag_refiltering > > - && !info->flag_sadump && !info->flag_mem_usage) > > + if (!info->fd_bitmap) > > Strictly speaking, zero is a valid FD. I can see that it is highly > unlikely to be the bitmap FD, but it would be a nice cleanup to > initialize fd_bitmap to a negative number and check info->fd_bitmap < > 0. > I'm just not sure where to put the initializition... > > OTOH I know I'm asking you to fix something that you didn't break. I had the same thought, and the same excuse not to address it in this patch set. If you grep makedumpfile.c for "fd_bitmap", you'll see many checks like "if (info->fd_bitmap)". I just followed that pattern for now. Martin _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH 3/3] close_dump_bitmap: simplify logic 2016-08-10 13:35 ` Martin Wilck @ 2016-08-16 0:37 ` Atsushi Kumagai 2016-08-16 5:59 ` Petr Tesarik 0 siblings, 1 reply; 24+ messages in thread From: Atsushi Kumagai @ 2016-08-16 0:37 UTC (permalink / raw) To: Martin Wilck, Petr Tesarik; +Cc: kexec >> > The boolean expression replicates the logic of open_dump_bitmap(). >> > It's simpler and less error-prone to simply check if fd_bitmap is >> > valid. >> > >> > Signed-off-by: Martin Wilck <mwilck@suse.de> >> > --- >> > makedumpfile.c | 3 +-- >> > 1 file changed, 1 insertion(+), 2 deletions(-) >> > >> > diff --git a/makedumpfile.c b/makedumpfile.c >> > index 43278f1..771ab7c 100644 >> > --- a/makedumpfile.c >> > +++ b/makedumpfile.c >> > @@ -8579,8 +8579,7 @@ close_dump_file(void) >> > void >> > close_dump_bitmap(void) >> > { >> > - if (!info->working_dir && !info->flag_reassemble && !info- >> > >flag_refiltering >> > - && !info->flag_sadump && !info->flag_mem_usage) >> > + if (!info->fd_bitmap) >> >> Strictly speaking, zero is a valid FD. I can see that it is highly >> unlikely to be the bitmap FD, but it would be a nice cleanup to >> initialize fd_bitmap to a negative number and check info->fd_bitmap < >> 0. >> I'm just not sure where to put the initializition... > > >> > OTOH I know I'm asking you to fix something that you didn't break. > >I had the same thought, and the same excuse not to address it in this >patch set. If you grep makedumpfile.c for "fd_bitmap", you'll see many >checks like "if (info->fd_bitmap)". I just followed that pattern for >now. I see, it would be better to make the checks strict on this occasion. So, could you work for that cleanup before your three patches as an additional cleanup patch ? Thanks, Atsushi Kumagai _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] close_dump_bitmap: simplify logic 2016-08-16 0:37 ` Atsushi Kumagai @ 2016-08-16 5:59 ` Petr Tesarik 2016-08-17 7:37 ` Martin Wilck 0 siblings, 1 reply; 24+ messages in thread From: Petr Tesarik @ 2016-08-16 5:59 UTC (permalink / raw) To: Atsushi Kumagai, Martin Wilck; +Cc: kexec On Tue, 16 Aug 2016 00:37:09 +0000 Atsushi Kumagai <ats-kumagai@wm.jp.nec.com> wrote: > >> > The boolean expression replicates the logic of open_dump_bitmap(). > >> > It's simpler and less error-prone to simply check if fd_bitmap is > >> > valid. > >> > > >> > Signed-off-by: Martin Wilck <mwilck@suse.de> > >> > --- > >> > makedumpfile.c | 3 +-- > >> > 1 file changed, 1 insertion(+), 2 deletions(-) > >> > > >> > diff --git a/makedumpfile.c b/makedumpfile.c > >> > index 43278f1..771ab7c 100644 > >> > --- a/makedumpfile.c > >> > +++ b/makedumpfile.c > >> > @@ -8579,8 +8579,7 @@ close_dump_file(void) > >> > void > >> > close_dump_bitmap(void) > >> > { > >> > - if (!info->working_dir && !info->flag_reassemble && !info- > >> > >flag_refiltering > >> > - && !info->flag_sadump && !info->flag_mem_usage) > >> > + if (!info->fd_bitmap) > >> > >> Strictly speaking, zero is a valid FD. I can see that it is highly > >> unlikely to be the bitmap FD, but it would be a nice cleanup to > >> initialize fd_bitmap to a negative number and check info->fd_bitmap < > >> 0. > >> I'm just not sure where to put the initializition... > > > > > >> > OTOH I know I'm asking you to fix something that you didn't break. > > > >I had the same thought, and the same excuse not to address it in this > >patch set. If you grep makedumpfile.c for "fd_bitmap", you'll see many > >checks like "if (info->fd_bitmap)". I just followed that pattern for > >now. > > I see, it would be better to make the checks strict on this occasion. > So, could you work for that cleanup before your three patches as an > additional cleanup patch ? OK, I take it. ;-) Martin, do you mind rebasing your patch when I'm done with the cleanup? Petr T _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/3] close_dump_bitmap: simplify logic 2016-08-16 5:59 ` Petr Tesarik @ 2016-08-17 7:37 ` Martin Wilck 2016-09-06 8:22 ` [PATCH] Cleanup: Use a negative number for uninitialized file descriptors Petr Tesarik 0 siblings, 1 reply; 24+ messages in thread From: Martin Wilck @ 2016-08-17 7:37 UTC (permalink / raw) To: Petr Tesarik, Atsushi Kumagai; +Cc: kexec On Tue, 2016-08-16 at 07:59 +0200, Petr Tesarik wrote: > On Tue, 16 Aug 2016 00:37:09 +0000 > Atsushi Kumagai <ats-kumagai@wm.jp.nec.com> wrote: > > > So, could you work for that cleanup before your three patches as an > > additional cleanup patch ? > > OK, I take it. ;-) > > Martin, do you mind rebasing your patch when I'm done with the > cleanup? No problem. Regards Martin _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] Cleanup: Use a negative number for uninitialized file descriptors 2016-08-17 7:37 ` Martin Wilck @ 2016-09-06 8:22 ` Petr Tesarik 2016-09-09 6:27 ` Atsushi Kumagai 0 siblings, 1 reply; 24+ messages in thread From: Petr Tesarik @ 2016-09-06 8:22 UTC (permalink / raw) To: Atsushi Kumagai; +Cc: Martin Wilck, kexec Do not use zero to denote an invalid file descriptor. First, zero is a valid value, although quite unlikely to be used for anything except standard input. Second, open(2) returns a negative value on failure, so there are already checks for a negative value in some places. The purpose of this patch is not to allow running in an evil environment (with closed stdin), but to aid in debugging by using a consistent value for uninitialized file descriptors which is also regarded as invalid by the kernel. For example, attempts to close a negative FD will fail (unlike an attempt to close FD 0). Signed-off-by: Petr Tesarik <ptesarik@suse.com> --- makedumpfile.c | 68 +++++++++++++++++++++++++++++++++------------------------- makedumpfile.h | 2 +- 2 files changed, 40 insertions(+), 30 deletions(-) diff --git a/makedumpfile.c b/makedumpfile.c index 21784e8..d168dfd 100644 --- a/makedumpfile.c +++ b/makedumpfile.c @@ -3730,10 +3730,10 @@ free_for_parallel() return; for (i = 0; i < info->num_threads; i++) { - if (FD_MEMORY_PARALLEL(i) > 0) + if (FD_MEMORY_PARALLEL(i) >= 0) close(FD_MEMORY_PARALLEL(i)); - if (FD_BITMAP_MEMORY_PARALLEL(i) > 0) + if (FD_BITMAP_MEMORY_PARALLEL(i) >= 0) close(FD_BITMAP_MEMORY_PARALLEL(i)); } } @@ -4038,13 +4038,13 @@ out: void initialize_bitmap(struct dump_bitmap *bitmap) { - if (info->fd_bitmap) { + if (info->fd_bitmap >= 0) { bitmap->fd = info->fd_bitmap; bitmap->file_name = info->name_bitmap; bitmap->no_block = -1; memset(bitmap->buf, 0, BUFSIZE_BITMAP); } else { - bitmap->fd = 0; + bitmap->fd = -1; bitmap->file_name = NULL; bitmap->no_block = -1; memset(bitmap->buf, 0, info->bufsize_cyclic); @@ -4154,7 +4154,7 @@ set_bitmap_buffer(struct dump_bitmap *bitmap, mdf_pfn_t pfn, int val, struct cyc int set_bitmap(struct dump_bitmap *bitmap, mdf_pfn_t pfn, int val, struct cycle *cycle) { - if (bitmap->fd) { + if (bitmap->fd >= 0) { return set_bitmap_file(bitmap, pfn, val); } else { return set_bitmap_buffer(bitmap, pfn, val, cycle); @@ -4170,7 +4170,7 @@ sync_bitmap(struct dump_bitmap *bitmap) /* * The bitmap doesn't have the fd, it's a on-memory bitmap. */ - if (bitmap->fd == 0) + if (bitmap->fd < 0) return TRUE; /* * The bitmap buffer is not dirty, and it is not necessary @@ -5403,7 +5403,7 @@ create_1st_bitmap_buffer(struct cycle *cycle) int create_1st_bitmap(struct cycle *cycle) { - if (info->bitmap1->fd) { + if (info->bitmap1->fd >= 0) { return create_1st_bitmap_file(); } else { return create_1st_bitmap_buffer(cycle); @@ -5414,7 +5414,7 @@ static inline int is_in_segs(unsigned long long paddr) { if (info->flag_refiltering || info->flag_sadump) { - if (info->bitmap1->fd == 0) { + if (info->bitmap1->fd < 0) { initialize_1st_bitmap(info->bitmap1); create_1st_bitmap_file(); } @@ -5872,7 +5872,7 @@ copy_bitmap_file(void) int copy_bitmap(void) { - if (info->fd_bitmap) { + if (info->fd_bitmap >= 0) { return copy_bitmap_file(); } else { return copy_bitmap_buffer(); @@ -6313,7 +6313,7 @@ prepare_bitmap1_buffer(void) return FALSE; } - if (info->fd_bitmap) { + if (info->fd_bitmap >= 0) { if ((info->bitmap1->buf = (char *)malloc(BUFSIZE_BITMAP)) == NULL) { ERRMSG("Can't allocate memory for the 1st bitmaps's buffer. %s\n", strerror(errno)); @@ -6352,7 +6352,7 @@ prepare_bitmap2_buffer(void) strerror(errno)); return FALSE; } - if (info->fd_bitmap) { + if (info->fd_bitmap >= 0) { if ((info->bitmap2->buf = (char *)malloc(BUFSIZE_BITMAP)) == NULL) { ERRMSG("Can't allocate memory for the 2nd bitmaps's buffer. %s\n", strerror(errno)); @@ -7582,7 +7582,7 @@ kdump_thread_function_cyclic(void *arg) { fd_memory = FD_MEMORY_PARALLEL(kdump_thread_args->thread_num); - if (info->fd_bitmap) { + if (info->fd_bitmap >= 0) { bitmap_parallel.buf = malloc(BUFSIZE_BITMAP); if (bitmap_parallel.buf == NULL){ ERRMSG("Can't allocate memory for bitmap_parallel.buf. %s\n", @@ -7628,7 +7628,7 @@ kdump_thread_function_cyclic(void *arg) { pthread_mutex_lock(&info->current_pfn_mutex); for (pfn = info->current_pfn; pfn < cycle->end_pfn; pfn++) { dumpable = is_dumpable( - info->fd_bitmap ? &bitmap_parallel : info->bitmap2, + info->fd_bitmap >= 0 ? &bitmap_parallel : info->bitmap2, pfn, cycle); if (dumpable) @@ -7723,7 +7723,7 @@ next: retval = NULL; fail: - if (bitmap_memory_parallel.fd > 0) + if (bitmap_memory_parallel.fd >= 0) close(bitmap_memory_parallel.fd); if (bitmap_parallel.buf != NULL) free(bitmap_parallel.buf); @@ -8461,7 +8461,7 @@ out: int write_kdump_bitmap1(struct cycle *cycle) { - if (info->bitmap1->fd) { + if (info->bitmap1->fd >= 0) { return write_kdump_bitmap1_file(); } else { return write_kdump_bitmap1_buffer(cycle); @@ -8470,7 +8470,7 @@ write_kdump_bitmap1(struct cycle *cycle) { int write_kdump_bitmap2(struct cycle *cycle) { - if (info->bitmap2->fd) { + if (info->bitmap2->fd >= 0) { return write_kdump_bitmap2_file(); } else { return write_kdump_bitmap2_buffer(cycle); @@ -8597,9 +8597,10 @@ close_vmcoreinfo(void) void close_dump_memory(void) { - if ((info->fd_memory = close(info->fd_memory)) < 0) + if (close(info->fd_memory) < 0) ERRMSG("Can't close the dump memory(%s). %s\n", info->name_memory, strerror(errno)); + info->fd_memory = -1; } void @@ -8608,9 +8609,10 @@ close_dump_file(void) if (info->flag_flatten) return; - if ((info->fd_dumpfile = close(info->fd_dumpfile)) < 0) + if (close(info->fd_dumpfile) < 0) ERRMSG("Can't close the dump file(%s). %s\n", info->name_dumpfile, strerror(errno)); + info->fd_dumpfile = -1; } void @@ -8620,9 +8622,10 @@ close_dump_bitmap(void) && !info->flag_sadump && !info->flag_mem_usage) return; - if ((info->fd_bitmap = close(info->fd_bitmap)) < 0) + if (close(info->fd_bitmap) < 0) ERRMSG("Can't close the bitmap file(%s). %s\n", info->name_bitmap, strerror(errno)); + info->fd_bitmap = -1; free(info->name_bitmap); info->name_bitmap = NULL; } @@ -8631,16 +8634,18 @@ void close_kernel_file(void) { if (info->name_vmlinux) { - if ((info->fd_vmlinux = close(info->fd_vmlinux)) < 0) { + if (close(info->fd_vmlinux) < 0) { ERRMSG("Can't close the kernel file(%s). %s\n", info->name_vmlinux, strerror(errno)); } + info->fd_vmlinux = -1; } if (info->name_xen_syms) { - if ((info->fd_xen_syms = close(info->fd_xen_syms)) < 0) { + if (close(info->fd_xen_syms) < 0) { ERRMSG("Can't close the kernel file(%s). %s\n", info->name_xen_syms, strerror(errno)); } + info->fd_xen_syms = -1; } } @@ -10202,7 +10207,7 @@ reassemble_kdump_header(void) ret = TRUE; out: - if (fd > 0) + if (fd >= 0) close(fd); free(buf_bitmap); @@ -10212,7 +10217,7 @@ out: int reassemble_kdump_pages(void) { - int i, fd = 0, ret = FALSE; + int i, fd = -1, ret = FALSE; off_t offset_first_ph, offset_ph_org, offset_eraseinfo; off_t offset_data_new, offset_zero_page = 0; mdf_pfn_t pfn, start_pfn, end_pfn; @@ -10336,7 +10341,7 @@ reassemble_kdump_pages(void) offset_data_new += pd.size; } close(fd); - fd = 0; + fd = -1; } if (!write_cache_bufsz(&cd_pd)) goto out; @@ -10379,7 +10384,7 @@ reassemble_kdump_pages(void) size_eraseinfo += SPLITTING_SIZE_EI(i); close(fd); - fd = 0; + fd = -1; } if (size_eraseinfo) { if (!write_cache_bufsz(&cd_data)) @@ -10400,7 +10405,7 @@ out: if (data) free(data); - if (fd > 0) + if (fd >= 0) close(fd); return ret; @@ -10973,6 +10978,11 @@ main(int argc, char *argv[]) strerror(errno)); goto out; } + info->fd_vmlinux = -1; + info->fd_xen_syms = -1; + info->fd_memory = -1; + info->fd_dumpfile = -1; + info->fd_bitmap = -1; initialize_tables(); /* @@ -11268,11 +11278,11 @@ out: free(info->bitmap_memory->buf); free(info->bitmap_memory); } - if (info->fd_memory) + if (info->fd_memory >= 0) close(info->fd_memory); - if (info->fd_dumpfile) + if (info->fd_dumpfile >= 0) close(info->fd_dumpfile); - if (info->fd_bitmap) + if (info->fd_bitmap >= 0) close(info->fd_bitmap); if (vt.node_online_map != NULL) free(vt.node_online_map); diff --git a/makedumpfile.h b/makedumpfile.h index 533e5b8..1814139 100644 --- a/makedumpfile.h +++ b/makedumpfile.h @@ -1955,7 +1955,7 @@ is_dumpable_file(struct dump_bitmap *bitmap, mdf_pfn_t pfn) static inline int is_dumpable(struct dump_bitmap *bitmap, mdf_pfn_t pfn, struct cycle *cycle) { - if (bitmap->fd == 0) { + if (bitmap->fd < 0) { return is_dumpable_buffer(bitmap, pfn, cycle); } else { return is_dumpable_file(bitmap, pfn); -- 2.6.6 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply related [flat|nested] 24+ messages in thread
* RE: [PATCH] Cleanup: Use a negative number for uninitialized file descriptors 2016-09-06 8:22 ` [PATCH] Cleanup: Use a negative number for uninitialized file descriptors Petr Tesarik @ 2016-09-09 6:27 ` Atsushi Kumagai 2016-09-09 7:31 ` Petr Tesarik 0 siblings, 1 reply; 24+ messages in thread From: Atsushi Kumagai @ 2016-09-09 6:27 UTC (permalink / raw) To: Petr Tesarik; +Cc: Martin Wilck, kexec Hello Petr, >Do not use zero to denote an invalid file descriptor. > >First, zero is a valid value, although quite unlikely to be used for >anything except standard input. > >Second, open(2) returns a negative value on failure, so there are >already checks for a negative value in some places. > >The purpose of this patch is not to allow running in an evil environment >(with closed stdin), but to aid in debugging by using a consistent value >for uninitialized file descriptors which is also regarded as invalid by >the kernel. For example, attempts to close a negative FD will fail >(unlike an attempt to close FD 0). > >Signed-off-by: Petr Tesarik <ptesarik@suse.com> Good, thanks for your work, but could you fix more two points as below ? dwarf_info.c:: 1595 if (dwarf_info.module_name 1596 && strcmp(dwarf_info.module_name, "vmlinux") 1597 && strcmp(dwarf_info.module_name, "xen-syms")) { 1598 if (dwarf_info.fd_debuginfo > 0) // should be '>= 0' 1599 close(dwarf_info.fd_debuginfo); sadump_info.c:: 1919 for (i = 1; i < si->num_disks; ++i) { 1920 if (si->diskset_info[i].fd_memory) // should be 'fd_memory >=0' 1921 close(si->diskset_info[i].fd_memory); Thanks, Atsushi Kumagai >--- > makedumpfile.c | 68 +++++++++++++++++++++++++++++++++------------------------- > makedumpfile.h | 2 +- > 2 files changed, 40 insertions(+), 30 deletions(-) > >diff --git a/makedumpfile.c b/makedumpfile.c >index 21784e8..d168dfd 100644 >--- a/makedumpfile.c >+++ b/makedumpfile.c >@@ -3730,10 +3730,10 @@ free_for_parallel() > return; > > for (i = 0; i < info->num_threads; i++) { >- if (FD_MEMORY_PARALLEL(i) > 0) >+ if (FD_MEMORY_PARALLEL(i) >= 0) > close(FD_MEMORY_PARALLEL(i)); > >- if (FD_BITMAP_MEMORY_PARALLEL(i) > 0) >+ if (FD_BITMAP_MEMORY_PARALLEL(i) >= 0) > close(FD_BITMAP_MEMORY_PARALLEL(i)); > } > } >@@ -4038,13 +4038,13 @@ out: > void > initialize_bitmap(struct dump_bitmap *bitmap) > { >- if (info->fd_bitmap) { >+ if (info->fd_bitmap >= 0) { > bitmap->fd = info->fd_bitmap; > bitmap->file_name = info->name_bitmap; > bitmap->no_block = -1; > memset(bitmap->buf, 0, BUFSIZE_BITMAP); > } else { >- bitmap->fd = 0; >+ bitmap->fd = -1; > bitmap->file_name = NULL; > bitmap->no_block = -1; > memset(bitmap->buf, 0, info->bufsize_cyclic); >@@ -4154,7 +4154,7 @@ set_bitmap_buffer(struct dump_bitmap *bitmap, mdf_pfn_t pfn, int val, struct cyc > int > set_bitmap(struct dump_bitmap *bitmap, mdf_pfn_t pfn, int val, struct cycle *cycle) > { >- if (bitmap->fd) { >+ if (bitmap->fd >= 0) { > return set_bitmap_file(bitmap, pfn, val); > } else { > return set_bitmap_buffer(bitmap, pfn, val, cycle); >@@ -4170,7 +4170,7 @@ sync_bitmap(struct dump_bitmap *bitmap) > /* > * The bitmap doesn't have the fd, it's a on-memory bitmap. > */ >- if (bitmap->fd == 0) >+ if (bitmap->fd < 0) > return TRUE; > /* > * The bitmap buffer is not dirty, and it is not necessary >@@ -5403,7 +5403,7 @@ create_1st_bitmap_buffer(struct cycle *cycle) > int > create_1st_bitmap(struct cycle *cycle) > { >- if (info->bitmap1->fd) { >+ if (info->bitmap1->fd >= 0) { > return create_1st_bitmap_file(); > } else { > return create_1st_bitmap_buffer(cycle); >@@ -5414,7 +5414,7 @@ static inline int > is_in_segs(unsigned long long paddr) > { > if (info->flag_refiltering || info->flag_sadump) { >- if (info->bitmap1->fd == 0) { >+ if (info->bitmap1->fd < 0) { > initialize_1st_bitmap(info->bitmap1); > create_1st_bitmap_file(); > } >@@ -5872,7 +5872,7 @@ copy_bitmap_file(void) > int > copy_bitmap(void) > { >- if (info->fd_bitmap) { >+ if (info->fd_bitmap >= 0) { > return copy_bitmap_file(); > } else { > return copy_bitmap_buffer(); >@@ -6313,7 +6313,7 @@ prepare_bitmap1_buffer(void) > return FALSE; > } > >- if (info->fd_bitmap) { >+ if (info->fd_bitmap >= 0) { > if ((info->bitmap1->buf = (char *)malloc(BUFSIZE_BITMAP)) == NULL) { > ERRMSG("Can't allocate memory for the 1st bitmaps's buffer. %s\n", > strerror(errno)); >@@ -6352,7 +6352,7 @@ prepare_bitmap2_buffer(void) > strerror(errno)); > return FALSE; > } >- if (info->fd_bitmap) { >+ if (info->fd_bitmap >= 0) { > if ((info->bitmap2->buf = (char *)malloc(BUFSIZE_BITMAP)) == NULL) { > ERRMSG("Can't allocate memory for the 2nd bitmaps's buffer. %s\n", > strerror(errno)); >@@ -7582,7 +7582,7 @@ kdump_thread_function_cyclic(void *arg) { > > fd_memory = FD_MEMORY_PARALLEL(kdump_thread_args->thread_num); > >- if (info->fd_bitmap) { >+ if (info->fd_bitmap >= 0) { > bitmap_parallel.buf = malloc(BUFSIZE_BITMAP); > if (bitmap_parallel.buf == NULL){ > ERRMSG("Can't allocate memory for bitmap_parallel.buf. %s\n", >@@ -7628,7 +7628,7 @@ kdump_thread_function_cyclic(void *arg) { > pthread_mutex_lock(&info->current_pfn_mutex); > for (pfn = info->current_pfn; pfn < cycle->end_pfn; pfn++) { > dumpable = is_dumpable( >- info->fd_bitmap ? &bitmap_parallel : info->bitmap2, >+ info->fd_bitmap >= 0 ? &bitmap_parallel : info->bitmap2, > pfn, > cycle); > if (dumpable) >@@ -7723,7 +7723,7 @@ next: > retval = NULL; > > fail: >- if (bitmap_memory_parallel.fd > 0) >+ if (bitmap_memory_parallel.fd >= 0) > close(bitmap_memory_parallel.fd); > if (bitmap_parallel.buf != NULL) > free(bitmap_parallel.buf); >@@ -8461,7 +8461,7 @@ out: > > int > write_kdump_bitmap1(struct cycle *cycle) { >- if (info->bitmap1->fd) { >+ if (info->bitmap1->fd >= 0) { > return write_kdump_bitmap1_file(); > } else { > return write_kdump_bitmap1_buffer(cycle); >@@ -8470,7 +8470,7 @@ write_kdump_bitmap1(struct cycle *cycle) { > > int > write_kdump_bitmap2(struct cycle *cycle) { >- if (info->bitmap2->fd) { >+ if (info->bitmap2->fd >= 0) { > return write_kdump_bitmap2_file(); > } else { > return write_kdump_bitmap2_buffer(cycle); >@@ -8597,9 +8597,10 @@ close_vmcoreinfo(void) > void > close_dump_memory(void) > { >- if ((info->fd_memory = close(info->fd_memory)) < 0) >+ if (close(info->fd_memory) < 0) > ERRMSG("Can't close the dump memory(%s). %s\n", > info->name_memory, strerror(errno)); >+ info->fd_memory = -1; > } > > void >@@ -8608,9 +8609,10 @@ close_dump_file(void) > if (info->flag_flatten) > return; > >- if ((info->fd_dumpfile = close(info->fd_dumpfile)) < 0) >+ if (close(info->fd_dumpfile) < 0) > ERRMSG("Can't close the dump file(%s). %s\n", > info->name_dumpfile, strerror(errno)); >+ info->fd_dumpfile = -1; > } > > void >@@ -8620,9 +8622,10 @@ close_dump_bitmap(void) > && !info->flag_sadump && !info->flag_mem_usage) > return; > >- if ((info->fd_bitmap = close(info->fd_bitmap)) < 0) >+ if (close(info->fd_bitmap) < 0) > ERRMSG("Can't close the bitmap file(%s). %s\n", > info->name_bitmap, strerror(errno)); >+ info->fd_bitmap = -1; > free(info->name_bitmap); > info->name_bitmap = NULL; > } >@@ -8631,16 +8634,18 @@ void > close_kernel_file(void) > { > if (info->name_vmlinux) { >- if ((info->fd_vmlinux = close(info->fd_vmlinux)) < 0) { >+ if (close(info->fd_vmlinux) < 0) { > ERRMSG("Can't close the kernel file(%s). %s\n", > info->name_vmlinux, strerror(errno)); > } >+ info->fd_vmlinux = -1; > } > if (info->name_xen_syms) { >- if ((info->fd_xen_syms = close(info->fd_xen_syms)) < 0) { >+ if (close(info->fd_xen_syms) < 0) { > ERRMSG("Can't close the kernel file(%s). %s\n", > info->name_xen_syms, strerror(errno)); > } >+ info->fd_xen_syms = -1; > } > } > >@@ -10202,7 +10207,7 @@ reassemble_kdump_header(void) > > ret = TRUE; > out: >- if (fd > 0) >+ if (fd >= 0) > close(fd); > free(buf_bitmap); > >@@ -10212,7 +10217,7 @@ out: > int > reassemble_kdump_pages(void) > { >- int i, fd = 0, ret = FALSE; >+ int i, fd = -1, ret = FALSE; > off_t offset_first_ph, offset_ph_org, offset_eraseinfo; > off_t offset_data_new, offset_zero_page = 0; > mdf_pfn_t pfn, start_pfn, end_pfn; >@@ -10336,7 +10341,7 @@ reassemble_kdump_pages(void) > offset_data_new += pd.size; > } > close(fd); >- fd = 0; >+ fd = -1; > } > if (!write_cache_bufsz(&cd_pd)) > goto out; >@@ -10379,7 +10384,7 @@ reassemble_kdump_pages(void) > size_eraseinfo += SPLITTING_SIZE_EI(i); > > close(fd); >- fd = 0; >+ fd = -1; > } > if (size_eraseinfo) { > if (!write_cache_bufsz(&cd_data)) >@@ -10400,7 +10405,7 @@ out: > > if (data) > free(data); >- if (fd > 0) >+ if (fd >= 0) > close(fd); > > return ret; >@@ -10973,6 +10978,11 @@ main(int argc, char *argv[]) > strerror(errno)); > goto out; > } >+ info->fd_vmlinux = -1; >+ info->fd_xen_syms = -1; >+ info->fd_memory = -1; >+ info->fd_dumpfile = -1; >+ info->fd_bitmap = -1; > initialize_tables(); > > /* >@@ -11268,11 +11278,11 @@ out: > free(info->bitmap_memory->buf); > free(info->bitmap_memory); > } >- if (info->fd_memory) >+ if (info->fd_memory >= 0) > close(info->fd_memory); >- if (info->fd_dumpfile) >+ if (info->fd_dumpfile >= 0) > close(info->fd_dumpfile); >- if (info->fd_bitmap) >+ if (info->fd_bitmap >= 0) > close(info->fd_bitmap); > if (vt.node_online_map != NULL) > free(vt.node_online_map); >diff --git a/makedumpfile.h b/makedumpfile.h >index 533e5b8..1814139 100644 >--- a/makedumpfile.h >+++ b/makedumpfile.h >@@ -1955,7 +1955,7 @@ is_dumpable_file(struct dump_bitmap *bitmap, mdf_pfn_t pfn) > static inline int > is_dumpable(struct dump_bitmap *bitmap, mdf_pfn_t pfn, struct cycle *cycle) > { >- if (bitmap->fd == 0) { >+ if (bitmap->fd < 0) { > return is_dumpable_buffer(bitmap, pfn, cycle); > } else { > return is_dumpable_file(bitmap, pfn); >-- >2.6.6 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] Cleanup: Use a negative number for uninitialized file descriptors 2016-09-09 6:27 ` Atsushi Kumagai @ 2016-09-09 7:31 ` Petr Tesarik 2016-09-09 8:11 ` [PATCH v2] " Petr Tesarik 0 siblings, 1 reply; 24+ messages in thread From: Petr Tesarik @ 2016-09-09 7:31 UTC (permalink / raw) To: Atsushi Kumagai; +Cc: Martin Wilck, kexec Hello Atsushi, On Fri, 9 Sep 2016 06:27:17 +0000 Atsushi Kumagai <ats-kumagai@wm.jp.nec.com> wrote: > Hello Petr, > > >Do not use zero to denote an invalid file descriptor. > > > >First, zero is a valid value, although quite unlikely to be used for > >anything except standard input. > > > >Second, open(2) returns a negative value on failure, so there are > >already checks for a negative value in some places. > > > >The purpose of this patch is not to allow running in an evil environment > >(with closed stdin), but to aid in debugging by using a consistent value > >for uninitialized file descriptors which is also regarded as invalid by > >the kernel. For example, attempts to close a negative FD will fail > >(unlike an attempt to close FD 0). > > > >Signed-off-by: Petr Tesarik <ptesarik@suse.com> > > Good, thanks for your work, but could you fix > more two points as below ? I see. I skipped elf_info.c, dwarf_info.c, sadump_info.c and other files, because the file descriptors there are initialized from other variables, already checked in the main module, but you're right, the checks should become consistent. Expect a version 2 of the patch soon. Petr T > dwarf_info.c:: > 1595 if (dwarf_info.module_name > 1596 && strcmp(dwarf_info.module_name, "vmlinux") > 1597 && strcmp(dwarf_info.module_name, "xen-syms")) { > 1598 if (dwarf_info.fd_debuginfo > 0) // should be '>= 0' > 1599 close(dwarf_info.fd_debuginfo); > > sadump_info.c:: > 1919 for (i = 1; i < si->num_disks; ++i) { > 1920 if (si->diskset_info[i].fd_memory) // should be 'fd_memory >=0' > 1921 close(si->diskset_info[i].fd_memory); > > > Thanks, > Atsushi Kumagai > > >--- > > makedumpfile.c | 68 +++++++++++++++++++++++++++++++++------------------------- > > makedumpfile.h | 2 +- > > 2 files changed, 40 insertions(+), 30 deletions(-) > > > >diff --git a/makedumpfile.c b/makedumpfile.c > >index 21784e8..d168dfd 100644 > >--- a/makedumpfile.c > >+++ b/makedumpfile.c > >@@ -3730,10 +3730,10 @@ free_for_parallel() > > return; > > > > for (i = 0; i < info->num_threads; i++) { > >- if (FD_MEMORY_PARALLEL(i) > 0) > >+ if (FD_MEMORY_PARALLEL(i) >= 0) > > close(FD_MEMORY_PARALLEL(i)); > > > >- if (FD_BITMAP_MEMORY_PARALLEL(i) > 0) > >+ if (FD_BITMAP_MEMORY_PARALLEL(i) >= 0) > > close(FD_BITMAP_MEMORY_PARALLEL(i)); > > } > > } > >@@ -4038,13 +4038,13 @@ out: > > void > > initialize_bitmap(struct dump_bitmap *bitmap) > > { > >- if (info->fd_bitmap) { > >+ if (info->fd_bitmap >= 0) { > > bitmap->fd = info->fd_bitmap; > > bitmap->file_name = info->name_bitmap; > > bitmap->no_block = -1; > > memset(bitmap->buf, 0, BUFSIZE_BITMAP); > > } else { > >- bitmap->fd = 0; > >+ bitmap->fd = -1; > > bitmap->file_name = NULL; > > bitmap->no_block = -1; > > memset(bitmap->buf, 0, info->bufsize_cyclic); > >@@ -4154,7 +4154,7 @@ set_bitmap_buffer(struct dump_bitmap *bitmap, mdf_pfn_t pfn, int val, struct cyc > > int > > set_bitmap(struct dump_bitmap *bitmap, mdf_pfn_t pfn, int val, struct cycle *cycle) > > { > >- if (bitmap->fd) { > >+ if (bitmap->fd >= 0) { > > return set_bitmap_file(bitmap, pfn, val); > > } else { > > return set_bitmap_buffer(bitmap, pfn, val, cycle); > >@@ -4170,7 +4170,7 @@ sync_bitmap(struct dump_bitmap *bitmap) > > /* > > * The bitmap doesn't have the fd, it's a on-memory bitmap. > > */ > >- if (bitmap->fd == 0) > >+ if (bitmap->fd < 0) > > return TRUE; > > /* > > * The bitmap buffer is not dirty, and it is not necessary > >@@ -5403,7 +5403,7 @@ create_1st_bitmap_buffer(struct cycle *cycle) > > int > > create_1st_bitmap(struct cycle *cycle) > > { > >- if (info->bitmap1->fd) { > >+ if (info->bitmap1->fd >= 0) { > > return create_1st_bitmap_file(); > > } else { > > return create_1st_bitmap_buffer(cycle); > >@@ -5414,7 +5414,7 @@ static inline int > > is_in_segs(unsigned long long paddr) > > { > > if (info->flag_refiltering || info->flag_sadump) { > >- if (info->bitmap1->fd == 0) { > >+ if (info->bitmap1->fd < 0) { > > initialize_1st_bitmap(info->bitmap1); > > create_1st_bitmap_file(); > > } > >@@ -5872,7 +5872,7 @@ copy_bitmap_file(void) > > int > > copy_bitmap(void) > > { > >- if (info->fd_bitmap) { > >+ if (info->fd_bitmap >= 0) { > > return copy_bitmap_file(); > > } else { > > return copy_bitmap_buffer(); > >@@ -6313,7 +6313,7 @@ prepare_bitmap1_buffer(void) > > return FALSE; > > } > > > >- if (info->fd_bitmap) { > >+ if (info->fd_bitmap >= 0) { > > if ((info->bitmap1->buf = (char *)malloc(BUFSIZE_BITMAP)) == NULL) { > > ERRMSG("Can't allocate memory for the 1st bitmaps's buffer. %s\n", > > strerror(errno)); > >@@ -6352,7 +6352,7 @@ prepare_bitmap2_buffer(void) > > strerror(errno)); > > return FALSE; > > } > >- if (info->fd_bitmap) { > >+ if (info->fd_bitmap >= 0) { > > if ((info->bitmap2->buf = (char *)malloc(BUFSIZE_BITMAP)) == NULL) { > > ERRMSG("Can't allocate memory for the 2nd bitmaps's buffer. %s\n", > > strerror(errno)); > >@@ -7582,7 +7582,7 @@ kdump_thread_function_cyclic(void *arg) { > > > > fd_memory = FD_MEMORY_PARALLEL(kdump_thread_args->thread_num); > > > >- if (info->fd_bitmap) { > >+ if (info->fd_bitmap >= 0) { > > bitmap_parallel.buf = malloc(BUFSIZE_BITMAP); > > if (bitmap_parallel.buf == NULL){ > > ERRMSG("Can't allocate memory for bitmap_parallel.buf. %s\n", > >@@ -7628,7 +7628,7 @@ kdump_thread_function_cyclic(void *arg) { > > pthread_mutex_lock(&info->current_pfn_mutex); > > for (pfn = info->current_pfn; pfn < cycle->end_pfn; pfn++) { > > dumpable = is_dumpable( > >- info->fd_bitmap ? &bitmap_parallel : info->bitmap2, > >+ info->fd_bitmap >= 0 ? &bitmap_parallel : info->bitmap2, > > pfn, > > cycle); > > if (dumpable) > >@@ -7723,7 +7723,7 @@ next: > > retval = NULL; > > > > fail: > >- if (bitmap_memory_parallel.fd > 0) > >+ if (bitmap_memory_parallel.fd >= 0) > > close(bitmap_memory_parallel.fd); > > if (bitmap_parallel.buf != NULL) > > free(bitmap_parallel.buf); > >@@ -8461,7 +8461,7 @@ out: > > > > int > > write_kdump_bitmap1(struct cycle *cycle) { > >- if (info->bitmap1->fd) { > >+ if (info->bitmap1->fd >= 0) { > > return write_kdump_bitmap1_file(); > > } else { > > return write_kdump_bitmap1_buffer(cycle); > >@@ -8470,7 +8470,7 @@ write_kdump_bitmap1(struct cycle *cycle) { > > > > int > > write_kdump_bitmap2(struct cycle *cycle) { > >- if (info->bitmap2->fd) { > >+ if (info->bitmap2->fd >= 0) { > > return write_kdump_bitmap2_file(); > > } else { > > return write_kdump_bitmap2_buffer(cycle); > >@@ -8597,9 +8597,10 @@ close_vmcoreinfo(void) > > void > > close_dump_memory(void) > > { > >- if ((info->fd_memory = close(info->fd_memory)) < 0) > >+ if (close(info->fd_memory) < 0) > > ERRMSG("Can't close the dump memory(%s). %s\n", > > info->name_memory, strerror(errno)); > >+ info->fd_memory = -1; > > } > > > > void > >@@ -8608,9 +8609,10 @@ close_dump_file(void) > > if (info->flag_flatten) > > return; > > > >- if ((info->fd_dumpfile = close(info->fd_dumpfile)) < 0) > >+ if (close(info->fd_dumpfile) < 0) > > ERRMSG("Can't close the dump file(%s). %s\n", > > info->name_dumpfile, strerror(errno)); > >+ info->fd_dumpfile = -1; > > } > > > > void > >@@ -8620,9 +8622,10 @@ close_dump_bitmap(void) > > && !info->flag_sadump && !info->flag_mem_usage) > > return; > > > >- if ((info->fd_bitmap = close(info->fd_bitmap)) < 0) > >+ if (close(info->fd_bitmap) < 0) > > ERRMSG("Can't close the bitmap file(%s). %s\n", > > info->name_bitmap, strerror(errno)); > >+ info->fd_bitmap = -1; > > free(info->name_bitmap); > > info->name_bitmap = NULL; > > } > >@@ -8631,16 +8634,18 @@ void > > close_kernel_file(void) > > { > > if (info->name_vmlinux) { > >- if ((info->fd_vmlinux = close(info->fd_vmlinux)) < 0) { > >+ if (close(info->fd_vmlinux) < 0) { > > ERRMSG("Can't close the kernel file(%s). %s\n", > > info->name_vmlinux, strerror(errno)); > > } > >+ info->fd_vmlinux = -1; > > } > > if (info->name_xen_syms) { > >- if ((info->fd_xen_syms = close(info->fd_xen_syms)) < 0) { > >+ if (close(info->fd_xen_syms) < 0) { > > ERRMSG("Can't close the kernel file(%s). %s\n", > > info->name_xen_syms, strerror(errno)); > > } > >+ info->fd_xen_syms = -1; > > } > > } > > > >@@ -10202,7 +10207,7 @@ reassemble_kdump_header(void) > > > > ret = TRUE; > > out: > >- if (fd > 0) > >+ if (fd >= 0) > > close(fd); > > free(buf_bitmap); > > > >@@ -10212,7 +10217,7 @@ out: > > int > > reassemble_kdump_pages(void) > > { > >- int i, fd = 0, ret = FALSE; > >+ int i, fd = -1, ret = FALSE; > > off_t offset_first_ph, offset_ph_org, offset_eraseinfo; > > off_t offset_data_new, offset_zero_page = 0; > > mdf_pfn_t pfn, start_pfn, end_pfn; > >@@ -10336,7 +10341,7 @@ reassemble_kdump_pages(void) > > offset_data_new += pd.size; > > } > > close(fd); > >- fd = 0; > >+ fd = -1; > > } > > if (!write_cache_bufsz(&cd_pd)) > > goto out; > >@@ -10379,7 +10384,7 @@ reassemble_kdump_pages(void) > > size_eraseinfo += SPLITTING_SIZE_EI(i); > > > > close(fd); > >- fd = 0; > >+ fd = -1; > > } > > if (size_eraseinfo) { > > if (!write_cache_bufsz(&cd_data)) > >@@ -10400,7 +10405,7 @@ out: > > > > if (data) > > free(data); > >- if (fd > 0) > >+ if (fd >= 0) > > close(fd); > > > > return ret; > >@@ -10973,6 +10978,11 @@ main(int argc, char *argv[]) > > strerror(errno)); > > goto out; > > } > >+ info->fd_vmlinux = -1; > >+ info->fd_xen_syms = -1; > >+ info->fd_memory = -1; > >+ info->fd_dumpfile = -1; > >+ info->fd_bitmap = -1; > > initialize_tables(); > > > > /* > >@@ -11268,11 +11278,11 @@ out: > > free(info->bitmap_memory->buf); > > free(info->bitmap_memory); > > } > >- if (info->fd_memory) > >+ if (info->fd_memory >= 0) > > close(info->fd_memory); > >- if (info->fd_dumpfile) > >+ if (info->fd_dumpfile >= 0) > > close(info->fd_dumpfile); > >- if (info->fd_bitmap) > >+ if (info->fd_bitmap >= 0) > > close(info->fd_bitmap); > > if (vt.node_online_map != NULL) > > free(vt.node_online_map); > >diff --git a/makedumpfile.h b/makedumpfile.h > >index 533e5b8..1814139 100644 > >--- a/makedumpfile.h > >+++ b/makedumpfile.h > >@@ -1955,7 +1955,7 @@ is_dumpable_file(struct dump_bitmap *bitmap, mdf_pfn_t pfn) > > static inline int > > is_dumpable(struct dump_bitmap *bitmap, mdf_pfn_t pfn, struct cycle *cycle) > > { > >- if (bitmap->fd == 0) { > >+ if (bitmap->fd < 0) { > > return is_dumpable_buffer(bitmap, pfn, cycle); > > } else { > > return is_dumpable_file(bitmap, pfn); > >-- > >2.6.6 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2] Cleanup: Use a negative number for uninitialized file descriptors 2016-09-09 7:31 ` Petr Tesarik @ 2016-09-09 8:11 ` Petr Tesarik 2016-09-12 8:17 ` Atsushi Kumagai 0 siblings, 1 reply; 24+ messages in thread From: Petr Tesarik @ 2016-09-09 8:11 UTC (permalink / raw) To: Atsushi Kumagai; +Cc: Martin Wilck, kexec Do not use zero to denote an invalid file descriptor. First, zero is a valid value, although quite unlikely to be used for anything except standard input. Second, open(2) returns a negative value on failure, so there are already checks for a negative value in some places. The purpose of this patch is not to allow running in an evil environment (with closed stdin), but to aid in debugging by using a consistent value for uninitialized file descriptors which is also regarded as invalid by the kernel. For example, attempts to close a negative FD will fail (unlike an attempt to close FD 0). Changes from v1: - Cleanup file descriptor usage in dwarf_info.c and sadump_info.c Signed-off-by: Petr Tesarik <ptesarik@suse.com> --- dwarf_info.c | 6 ++++-- makedumpfile.c | 68 +++++++++++++++++++++++++++++++++------------------------- makedumpfile.h | 2 +- sadump_info.c | 3 ++- 4 files changed, 46 insertions(+), 33 deletions(-) diff --git a/dwarf_info.c b/dwarf_info.c index 8c491d3..4f9ad12 100644 --- a/dwarf_info.c +++ b/dwarf_info.c @@ -53,7 +53,9 @@ struct dwarf_info { char src_name[LEN_SRCFILE]; /* OUT */ Dwarf_Off die_offset; /* OUT */ }; -static struct dwarf_info dwarf_info; +static struct dwarf_info dwarf_info = { + .fd_debuginfo = -1, +}; /* @@ -1595,7 +1597,7 @@ set_dwarf_debuginfo(char *mod_name, char *os_release, if (dwarf_info.module_name && strcmp(dwarf_info.module_name, "vmlinux") && strcmp(dwarf_info.module_name, "xen-syms")) { - if (dwarf_info.fd_debuginfo > 0) + if (dwarf_info.fd_debuginfo >= 0) close(dwarf_info.fd_debuginfo); if (dwarf_info.name_debuginfo) free(dwarf_info.name_debuginfo); diff --git a/makedumpfile.c b/makedumpfile.c index 21784e8..d168dfd 100644 --- a/makedumpfile.c +++ b/makedumpfile.c @@ -3730,10 +3730,10 @@ free_for_parallel() return; for (i = 0; i < info->num_threads; i++) { - if (FD_MEMORY_PARALLEL(i) > 0) + if (FD_MEMORY_PARALLEL(i) >= 0) close(FD_MEMORY_PARALLEL(i)); - if (FD_BITMAP_MEMORY_PARALLEL(i) > 0) + if (FD_BITMAP_MEMORY_PARALLEL(i) >= 0) close(FD_BITMAP_MEMORY_PARALLEL(i)); } } @@ -4038,13 +4038,13 @@ out: void initialize_bitmap(struct dump_bitmap *bitmap) { - if (info->fd_bitmap) { + if (info->fd_bitmap >= 0) { bitmap->fd = info->fd_bitmap; bitmap->file_name = info->name_bitmap; bitmap->no_block = -1; memset(bitmap->buf, 0, BUFSIZE_BITMAP); } else { - bitmap->fd = 0; + bitmap->fd = -1; bitmap->file_name = NULL; bitmap->no_block = -1; memset(bitmap->buf, 0, info->bufsize_cyclic); @@ -4154,7 +4154,7 @@ set_bitmap_buffer(struct dump_bitmap *bitmap, mdf_pfn_t pfn, int val, struct cyc int set_bitmap(struct dump_bitmap *bitmap, mdf_pfn_t pfn, int val, struct cycle *cycle) { - if (bitmap->fd) { + if (bitmap->fd >= 0) { return set_bitmap_file(bitmap, pfn, val); } else { return set_bitmap_buffer(bitmap, pfn, val, cycle); @@ -4170,7 +4170,7 @@ sync_bitmap(struct dump_bitmap *bitmap) /* * The bitmap doesn't have the fd, it's a on-memory bitmap. */ - if (bitmap->fd == 0) + if (bitmap->fd < 0) return TRUE; /* * The bitmap buffer is not dirty, and it is not necessary @@ -5403,7 +5403,7 @@ create_1st_bitmap_buffer(struct cycle *cycle) int create_1st_bitmap(struct cycle *cycle) { - if (info->bitmap1->fd) { + if (info->bitmap1->fd >= 0) { return create_1st_bitmap_file(); } else { return create_1st_bitmap_buffer(cycle); @@ -5414,7 +5414,7 @@ static inline int is_in_segs(unsigned long long paddr) { if (info->flag_refiltering || info->flag_sadump) { - if (info->bitmap1->fd == 0) { + if (info->bitmap1->fd < 0) { initialize_1st_bitmap(info->bitmap1); create_1st_bitmap_file(); } @@ -5872,7 +5872,7 @@ copy_bitmap_file(void) int copy_bitmap(void) { - if (info->fd_bitmap) { + if (info->fd_bitmap >= 0) { return copy_bitmap_file(); } else { return copy_bitmap_buffer(); @@ -6313,7 +6313,7 @@ prepare_bitmap1_buffer(void) return FALSE; } - if (info->fd_bitmap) { + if (info->fd_bitmap >= 0) { if ((info->bitmap1->buf = (char *)malloc(BUFSIZE_BITMAP)) == NULL) { ERRMSG("Can't allocate memory for the 1st bitmaps's buffer. %s\n", strerror(errno)); @@ -6352,7 +6352,7 @@ prepare_bitmap2_buffer(void) strerror(errno)); return FALSE; } - if (info->fd_bitmap) { + if (info->fd_bitmap >= 0) { if ((info->bitmap2->buf = (char *)malloc(BUFSIZE_BITMAP)) == NULL) { ERRMSG("Can't allocate memory for the 2nd bitmaps's buffer. %s\n", strerror(errno)); @@ -7582,7 +7582,7 @@ kdump_thread_function_cyclic(void *arg) { fd_memory = FD_MEMORY_PARALLEL(kdump_thread_args->thread_num); - if (info->fd_bitmap) { + if (info->fd_bitmap >= 0) { bitmap_parallel.buf = malloc(BUFSIZE_BITMAP); if (bitmap_parallel.buf == NULL){ ERRMSG("Can't allocate memory for bitmap_parallel.buf. %s\n", @@ -7628,7 +7628,7 @@ kdump_thread_function_cyclic(void *arg) { pthread_mutex_lock(&info->current_pfn_mutex); for (pfn = info->current_pfn; pfn < cycle->end_pfn; pfn++) { dumpable = is_dumpable( - info->fd_bitmap ? &bitmap_parallel : info->bitmap2, + info->fd_bitmap >= 0 ? &bitmap_parallel : info->bitmap2, pfn, cycle); if (dumpable) @@ -7723,7 +7723,7 @@ next: retval = NULL; fail: - if (bitmap_memory_parallel.fd > 0) + if (bitmap_memory_parallel.fd >= 0) close(bitmap_memory_parallel.fd); if (bitmap_parallel.buf != NULL) free(bitmap_parallel.buf); @@ -8461,7 +8461,7 @@ out: int write_kdump_bitmap1(struct cycle *cycle) { - if (info->bitmap1->fd) { + if (info->bitmap1->fd >= 0) { return write_kdump_bitmap1_file(); } else { return write_kdump_bitmap1_buffer(cycle); @@ -8470,7 +8470,7 @@ write_kdump_bitmap1(struct cycle *cycle) { int write_kdump_bitmap2(struct cycle *cycle) { - if (info->bitmap2->fd) { + if (info->bitmap2->fd >= 0) { return write_kdump_bitmap2_file(); } else { return write_kdump_bitmap2_buffer(cycle); @@ -8597,9 +8597,10 @@ close_vmcoreinfo(void) void close_dump_memory(void) { - if ((info->fd_memory = close(info->fd_memory)) < 0) + if (close(info->fd_memory) < 0) ERRMSG("Can't close the dump memory(%s). %s\n", info->name_memory, strerror(errno)); + info->fd_memory = -1; } void @@ -8608,9 +8609,10 @@ close_dump_file(void) if (info->flag_flatten) return; - if ((info->fd_dumpfile = close(info->fd_dumpfile)) < 0) + if (close(info->fd_dumpfile) < 0) ERRMSG("Can't close the dump file(%s). %s\n", info->name_dumpfile, strerror(errno)); + info->fd_dumpfile = -1; } void @@ -8620,9 +8622,10 @@ close_dump_bitmap(void) && !info->flag_sadump && !info->flag_mem_usage) return; - if ((info->fd_bitmap = close(info->fd_bitmap)) < 0) + if (close(info->fd_bitmap) < 0) ERRMSG("Can't close the bitmap file(%s). %s\n", info->name_bitmap, strerror(errno)); + info->fd_bitmap = -1; free(info->name_bitmap); info->name_bitmap = NULL; } @@ -8631,16 +8634,18 @@ void close_kernel_file(void) { if (info->name_vmlinux) { - if ((info->fd_vmlinux = close(info->fd_vmlinux)) < 0) { + if (close(info->fd_vmlinux) < 0) { ERRMSG("Can't close the kernel file(%s). %s\n", info->name_vmlinux, strerror(errno)); } + info->fd_vmlinux = -1; } if (info->name_xen_syms) { - if ((info->fd_xen_syms = close(info->fd_xen_syms)) < 0) { + if (close(info->fd_xen_syms) < 0) { ERRMSG("Can't close the kernel file(%s). %s\n", info->name_xen_syms, strerror(errno)); } + info->fd_xen_syms = -1; } } @@ -10202,7 +10207,7 @@ reassemble_kdump_header(void) ret = TRUE; out: - if (fd > 0) + if (fd >= 0) close(fd); free(buf_bitmap); @@ -10212,7 +10217,7 @@ out: int reassemble_kdump_pages(void) { - int i, fd = 0, ret = FALSE; + int i, fd = -1, ret = FALSE; off_t offset_first_ph, offset_ph_org, offset_eraseinfo; off_t offset_data_new, offset_zero_page = 0; mdf_pfn_t pfn, start_pfn, end_pfn; @@ -10336,7 +10341,7 @@ reassemble_kdump_pages(void) offset_data_new += pd.size; } close(fd); - fd = 0; + fd = -1; } if (!write_cache_bufsz(&cd_pd)) goto out; @@ -10379,7 +10384,7 @@ reassemble_kdump_pages(void) size_eraseinfo += SPLITTING_SIZE_EI(i); close(fd); - fd = 0; + fd = -1; } if (size_eraseinfo) { if (!write_cache_bufsz(&cd_data)) @@ -10400,7 +10405,7 @@ out: if (data) free(data); - if (fd > 0) + if (fd >= 0) close(fd); return ret; @@ -10973,6 +10978,11 @@ main(int argc, char *argv[]) strerror(errno)); goto out; } + info->fd_vmlinux = -1; + info->fd_xen_syms = -1; + info->fd_memory = -1; + info->fd_dumpfile = -1; + info->fd_bitmap = -1; initialize_tables(); /* @@ -11268,11 +11278,11 @@ out: free(info->bitmap_memory->buf); free(info->bitmap_memory); } - if (info->fd_memory) + if (info->fd_memory >= 0) close(info->fd_memory); - if (info->fd_dumpfile) + if (info->fd_dumpfile >= 0) close(info->fd_dumpfile); - if (info->fd_bitmap) + if (info->fd_bitmap >= 0) close(info->fd_bitmap); if (vt.node_online_map != NULL) free(vt.node_online_map); diff --git a/makedumpfile.h b/makedumpfile.h index 533e5b8..1814139 100644 --- a/makedumpfile.h +++ b/makedumpfile.h @@ -1955,7 +1955,7 @@ is_dumpable_file(struct dump_bitmap *bitmap, mdf_pfn_t pfn) static inline int is_dumpable(struct dump_bitmap *bitmap, mdf_pfn_t pfn, struct cycle *cycle) { - if (bitmap->fd == 0) { + if (bitmap->fd < 0) { return is_dumpable_buffer(bitmap, pfn, cycle); } else { return is_dumpable_file(bitmap, pfn); diff --git a/sadump_info.c b/sadump_info.c index 5ff5595..f77a020 100644 --- a/sadump_info.c +++ b/sadump_info.c @@ -1853,6 +1853,7 @@ sadump_add_diskset_info(char *name_memory) } si->diskset_info[si->num_disks - 1].name_memory = name_memory; + si->diskset_info[si->num_disks - 1].fd_memory = -1; return TRUE; } @@ -1917,7 +1918,7 @@ free_sadump_info(void) int i; for (i = 1; i < si->num_disks; ++i) { - if (si->diskset_info[i].fd_memory) + if (si->diskset_info[i].fd_memory >= 0) close(si->diskset_info[i].fd_memory); if (si->diskset_info[i].sph_memory) free(si->diskset_info[i].sph_memory); -- 2.6.6 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply related [flat|nested] 24+ messages in thread
* RE: [PATCH v2] Cleanup: Use a negative number for uninitialized file descriptors 2016-09-09 8:11 ` [PATCH v2] " Petr Tesarik @ 2016-09-12 8:17 ` Atsushi Kumagai 2016-09-14 10:17 ` [PATCH v2 1/3] open_dump_bitmap: open bitmap file in non-cyclic case Martin Wilck 0 siblings, 1 reply; 24+ messages in thread From: Atsushi Kumagai @ 2016-09-12 8:17 UTC (permalink / raw) To: Petr Tesarik, Martin Wilck; +Cc: kexec Hello, >Do not use zero to denote an invalid file descriptor. > >First, zero is a valid value, although quite unlikely to be used for >anything except standard input. > >Second, open(2) returns a negative value on failure, so there are >already checks for a negative value in some places. > >The purpose of this patch is not to allow running in an evil environment >(with closed stdin), but to aid in debugging by using a consistent value >for uninitialized file descriptors which is also regarded as invalid by >the kernel. For example, attempts to close a negative FD will fail >(unlike an attempt to close FD 0). > >Changes from v1: > - Cleanup file descriptor usage in dwarf_info.c and sadump_info.c Thanks for your quick response. This fix isn't necessary but better to do it as you said. Martin, could you rebase your three patches ? I've updated the devel branch. Thanks, Atsushi Kumagai >Signed-off-by: Petr Tesarik <ptesarik@suse.com> > >--- > dwarf_info.c | 6 ++++-- > makedumpfile.c | 68 +++++++++++++++++++++++++++++++++------------------------- > makedumpfile.h | 2 +- > sadump_info.c | 3 ++- > 4 files changed, 46 insertions(+), 33 deletions(-) > >diff --git a/dwarf_info.c b/dwarf_info.c >index 8c491d3..4f9ad12 100644 >--- a/dwarf_info.c >+++ b/dwarf_info.c >@@ -53,7 +53,9 @@ struct dwarf_info { > char src_name[LEN_SRCFILE]; /* OUT */ > Dwarf_Off die_offset; /* OUT */ > }; >-static struct dwarf_info dwarf_info; >+static struct dwarf_info dwarf_info = { >+ .fd_debuginfo = -1, >+}; > > > /* >@@ -1595,7 +1597,7 @@ set_dwarf_debuginfo(char *mod_name, char *os_release, > if (dwarf_info.module_name > && strcmp(dwarf_info.module_name, "vmlinux") > && strcmp(dwarf_info.module_name, "xen-syms")) { >- if (dwarf_info.fd_debuginfo > 0) >+ if (dwarf_info.fd_debuginfo >= 0) > close(dwarf_info.fd_debuginfo); > if (dwarf_info.name_debuginfo) > free(dwarf_info.name_debuginfo); >diff --git a/makedumpfile.c b/makedumpfile.c >index 21784e8..d168dfd 100644 >--- a/makedumpfile.c >+++ b/makedumpfile.c >@@ -3730,10 +3730,10 @@ free_for_parallel() > return; > > for (i = 0; i < info->num_threads; i++) { >- if (FD_MEMORY_PARALLEL(i) > 0) >+ if (FD_MEMORY_PARALLEL(i) >= 0) > close(FD_MEMORY_PARALLEL(i)); > >- if (FD_BITMAP_MEMORY_PARALLEL(i) > 0) >+ if (FD_BITMAP_MEMORY_PARALLEL(i) >= 0) > close(FD_BITMAP_MEMORY_PARALLEL(i)); > } > } >@@ -4038,13 +4038,13 @@ out: > void > initialize_bitmap(struct dump_bitmap *bitmap) > { >- if (info->fd_bitmap) { >+ if (info->fd_bitmap >= 0) { > bitmap->fd = info->fd_bitmap; > bitmap->file_name = info->name_bitmap; > bitmap->no_block = -1; > memset(bitmap->buf, 0, BUFSIZE_BITMAP); > } else { >- bitmap->fd = 0; >+ bitmap->fd = -1; > bitmap->file_name = NULL; > bitmap->no_block = -1; > memset(bitmap->buf, 0, info->bufsize_cyclic); >@@ -4154,7 +4154,7 @@ set_bitmap_buffer(struct dump_bitmap *bitmap, mdf_pfn_t pfn, int val, struct cyc > int > set_bitmap(struct dump_bitmap *bitmap, mdf_pfn_t pfn, int val, struct cycle *cycle) > { >- if (bitmap->fd) { >+ if (bitmap->fd >= 0) { > return set_bitmap_file(bitmap, pfn, val); > } else { > return set_bitmap_buffer(bitmap, pfn, val, cycle); >@@ -4170,7 +4170,7 @@ sync_bitmap(struct dump_bitmap *bitmap) > /* > * The bitmap doesn't have the fd, it's a on-memory bitmap. > */ >- if (bitmap->fd == 0) >+ if (bitmap->fd < 0) > return TRUE; > /* > * The bitmap buffer is not dirty, and it is not necessary >@@ -5403,7 +5403,7 @@ create_1st_bitmap_buffer(struct cycle *cycle) > int > create_1st_bitmap(struct cycle *cycle) > { >- if (info->bitmap1->fd) { >+ if (info->bitmap1->fd >= 0) { > return create_1st_bitmap_file(); > } else { > return create_1st_bitmap_buffer(cycle); >@@ -5414,7 +5414,7 @@ static inline int > is_in_segs(unsigned long long paddr) > { > if (info->flag_refiltering || info->flag_sadump) { >- if (info->bitmap1->fd == 0) { >+ if (info->bitmap1->fd < 0) { > initialize_1st_bitmap(info->bitmap1); > create_1st_bitmap_file(); > } >@@ -5872,7 +5872,7 @@ copy_bitmap_file(void) > int > copy_bitmap(void) > { >- if (info->fd_bitmap) { >+ if (info->fd_bitmap >= 0) { > return copy_bitmap_file(); > } else { > return copy_bitmap_buffer(); >@@ -6313,7 +6313,7 @@ prepare_bitmap1_buffer(void) > return FALSE; > } > >- if (info->fd_bitmap) { >+ if (info->fd_bitmap >= 0) { > if ((info->bitmap1->buf = (char *)malloc(BUFSIZE_BITMAP)) == NULL) { > ERRMSG("Can't allocate memory for the 1st bitmaps's buffer. %s\n", > strerror(errno)); >@@ -6352,7 +6352,7 @@ prepare_bitmap2_buffer(void) > strerror(errno)); > return FALSE; > } >- if (info->fd_bitmap) { >+ if (info->fd_bitmap >= 0) { > if ((info->bitmap2->buf = (char *)malloc(BUFSIZE_BITMAP)) == NULL) { > ERRMSG("Can't allocate memory for the 2nd bitmaps's buffer. %s\n", > strerror(errno)); >@@ -7582,7 +7582,7 @@ kdump_thread_function_cyclic(void *arg) { > > fd_memory = FD_MEMORY_PARALLEL(kdump_thread_args->thread_num); > >- if (info->fd_bitmap) { >+ if (info->fd_bitmap >= 0) { > bitmap_parallel.buf = malloc(BUFSIZE_BITMAP); > if (bitmap_parallel.buf == NULL){ > ERRMSG("Can't allocate memory for bitmap_parallel.buf. %s\n", >@@ -7628,7 +7628,7 @@ kdump_thread_function_cyclic(void *arg) { > pthread_mutex_lock(&info->current_pfn_mutex); > for (pfn = info->current_pfn; pfn < cycle->end_pfn; pfn++) { > dumpable = is_dumpable( >- info->fd_bitmap ? &bitmap_parallel : info->bitmap2, >+ info->fd_bitmap >= 0 ? &bitmap_parallel : info->bitmap2, > pfn, > cycle); > if (dumpable) >@@ -7723,7 +7723,7 @@ next: > retval = NULL; > > fail: >- if (bitmap_memory_parallel.fd > 0) >+ if (bitmap_memory_parallel.fd >= 0) > close(bitmap_memory_parallel.fd); > if (bitmap_parallel.buf != NULL) > free(bitmap_parallel.buf); >@@ -8461,7 +8461,7 @@ out: > > int > write_kdump_bitmap1(struct cycle *cycle) { >- if (info->bitmap1->fd) { >+ if (info->bitmap1->fd >= 0) { > return write_kdump_bitmap1_file(); > } else { > return write_kdump_bitmap1_buffer(cycle); >@@ -8470,7 +8470,7 @@ write_kdump_bitmap1(struct cycle *cycle) { > > int > write_kdump_bitmap2(struct cycle *cycle) { >- if (info->bitmap2->fd) { >+ if (info->bitmap2->fd >= 0) { > return write_kdump_bitmap2_file(); > } else { > return write_kdump_bitmap2_buffer(cycle); >@@ -8597,9 +8597,10 @@ close_vmcoreinfo(void) > void > close_dump_memory(void) > { >- if ((info->fd_memory = close(info->fd_memory)) < 0) >+ if (close(info->fd_memory) < 0) > ERRMSG("Can't close the dump memory(%s). %s\n", > info->name_memory, strerror(errno)); >+ info->fd_memory = -1; > } > > void >@@ -8608,9 +8609,10 @@ close_dump_file(void) > if (info->flag_flatten) > return; > >- if ((info->fd_dumpfile = close(info->fd_dumpfile)) < 0) >+ if (close(info->fd_dumpfile) < 0) > ERRMSG("Can't close the dump file(%s). %s\n", > info->name_dumpfile, strerror(errno)); >+ info->fd_dumpfile = -1; > } > > void >@@ -8620,9 +8622,10 @@ close_dump_bitmap(void) > && !info->flag_sadump && !info->flag_mem_usage) > return; > >- if ((info->fd_bitmap = close(info->fd_bitmap)) < 0) >+ if (close(info->fd_bitmap) < 0) > ERRMSG("Can't close the bitmap file(%s). %s\n", > info->name_bitmap, strerror(errno)); >+ info->fd_bitmap = -1; > free(info->name_bitmap); > info->name_bitmap = NULL; > } >@@ -8631,16 +8634,18 @@ void > close_kernel_file(void) > { > if (info->name_vmlinux) { >- if ((info->fd_vmlinux = close(info->fd_vmlinux)) < 0) { >+ if (close(info->fd_vmlinux) < 0) { > ERRMSG("Can't close the kernel file(%s). %s\n", > info->name_vmlinux, strerror(errno)); > } >+ info->fd_vmlinux = -1; > } > if (info->name_xen_syms) { >- if ((info->fd_xen_syms = close(info->fd_xen_syms)) < 0) { >+ if (close(info->fd_xen_syms) < 0) { > ERRMSG("Can't close the kernel file(%s). %s\n", > info->name_xen_syms, strerror(errno)); > } >+ info->fd_xen_syms = -1; > } > } > >@@ -10202,7 +10207,7 @@ reassemble_kdump_header(void) > > ret = TRUE; > out: >- if (fd > 0) >+ if (fd >= 0) > close(fd); > free(buf_bitmap); > >@@ -10212,7 +10217,7 @@ out: > int > reassemble_kdump_pages(void) > { >- int i, fd = 0, ret = FALSE; >+ int i, fd = -1, ret = FALSE; > off_t offset_first_ph, offset_ph_org, offset_eraseinfo; > off_t offset_data_new, offset_zero_page = 0; > mdf_pfn_t pfn, start_pfn, end_pfn; >@@ -10336,7 +10341,7 @@ reassemble_kdump_pages(void) > offset_data_new += pd.size; > } > close(fd); >- fd = 0; >+ fd = -1; > } > if (!write_cache_bufsz(&cd_pd)) > goto out; >@@ -10379,7 +10384,7 @@ reassemble_kdump_pages(void) > size_eraseinfo += SPLITTING_SIZE_EI(i); > > close(fd); >- fd = 0; >+ fd = -1; > } > if (size_eraseinfo) { > if (!write_cache_bufsz(&cd_data)) >@@ -10400,7 +10405,7 @@ out: > > if (data) > free(data); >- if (fd > 0) >+ if (fd >= 0) > close(fd); > > return ret; >@@ -10973,6 +10978,11 @@ main(int argc, char *argv[]) > strerror(errno)); > goto out; > } >+ info->fd_vmlinux = -1; >+ info->fd_xen_syms = -1; >+ info->fd_memory = -1; >+ info->fd_dumpfile = -1; >+ info->fd_bitmap = -1; > initialize_tables(); > > /* >@@ -11268,11 +11278,11 @@ out: > free(info->bitmap_memory->buf); > free(info->bitmap_memory); > } >- if (info->fd_memory) >+ if (info->fd_memory >= 0) > close(info->fd_memory); >- if (info->fd_dumpfile) >+ if (info->fd_dumpfile >= 0) > close(info->fd_dumpfile); >- if (info->fd_bitmap) >+ if (info->fd_bitmap >= 0) > close(info->fd_bitmap); > if (vt.node_online_map != NULL) > free(vt.node_online_map); >diff --git a/makedumpfile.h b/makedumpfile.h >index 533e5b8..1814139 100644 >--- a/makedumpfile.h >+++ b/makedumpfile.h >@@ -1955,7 +1955,7 @@ is_dumpable_file(struct dump_bitmap *bitmap, mdf_pfn_t pfn) > static inline int > is_dumpable(struct dump_bitmap *bitmap, mdf_pfn_t pfn, struct cycle *cycle) > { >- if (bitmap->fd == 0) { >+ if (bitmap->fd < 0) { > return is_dumpable_buffer(bitmap, pfn, cycle); > } else { > return is_dumpable_file(bitmap, pfn); >diff --git a/sadump_info.c b/sadump_info.c >index 5ff5595..f77a020 100644 >--- a/sadump_info.c >+++ b/sadump_info.c >@@ -1853,6 +1853,7 @@ sadump_add_diskset_info(char *name_memory) > } > > si->diskset_info[si->num_disks - 1].name_memory = name_memory; >+ si->diskset_info[si->num_disks - 1].fd_memory = -1; > > return TRUE; > } >@@ -1917,7 +1918,7 @@ free_sadump_info(void) > int i; > > for (i = 1; i < si->num_disks; ++i) { >- if (si->diskset_info[i].fd_memory) >+ if (si->diskset_info[i].fd_memory >= 0) > close(si->diskset_info[i].fd_memory); > if (si->diskset_info[i].sph_memory) > free(si->diskset_info[i].sph_memory); >-- >2.6.6 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 1/3] open_dump_bitmap: open bitmap file in non-cyclic case 2016-09-12 8:17 ` Atsushi Kumagai @ 2016-09-14 10:17 ` Martin Wilck 2016-09-14 10:17 ` [PATCH v2 2/3] move call to open_dump_bitmap() to after call to initial() Martin Wilck 2016-09-14 10:17 ` [PATCH v2 3/3] close_dump_bitmap: simplify logic Martin Wilck 0 siblings, 2 replies; 24+ messages in thread From: Martin Wilck @ 2016-09-14 10:17 UTC (permalink / raw) To: ats-kumagai; +Cc: mwilck, ptesarik, kexec The logic of set_bitmap() requires that a bitmap fd exists in the non-cyclic case. --- makedumpfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/makedumpfile.c b/makedumpfile.c index d168dfd..6164468 100644 --- a/makedumpfile.c +++ b/makedumpfile.c @@ -1365,7 +1365,7 @@ open_dump_bitmap(void) /* Unnecessary to open */ if (!info->working_dir && !info->flag_reassemble && !info->flag_refiltering - && !info->flag_sadump && !info->flag_mem_usage) + && !info->flag_sadump && !info->flag_mem_usage && info->flag_cyclic) return TRUE; tmpname = getenv("TMPDIR"); -- 2.9.3 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 2/3] move call to open_dump_bitmap() to after call to initial() 2016-09-14 10:17 ` [PATCH v2 1/3] open_dump_bitmap: open bitmap file in non-cyclic case Martin Wilck @ 2016-09-14 10:17 ` Martin Wilck 2016-09-14 10:17 ` [PATCH v2 3/3] close_dump_bitmap: simplify logic Martin Wilck 1 sibling, 0 replies; 24+ messages in thread From: Martin Wilck @ 2016-09-14 10:17 UTC (permalink / raw) To: ats-kumagai; +Cc: mwilck, ptesarik, kexec When open_files_for_creating_dumpfile() is called, we don't have all necessary information to determine whether a bitmap file is actually needed. In particular, we don't know whether info->flag_cyclic will ultimately be set. This patch moves the call to open_dump_bitmap() to after initialize() when all flags are known. For the dump_dmesg() path, no bitmap file is needed. --- makedumpfile.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/makedumpfile.c b/makedumpfile.c index 6164468..30e1fa8 100644 --- a/makedumpfile.c +++ b/makedumpfile.c @@ -1480,9 +1480,6 @@ open_files_for_creating_dumpfile(void) if (!open_dump_memory()) return FALSE; - if (!open_dump_bitmap()) - return FALSE; - return TRUE; } @@ -9747,6 +9744,9 @@ create_dumpfile(void) if (!initial()) return FALSE; + if (!open_dump_bitmap()) + return FALSE; + /* create an array of translations from pfn to vmemmap pages */ if (info->flag_excludevm) { if (find_vmemmap() == FAILED) { @@ -10917,6 +10917,9 @@ int show_mem_usage(void) if (!initial()) return FALSE; + if (!open_dump_bitmap()) + return FALSE; + if (!prepare_bitmap_buffer()) return FALSE; -- 2.9.3 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 3/3] close_dump_bitmap: simplify logic 2016-09-14 10:17 ` [PATCH v2 1/3] open_dump_bitmap: open bitmap file in non-cyclic case Martin Wilck 2016-09-14 10:17 ` [PATCH v2 2/3] move call to open_dump_bitmap() to after call to initial() Martin Wilck @ 2016-09-14 10:17 ` Martin Wilck 2016-09-14 11:50 ` [PATCH v3 " Martin Wilck 1 sibling, 1 reply; 24+ messages in thread From: Martin Wilck @ 2016-09-14 10:17 UTC (permalink / raw) To: ats-kumagai; +Cc: mwilck, ptesarik, kexec The boolean expression replicates the logic of open_dump_bitmap(). It's simpler and less error-prone to simply check if fd_bitmap is valid. --- makedumpfile.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/makedumpfile.c b/makedumpfile.c index 30e1fa8..d46777c 100644 --- a/makedumpfile.c +++ b/makedumpfile.c @@ -8615,8 +8615,7 @@ close_dump_file(void) void close_dump_bitmap(void) { - if (!info->working_dir && !info->flag_reassemble && !info->flag_refiltering - && !info->flag_sadump && !info->flag_mem_usage) + if (!info->fd_bitmap) return; if (close(info->fd_bitmap) < 0) -- 2.9.3 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 3/3] close_dump_bitmap: simplify logic 2016-09-14 10:17 ` [PATCH v2 3/3] close_dump_bitmap: simplify logic Martin Wilck @ 2016-09-14 11:50 ` Martin Wilck 2016-09-20 9:43 ` Atsushi Kumagai 0 siblings, 1 reply; 24+ messages in thread From: Martin Wilck @ 2016-09-14 11:50 UTC (permalink / raw) To: ats-kumagai; +Cc: mwilck, ptesarik, kexec The boolean expression replicates the logic of open_dump_bitmap(). It's simpler and less error-prone to simply check if fd_bitmap is valid. (I forgot to apply the very change that Petr had asked for in V2 of this patch. I'm very sorry. Please apply V3). --- makedumpfile.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/makedumpfile.c b/makedumpfile.c index 30e1fa8..0f5be7f 100644 --- a/makedumpfile.c +++ b/makedumpfile.c @@ -8615,8 +8615,7 @@ close_dump_file(void) void close_dump_bitmap(void) { - if (!info->working_dir && !info->flag_reassemble && !info->flag_refiltering - && !info->flag_sadump && !info->flag_mem_usage) + if (info->fd_bitmap < 0) return; if (close(info->fd_bitmap) < 0) -- 2.9.3 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply related [flat|nested] 24+ messages in thread
* RE: [PATCH v3 3/3] close_dump_bitmap: simplify logic 2016-09-14 11:50 ` [PATCH v3 " Martin Wilck @ 2016-09-20 9:43 ` Atsushi Kumagai 2016-09-20 10:31 ` Martin Wilck 2016-09-20 10:33 ` [PATCH v3 1/3] open_dump_bitmap: open bitmap file in non-cyclic case Martin Wilck 0 siblings, 2 replies; 24+ messages in thread From: Atsushi Kumagai @ 2016-09-20 9:43 UTC (permalink / raw) To: Martin Wilck; +Cc: ptesarik, kexec Hello Martin, >The boolean expression replicates the logic of open_dump_bitmap(). >It's simpler and less error-prone to simply check if fd_bitmap is >valid. > >(I forgot to apply the very change that Petr had asked for in V2 of >this patch. I'm very sorry. Please apply V3). This version looks good, so could you add your Signed-off-by ? Thanks, Atsushi Kumagai >--- > makedumpfile.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > >diff --git a/makedumpfile.c b/makedumpfile.c >index 30e1fa8..0f5be7f 100644 >--- a/makedumpfile.c >+++ b/makedumpfile.c >@@ -8615,8 +8615,7 @@ close_dump_file(void) > void > close_dump_bitmap(void) > { >- if (!info->working_dir && !info->flag_reassemble && !info->flag_refiltering >- && !info->flag_sadump && !info->flag_mem_usage) >+ if (info->fd_bitmap < 0) > return; > > if (close(info->fd_bitmap) < 0) >-- >2.9.3 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v3 3/3] close_dump_bitmap: simplify logic 2016-09-20 9:43 ` Atsushi Kumagai @ 2016-09-20 10:31 ` Martin Wilck 2016-09-20 10:33 ` [PATCH v3 1/3] open_dump_bitmap: open bitmap file in non-cyclic case Martin Wilck 1 sibling, 0 replies; 24+ messages in thread From: Martin Wilck @ 2016-09-20 10:31 UTC (permalink / raw) To: Atsushi Kumagai; +Cc: ptesarik, kexec Kumagai-san, On Tue, 2016-09-20 at 09:43 +0000, Atsushi Kumagai wrote: > > This version looks good, so could you add your Signed-off-by ? OK, I'll resend the whole v3 series. Martin _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3 1/3] open_dump_bitmap: open bitmap file in non-cyclic case 2016-09-20 9:43 ` Atsushi Kumagai 2016-09-20 10:31 ` Martin Wilck @ 2016-09-20 10:33 ` Martin Wilck 2016-09-20 10:33 ` [PATCH v3 2/3] move call to open_dump_bitmap() to after call to initial() Martin Wilck ` (2 more replies) 1 sibling, 3 replies; 24+ messages in thread From: Martin Wilck @ 2016-09-20 10:33 UTC (permalink / raw) To: ats-kumagai; +Cc: Martin Wilck, ptesarik, kexec The logic of set_bitmap() requires that a bitmap fd exists in the non-cyclic case. Signed-off-by: Martin Wilck <mwilck@suse.de> --- makedumpfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/makedumpfile.c b/makedumpfile.c index d168dfd..6164468 100644 --- a/makedumpfile.c +++ b/makedumpfile.c @@ -1365,7 +1365,7 @@ open_dump_bitmap(void) /* Unnecessary to open */ if (!info->working_dir && !info->flag_reassemble && !info->flag_refiltering - && !info->flag_sadump && !info->flag_mem_usage) + && !info->flag_sadump && !info->flag_mem_usage && info->flag_cyclic) return TRUE; tmpname = getenv("TMPDIR"); -- 2.9.3 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 2/3] move call to open_dump_bitmap() to after call to initial() 2016-09-20 10:33 ` [PATCH v3 1/3] open_dump_bitmap: open bitmap file in non-cyclic case Martin Wilck @ 2016-09-20 10:33 ` Martin Wilck 2016-09-20 10:33 ` [PATCH v3 3/3] close_dump_bitmap: simplify logic Martin Wilck 2016-09-21 6:29 ` [PATCH v3 1/3] open_dump_bitmap: open bitmap file in non-cyclic case Atsushi Kumagai 2 siblings, 0 replies; 24+ messages in thread From: Martin Wilck @ 2016-09-20 10:33 UTC (permalink / raw) To: ats-kumagai; +Cc: Martin Wilck, ptesarik, kexec When open_files_for_creating_dumpfile() is called, we don't have all necessary information to determine whether a bitmap file is actually needed. In particular, we don't know whether info->flag_cyclic will ultimately be set. This patch moves the call to open_dump_bitmap() to after initialize() when all flags are known. For the dump_dmesg() path, no bitmap file is needed. Signed-off-by: Martin Wilck <mwilck@suse.de> --- makedumpfile.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/makedumpfile.c b/makedumpfile.c index 6164468..30e1fa8 100644 --- a/makedumpfile.c +++ b/makedumpfile.c @@ -1480,9 +1480,6 @@ open_files_for_creating_dumpfile(void) if (!open_dump_memory()) return FALSE; - if (!open_dump_bitmap()) - return FALSE; - return TRUE; } @@ -9747,6 +9744,9 @@ create_dumpfile(void) if (!initial()) return FALSE; + if (!open_dump_bitmap()) + return FALSE; + /* create an array of translations from pfn to vmemmap pages */ if (info->flag_excludevm) { if (find_vmemmap() == FAILED) { @@ -10917,6 +10917,9 @@ int show_mem_usage(void) if (!initial()) return FALSE; + if (!open_dump_bitmap()) + return FALSE; + if (!prepare_bitmap_buffer()) return FALSE; -- 2.9.3 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v3 3/3] close_dump_bitmap: simplify logic 2016-09-20 10:33 ` [PATCH v3 1/3] open_dump_bitmap: open bitmap file in non-cyclic case Martin Wilck 2016-09-20 10:33 ` [PATCH v3 2/3] move call to open_dump_bitmap() to after call to initial() Martin Wilck @ 2016-09-20 10:33 ` Martin Wilck 2016-09-21 6:29 ` [PATCH v3 1/3] open_dump_bitmap: open bitmap file in non-cyclic case Atsushi Kumagai 2 siblings, 0 replies; 24+ messages in thread From: Martin Wilck @ 2016-09-20 10:33 UTC (permalink / raw) To: ats-kumagai; +Cc: Martin Wilck, ptesarik, kexec The boolean expression replicates the logic of open_dump_bitmap(). It's simpler and less error-prone to simply check if fd_bitmap is valid. Signed-off-by: Martin Wilck <mwilck@suse.de> --- makedumpfile.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/makedumpfile.c b/makedumpfile.c index 30e1fa8..0f5be7f 100644 --- a/makedumpfile.c +++ b/makedumpfile.c @@ -8615,8 +8615,7 @@ close_dump_file(void) void close_dump_bitmap(void) { - if (!info->working_dir && !info->flag_reassemble && !info->flag_refiltering - && !info->flag_sadump && !info->flag_mem_usage) + if (info->fd_bitmap < 0) return; if (close(info->fd_bitmap) < 0) -- 2.9.3 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply related [flat|nested] 24+ messages in thread
* RE: [PATCH v3 1/3] open_dump_bitmap: open bitmap file in non-cyclic case 2016-09-20 10:33 ` [PATCH v3 1/3] open_dump_bitmap: open bitmap file in non-cyclic case Martin Wilck 2016-09-20 10:33 ` [PATCH v3 2/3] move call to open_dump_bitmap() to after call to initial() Martin Wilck 2016-09-20 10:33 ` [PATCH v3 3/3] close_dump_bitmap: simplify logic Martin Wilck @ 2016-09-21 6:29 ` Atsushi Kumagai 2 siblings, 0 replies; 24+ messages in thread From: Atsushi Kumagai @ 2016-09-21 6:29 UTC (permalink / raw) To: Martin Wilck; +Cc: ptesarik, kexec Hello Martin, >The logic of set_bitmap() requires that a bitmap fd exists in the >non-cyclic case. > >Signed-off-by: Martin Wilck <mwilck@suse.de> I'll merge this version into v1.6.1, thanks for your work. Regards, Atsushi Kumagai >--- > makedumpfile.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/makedumpfile.c b/makedumpfile.c >index d168dfd..6164468 100644 >--- a/makedumpfile.c >+++ b/makedumpfile.c >@@ -1365,7 +1365,7 @@ open_dump_bitmap(void) > > /* Unnecessary to open */ > if (!info->working_dir && !info->flag_reassemble && !info->flag_refiltering >- && !info->flag_sadump && !info->flag_mem_usage) >+ && !info->flag_sadump && !info->flag_mem_usage && info->flag_cyclic) > return TRUE; > > tmpname = getenv("TMPDIR"); >-- >2.9.3 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2016-09-21 6:33 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-08-10 12:56 [PATCH 0/3] makedumpfile: fix segfault with -X in XEN environment Martin Wilck 2016-08-10 12:56 ` [PATCH 1/3] open_dump_bitmap: open bitmap file in non-cyclic case Martin Wilck 2016-08-10 12:56 ` [PATCH 2/3] move call to open_dump_bitmap() to after call to initial() Martin Wilck 2016-08-10 12:56 ` [PATCH 3/3] close_dump_bitmap: simplify logic Martin Wilck 2016-08-10 13:08 ` Petr Tesarik 2016-08-10 13:35 ` Martin Wilck 2016-08-16 0:37 ` Atsushi Kumagai 2016-08-16 5:59 ` Petr Tesarik 2016-08-17 7:37 ` Martin Wilck 2016-09-06 8:22 ` [PATCH] Cleanup: Use a negative number for uninitialized file descriptors Petr Tesarik 2016-09-09 6:27 ` Atsushi Kumagai 2016-09-09 7:31 ` Petr Tesarik 2016-09-09 8:11 ` [PATCH v2] " Petr Tesarik 2016-09-12 8:17 ` Atsushi Kumagai 2016-09-14 10:17 ` [PATCH v2 1/3] open_dump_bitmap: open bitmap file in non-cyclic case Martin Wilck 2016-09-14 10:17 ` [PATCH v2 2/3] move call to open_dump_bitmap() to after call to initial() Martin Wilck 2016-09-14 10:17 ` [PATCH v2 3/3] close_dump_bitmap: simplify logic Martin Wilck 2016-09-14 11:50 ` [PATCH v3 " Martin Wilck 2016-09-20 9:43 ` Atsushi Kumagai 2016-09-20 10:31 ` Martin Wilck 2016-09-20 10:33 ` [PATCH v3 1/3] open_dump_bitmap: open bitmap file in non-cyclic case Martin Wilck 2016-09-20 10:33 ` [PATCH v3 2/3] move call to open_dump_bitmap() to after call to initial() Martin Wilck 2016-09-20 10:33 ` [PATCH v3 3/3] close_dump_bitmap: simplify logic Martin Wilck 2016-09-21 6:29 ` [PATCH v3 1/3] open_dump_bitmap: open bitmap file in non-cyclic case Atsushi Kumagai
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.