[PATCH v2] Cleanup: Use a negative number for uninitialized file descriptors

Petr Tesarik ptesarik at suse.cz
Fri Sep 9 01:11:29 PDT 2016


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 at 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




More information about the kexec mailing list