diff mbox

[RFC] coredump: fix incomplete core file created when dump_skip was used last

Message ID 1413932228-27642-2-git-send-email-victor.kamensky@linaro.org
State New
Headers show

Commit Message

vkamensky Oct. 21, 2014, 10:57 p.m. UTC
There are situation when dump_skip is used on last part
of core file with intent of creating zeros. However if it
is not followed by any write (through dump_emit) zeros
will not be written into the file. I.e llseek without
subsequent write does not create any content.

Such issue happened during bigcore.exp gdb testsuite run on
ARM V7 rootfs running on top of ARM V8 kenel (compat mode).
For last vma in elf_core_dump function last 5 pages could
not be mapped and as result dump_skip was called. And resulting
core file was missing 5 last pages and gdb complained about
that.

Solution is to introduce new field into coredump_params struct
that would track whether last helper used did real write or
llseek. If last operation was llseek, move position back to
1 byte and write 1 zero byte. As result all content intended
with last dump_skip will appear in core file as zeros.

Signed-off-by: Victor Kamensky <victor.kamensky@linaro.org>
---
 fs/coredump.c           | 26 ++++++++++++++++++++++++++
 include/linux/binfmts.h |  6 ++++++
 2 files changed, 32 insertions(+)

Comments

Oleg Nesterov Oct. 22, 2014, 4:55 p.m. UTC | #1
On 10/21, Victor Kamensky wrote:
>
> +static int dump_write_last_byte(struct coredump_params *cprm)
> +{
> +	char lastbyte = 0;
> +	struct file *file = cprm->file;
> +
> +	if (file->f_op->llseek && file->f_op->llseek != no_llseek) {
> +		if (dump_interrupted() ||
> +		    file->f_op->llseek(file, -1, SEEK_CUR) < 0)
> +			return 0;
> +		if (!dump_emit(cprm, &lastbyte, 1))
> +			return 0;
> +	}
> +	return 1;
> +}

Perhaps do_truncate(cprm.file->f_path.dentry, ->f_pos) makes more sense?

and unless I missed something cprm->last_op_status can be avoided, we can
simply check f_pos != i_size_read() at the end?

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
diff mbox

Patch

diff --git a/fs/coredump.c b/fs/coredump.c
index a93f7e6..da0000b 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -489,6 +489,27 @@  static int umh_pipe_setup(struct subprocess_info *info, struct cred *new)
 	return err;
 }
 
+/*
+ * Write last byte into core file. It is called if last operation
+ * on core file was llseek through dump_skip, and we need to write
+ * something at the end of the file, so zeros intended to be added
+ * by dump_skip will go into the file.
+ */
+static int dump_write_last_byte(struct coredump_params *cprm)
+{
+	char lastbyte = 0;
+	struct file *file = cprm->file;
+
+	if (file->f_op->llseek && file->f_op->llseek != no_llseek) {
+		if (dump_interrupted() ||
+		    file->f_op->llseek(file, -1, SEEK_CUR) < 0)
+			return 0;
+		if (!dump_emit(cprm, &lastbyte, 1))
+			return 0;
+	}
+	return 1;
+}
+
 void do_coredump(const siginfo_t *siginfo)
 {
 	struct core_state core_state;
@@ -514,6 +535,7 @@  void do_coredump(const siginfo_t *siginfo)
 		 * by any locks.
 		 */
 		.mm_flags = mm->flags,
+		.last_op_status = COREDUMP_LAST_WRITE,
 	};
 
 	audit_core_dumps(siginfo->si_signo);
@@ -664,6 +686,8 @@  void do_coredump(const siginfo_t *siginfo)
 	if (!dump_interrupted()) {
 		file_start_write(cprm.file);
 		core_dumped = binfmt->core_dump(&cprm);
+		if (cprm.last_op_status == COREDUMP_LAST_LLSEEK)
+			dump_write_last_byte(&cprm);
 		file_end_write(cprm.file);
 	}
 	if (ispipe && core_pipe_limit)
@@ -706,6 +730,7 @@  int dump_emit(struct coredump_params *cprm, const void *addr, int nr)
 		cprm->written += n;
 		nr -= n;
 	}
+	cprm->last_op_status = COREDUMP_LAST_WRITE;
 	return 1;
 }
 EXPORT_SYMBOL(dump_emit);
@@ -721,6 +746,7 @@  int dump_skip(struct coredump_params *cprm, size_t nr)
 		    file->f_op->llseek(file, nr, SEEK_CUR) < 0)
 			return 0;
 		cprm->written += nr;
+		cprm->last_op_status = COREDUMP_LAST_LLSEEK;
 		return 1;
 	} else {
 		while (nr > PAGE_SIZE) {
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 61f29e5..5f4ad91 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -53,6 +53,11 @@  struct linux_binprm {
 #define BINPRM_FLAGS_EXECFD_BIT 1
 #define BINPRM_FLAGS_EXECFD (1 << BINPRM_FLAGS_EXECFD_BIT)
 
+/* last coredump operation status */
+
+#define COREDUMP_LAST_WRITE 1
+#define COREDUMP_LAST_LLSEEK 2
+
 /* Function parameter for binfmt->coredump */
 struct coredump_params {
 	const siginfo_t *siginfo;
@@ -61,6 +66,7 @@  struct coredump_params {
 	unsigned long limit;
 	unsigned long mm_flags;
 	loff_t written;
+	int last_op_status;
 };
 
 /*