[PATCH 00/11] coredump: allow to create sparse coredumps on the coredump socket

[PATCH 00/11] coredump: allow to create sparse coredumps on the coredump socket

[Christian Brauner]

A coredump generated via the coredump socket ends up transferring
zeroed data when a mapping contains holes. For a large process that
maps a bunch of data that's wasting a ton of work.

Jacob ran into this and Josef has bitched^wcomplained about this to me
before. I dislike the coredump_filter bit solution in [1] which stops
each PT_LOAD at the last populated page.

The problem is real though. I don't think coredump_filter is where we
need to solve this. That mask says which kinds of memory to include and
it propagates across fork and exec, whereas what is being selected here
is an encoding mechanism.

I also think that the usermodehelper - may it swiftly die - isn't really
salvagable for this and it's not the future anyway. The coredump socket
already has a handshake for stuff like this.

I always had an idea how this would look like but punted on it back
then. So here it is.

A server that raises COREDUMP_HEADER in coredump_ack->mask doesn't get
the coredump as a plain byte stream but as a sequence of frames. Each
one a struct coredump_frame_header followed by what it describes. A data
frame carries its bytes. If a server also raises COREDUMP_SPARSE, zero
frames are sent for unpopulated mappings. They only indicate how many
zero bytes need to be written and to not include data. Reassembling the
frames gives back the same coredump. A debugger and everything else
still see an ordinary core file and nothing outside the coredump server
has to learn anything.

Numbers from the selftest in patch 11, on a kernel built from this
series:

- a process with 128 threads: 1740014 bytes on the socket for a
  coredump of 1075150848 bytes
- a 256MB mapping with one page touched: 170542 bytes on the socket for
  a coredump of 268890112 bytes
- the same 256MB mapping with COREDUMP_HEADER alone: 270993024 bytes on
  the socket, so the framing overhead itself is under one percent

The first one is the interesting case. Almost all of it is thread stacks.
All stacks are 8MB reservations that are nearly all holes. And they are
holes in the middle of the dump rather than at the end.

Link: https://lore.kernel.org/all/20260731171336.2255844-1-jalalonde@meta.com [1]

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Christian Brauner (11):
      selftests/coredump: discard the right amount after the coredump request
      selftests/coredump: collapse the expected request check into the helper
      coredump: pin the protocol struct sizes
      coredump: move the negotiated mask into struct coredump_params
      coredump: deduplicate the to_skip flush
      coredump: add COREDUMP_HEADER to the coredump socket protocol
      coredump: add COREDUMP_SPARSE to the coredump socket protocol
      tools: sync coredump.h header
      coredump: frame the coredump when COREDUMP_HEADER is negotiated
      coredump: describe the holes when COREDUMP_SPARSE is negotiated
      selftests/coredump: test COREDUMP_HEADER and COREDUMP_SPARSE

 fs/coredump.c                                      | 178 +++++++--
 include/linux/coredump.h                           |   7 +
 include/uapi/linux/coredump.h                      |  65 +++-
 tools/include/uapi/linux/coredump.h                |  65 +++-
 .../coredump/coredump_socket_protocol_test.c       | 415 +++++++++++++++++++--
 tools/testing/selftests/coredump/coredump_test.h   |   9 +-
 .../selftests/coredump/coredump_test_helpers.c     | 200 +++++++++-
 7 files changed, 849 insertions(+), 90 deletions(-)
---
base-commit: db2ddb87143519e20a95aa36c60b36107b736a58
change-id: 20260811-work-coredump-sparse-18177d77b014

[PATCH 01/11] selftests/coredump: discard the right amount after the coredump request

[Christian Brauner]

read_coredump_req() gets the leftover wrong twice.

It takes the absolute difference of the two sizes, so a test binary that
knows a larger struct coredump_req than the kernel sends tries to discard
bytes that were never sent. And it hands recv() sizeof(buffer) instead of
the number of bytes it wants. So MSG_WAITALL waits for a whole page.
Either one blocks until the kernel closes the socket. Which it won't
because it is waiting for the coredump ack...

Its benign today because struct coredump_req hasn't grown. But let's fix
it for the future. Compute the leftover as what the kernel sent beyond
what was consumed.

Fixes: 59cd658eaf40 ("selftests/coredump: add coredump server selftests")
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 tools/testing/selftests/coredump/coredump_test_helpers.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.c b/tools/testing/selftests/coredump/coredump_test_helpers.c
index 2a20faf9cb0a..524fa5370593 100644
--- a/tools/testing/selftests/coredump/coredump_test_helpers.c
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.c
@@ -235,10 +235,10 @@ bool read_coredump_req(int fd, struct coredump_req *req)
 	fprintf(stderr, "Read coredump request with size %u and mask 0x%llx\n",
 		req->size, (unsigned long long)req->mask);
 
-	if (user_size > kernel_size)
-		remaining_size = user_size - kernel_size;
-	else
+	if (kernel_size > user_size)
 		remaining_size = kernel_size - user_size;
+	else
+		remaining_size = 0;
 
 	if (PAGE_SIZE <= remaining_size)
 		return false;
@@ -250,7 +250,7 @@ bool read_coredump_req(int fd, struct coredump_req *req)
 	if (remaining_size) {
 		char buffer[PAGE_SIZE];
 
-		ret = recv(fd, buffer, sizeof(buffer), MSG_WAITALL);
+		ret = recv(fd, buffer, remaining_size, MSG_WAITALL);
 		if (ret != remaining_size)
 			return false;
 		fprintf(stderr, "Discarded %zu bytes of data after coredump request\n", remaining_size);

-- 
2.53.0

[PATCH 02/11] selftests/coredump: collapse the expected request check into the helper

[Christian Brauner]

All nine callers spell the expected flags. So every new feature bit the
kernel learns has to be cargo culted.

The callers also all pass COREDUMP_ACK_SIZE_VER0 as the minimum request
size although what is being validated is coredump_req->size. And
read_coredump_req() makes the same mixup twice more.

Clean this all up.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 .../coredump/coredump_socket_protocol_test.c       | 36 ++++++----------------
 tools/testing/selftests/coredump/coredump_test.h   |  3 +-
 .../selftests/coredump/coredump_test_helpers.c     | 35 +++++++++++++--------
 3 files changed, 32 insertions(+), 42 deletions(-)

diff --git a/tools/testing/selftests/coredump/coredump_socket_protocol_test.c b/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
index d9fa6239b5a9..60a357e628eb 100644
--- a/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
+++ b/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
@@ -151,9 +151,7 @@ TEST_F(coredump, socket_request_kernel)
 			goto out;
 		}
 
-		if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
-					COREDUMP_KERNEL | COREDUMP_USERSPACE |
-					COREDUMP_REJECT | COREDUMP_WAIT)) {
+		if (!check_coredump_req(&req)) {
 			fprintf(stderr, "socket_request_kernel: check_coredump_req failed\n");
 			goto out;
 		}
@@ -301,9 +299,7 @@ TEST_F(coredump, socket_request_userspace)
 			goto out;
 		}
 
-		if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
-					COREDUMP_KERNEL | COREDUMP_USERSPACE |
-					COREDUMP_REJECT | COREDUMP_WAIT)) {
+		if (!check_coredump_req(&req)) {
 			fprintf(stderr, "socket_request_userspace: check_coredump_req failed\n");
 			goto out;
 		}
@@ -441,9 +437,7 @@ TEST_F(coredump, socket_request_reject)
 			goto out;
 		}
 
-		if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
-					COREDUMP_KERNEL | COREDUMP_USERSPACE |
-					COREDUMP_REJECT | COREDUMP_WAIT)) {
+		if (!check_coredump_req(&req)) {
 			fprintf(stderr, "socket_request_reject: check_coredump_req failed\n");
 			goto out;
 		}
@@ -581,9 +575,7 @@ TEST_F(coredump, socket_request_invalid_flag_combination)
 			goto out;
 		}
 
-		if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
-					COREDUMP_KERNEL | COREDUMP_USERSPACE |
-					COREDUMP_REJECT | COREDUMP_WAIT)) {
+		if (!check_coredump_req(&req)) {
 			fprintf(stderr, "socket_request_invalid_flag_combination: check_coredump_req failed\n");
 			goto out;
 		}
@@ -702,9 +694,7 @@ TEST_F(coredump, socket_request_unknown_flag)
 			goto out;
 		}
 
-		if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
-					COREDUMP_KERNEL | COREDUMP_USERSPACE |
-					COREDUMP_REJECT | COREDUMP_WAIT)) {
+		if (!check_coredump_req(&req)) {
 			fprintf(stderr, "socket_request_unknown_flag: check_coredump_req failed\n");
 			goto out;
 		}
@@ -822,9 +812,7 @@ TEST_F(coredump, socket_request_invalid_size_small)
 			goto out;
 		}
 
-		if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
-					COREDUMP_KERNEL | COREDUMP_USERSPACE |
-					COREDUMP_REJECT | COREDUMP_WAIT)) {
+		if (!check_coredump_req(&req)) {
 			fprintf(stderr, "socket_request_invalid_size_small: check_coredump_req failed\n");
 			goto out;
 		}
@@ -944,9 +932,7 @@ TEST_F(coredump, socket_request_invalid_size_large)
 			goto out;
 		}
 
-		if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
-					COREDUMP_KERNEL | COREDUMP_USERSPACE |
-					COREDUMP_REJECT | COREDUMP_WAIT)) {
+		if (!check_coredump_req(&req)) {
 			fprintf(stderr, "socket_request_invalid_size_large: check_coredump_req failed\n");
 			goto out;
 		}
@@ -1355,9 +1341,7 @@ TEST_F_TIMEOUT(coredump, socket_multiple_crashing_coredumps, 500)
 				goto out;
 			}
 
-			if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
-						COREDUMP_KERNEL | COREDUMP_USERSPACE |
-						COREDUMP_REJECT | COREDUMP_WAIT)) {
+			if (!check_coredump_req(&req)) {
 				fprintf(stderr, "check_coredump_req failed for fd %d\n", fd_coredump);
 				goto out;
 			}
@@ -1509,9 +1493,7 @@ TEST_F_TIMEOUT(coredump, socket_multiple_crashing_coredumps_epoll_workers, 500)
 				fprintf(stderr, "socket_multiple_crashing_coredumps_epoll_workers: read_coredump_req failed\n");
 				goto out;
 			}
-			if (!check_coredump_req(&req, COREDUMP_ACK_SIZE_VER0,
-						COREDUMP_KERNEL | COREDUMP_USERSPACE |
-						COREDUMP_REJECT | COREDUMP_WAIT)) {
+			if (!check_coredump_req(&req)) {
 				fprintf(stderr, "socket_multiple_crashing_coredumps_epoll_workers: check_coredump_req failed\n");
 				goto out;
 			}
diff --git a/tools/testing/selftests/coredump/coredump_test.h b/tools/testing/selftests/coredump/coredump_test.h
index ed47f01fa53c..a02809145e2d 100644
--- a/tools/testing/selftests/coredump/coredump_test.h
+++ b/tools/testing/selftests/coredump/coredump_test.h
@@ -51,8 +51,7 @@ bool read_marker(int fd, enum coredump_mark mark);
 bool read_coredump_req(int fd, struct coredump_req *req);
 bool send_coredump_ack(int fd, const struct coredump_req *req,
 		       __u64 mask, size_t size_ack);
-bool check_coredump_req(const struct coredump_req *req, size_t min_size,
-			__u64 required_mask);
+bool check_coredump_req(const struct coredump_req *req);
 int open_coredump_tmpfile(int fd_tmpfs_detached);
 void process_coredump_worker(int fd_coredump, int fd_peer_pidfd, int fd_core_file);
 
diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.c b/tools/testing/selftests/coredump/coredump_test_helpers.c
index 524fa5370593..d32d96436779 100644
--- a/tools/testing/selftests/coredump/coredump_test_helpers.c
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.c
@@ -200,7 +200,7 @@ bool read_marker(int fd, enum coredump_mark mark)
 bool read_coredump_req(int fd, struct coredump_req *req)
 {
 	ssize_t ret;
-	size_t field_size, user_size, ack_size, kernel_size, remaining_size;
+	size_t field_size, user_size, known, kernel_size, remaining_size;
 
 	memset(req, 0, sizeof(*req));
 	field_size = sizeof(req->size);
@@ -214,9 +214,9 @@ bool read_coredump_req(int fd, struct coredump_req *req)
 	}
 	kernel_size = req->size;
 
-	if (kernel_size < COREDUMP_ACK_SIZE_VER0) {
+	if (kernel_size < COREDUMP_REQ_SIZE_VER0) {
 		fprintf(stderr, "read_coredump_req: kernel_size %zu < min %d\n",
-			kernel_size, COREDUMP_ACK_SIZE_VER0);
+			kernel_size, COREDUMP_REQ_SIZE_VER0);
 		return false;
 	}
 	if (kernel_size >= PAGE_SIZE) {
@@ -225,11 +225,11 @@ bool read_coredump_req(int fd, struct coredump_req *req)
 		return false;
 	}
 
-	/* Use the minimum of user and kernel size to read the full request. */
+	/* Consume as much of the request as we know about. */
 	user_size = sizeof(struct coredump_req);
-	ack_size = user_size < kernel_size ? user_size : kernel_size;
-	ret = recv(fd, req, ack_size, MSG_WAITALL);
-	if (ret != ack_size)
+	known = user_size < kernel_size ? user_size : kernel_size;
+	ret = recv(fd, req, known, MSG_WAITALL);
+	if (ret != known)
 		return false;
 
 	fprintf(stderr, "Read coredump request with size %u and mask 0x%llx\n",
@@ -287,15 +287,24 @@ bool send_coredump_ack(int fd, const struct coredump_req *req,
 	return true;
 }
 
-bool check_coredump_req(const struct coredump_req *req, size_t min_size,
-			__u64 required_mask)
+/* Every option the kernel is expected to advertise in coredump_req->mask. */
+#define COREDUMP_REQ_MASK_ALL					\
+	(COREDUMP_KERNEL | COREDUMP_USERSPACE |			\
+	 COREDUMP_REJECT | COREDUMP_WAIT)
+
+bool check_coredump_req(const struct coredump_req *req)
 {
-	if (req->size < min_size)
-		return false;
-	if ((req->mask & required_mask) != required_mask)
+	if (req->size < COREDUMP_REQ_SIZE_VER0) {
+		fprintf(stderr, "%s: size %u below minimum %d\n",
+			__func__, req->size, COREDUMP_REQ_SIZE_VER0);
 		return false;
-	if (req->mask & ~required_mask)
+	}
+	if (req->mask != COREDUMP_REQ_MASK_ALL) {
+		fprintf(stderr, "%s: mask 0x%llx, expected 0x%llx\n",
+			__func__, (unsigned long long)req->mask,
+			(unsigned long long)COREDUMP_REQ_MASK_ALL);
 		return false;
+	}
 	return true;
 }
 

-- 
2.53.0

[PATCH 03/11] coredump: pin the protocol struct sizes

[Christian Brauner]

COREDUMP_REQ_SIZE_VER0 and COREDUMP_ACK_SIZE_VER0 define the initial
struct sizes.

Assert that both published sizes still match their structs, next to the
assert the coredump marks already have. While at it fix the "currently
know" typo in the doc.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/coredump.c                 | 2 ++
 include/uapi/linux/coredump.h | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index e68a76ff92a3..235b54484107 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -752,6 +752,8 @@ static inline bool coredump_sock_send(struct file *file, struct coredump_req *re
 	return ret == sizeof(*req);
 }
 
+static_assert(sizeof(struct coredump_req) == COREDUMP_REQ_SIZE_VER0);
+static_assert(sizeof(struct coredump_ack) == COREDUMP_ACK_SIZE_VER0);
 static_assert(sizeof(enum coredump_mark) == sizeof(__u32));
 
 static inline bool coredump_sock_mark(struct file *file, enum coredump_mark mark)
diff --git a/include/uapi/linux/coredump.h b/include/uapi/linux/coredump.h
index dc3789b78af0..662e0468da6e 100644
--- a/include/uapi/linux/coredump.h
+++ b/include/uapi/linux/coredump.h
@@ -30,11 +30,11 @@ enum {
  * member is set to the size of struct coredump_req and provides a hint
  * to userspace how much data can be read. Userspace may use MSG_PEEK to
  * peek the size of struct coredump_req and then choose to consume it in
- * one go. Userspace may also simply read a COREDUMP_ACK_SIZE_VER0
+ * one go. Userspace may also simply read a COREDUMP_REQ_SIZE_VER0
  * request. If the size the kernel sends is larger userspace simply
  * discards any remaining data.
  *
- * The coredump_req->mask member is set to the currently know features.
+ * The coredump_req->mask member is set to the currently known features.
  * Userspace may only set coredump_ack->mask to the bits raised by the
  * kernel in coredump_req->mask.
  *

-- 
2.53.0

[PATCH 04/11] coredump: move the negotiated mask into struct coredump_params

[Christian Brauner]

The coredump server negotiates a set of COREDUMP_* options with the
kernel. The core dump path cannot see them though.

Move the mask into struct coredump_params so the negotiated options are
available to the core dump path.

No functional change.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/coredump.c            | 15 +++++++--------
 include/linux/coredump.h |  2 ++
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index 235b54484107..e5463e3b3f4b 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -100,7 +100,6 @@ struct core_name {
 	unsigned int core_pipe_limit;
 	bool core_dumped;
 	enum coredump_type_t core_type;
-	u64 mask;
 };
 
 static int expand_corename(struct core_name *cn, int size)
@@ -245,9 +244,9 @@ static bool coredump_parse(struct core_name *cn, struct coredump_params *cprm,
 	int pid_in_pattern = 0;
 	int err = 0;
 
-	cn->mask = COREDUMP_KERNEL;
+	cprm->mask = COREDUMP_KERNEL;
 	if (core_pipe_limit)
-		cn->mask |= COREDUMP_WAIT;
+		cprm->mask |= COREDUMP_WAIT;
 	cn->used = 0;
 	cn->corename = NULL;
 	cn->core_pipe_limit = 0;
@@ -853,7 +852,7 @@ static bool coredump_sock_request(struct core_name *cn, struct coredump_params *
 		return false;
 	}
 
-	cn->mask = ack.mask;
+	cprm->mask = ack.mask;
 	return coredump_sock_mark(cprm->file, COREDUMP_MARK_REQACK);
 }
 
@@ -1122,7 +1121,7 @@ static void do_coredump(struct core_name *cn, struct coredump_params *cprm,
 	}
 
 	/* Don't even generate the coredump. */
-	if (cn->mask & COREDUMP_REJECT)
+	if (cprm->mask & COREDUMP_REJECT)
 		return;
 
 	/* get us an unshared descriptor table; almost always a no-op */
@@ -1130,13 +1129,13 @@ static void do_coredump(struct core_name *cn, struct coredump_params *cprm,
 	if (unshare_files())
 		return;
 
-	if ((cn->mask & COREDUMP_KERNEL) && !coredump_write(cn, cprm, binfmt))
+	if ((cprm->mask & COREDUMP_KERNEL) && !coredump_write(cn, cprm, binfmt))
 		return;
 
 	coredump_sock_shutdown(cprm->file);
 
 	/* Let the parent know that a coredump was generated. */
-	if (cn->mask & COREDUMP_USERSPACE)
+	if (cprm->mask & COREDUMP_USERSPACE)
 		cn->core_dumped = true;
 
 	/*
@@ -1144,7 +1143,7 @@ static void do_coredump(struct core_name *cn, struct coredump_params *cprm,
 	 * or usermodehelper to finish before exiting so it can e.g.,
 	 * inspect /proc/<pid>.
 	 */
-	if (cn->mask & COREDUMP_WAIT) {
+	if (cprm->mask & COREDUMP_WAIT) {
 		switch (cn->core_type) {
 		case COREDUMP_PIPE:
 			wait_for_dump_helpers(cprm->file);
diff --git a/include/linux/coredump.h b/include/linux/coredump.h
index 7b38ee2e7913..dc7a05b1bb0a 100644
--- a/include/linux/coredump.h
+++ b/include/linux/coredump.h
@@ -26,6 +26,8 @@ struct coredump_params {
 	/* Snapshot of dumpable at dump start. */
 	enum task_dumpable dumpable;
 	int cpu;
+	/* COREDUMP_* options negotiated with the coredump server. */
+	u64 mask;
 	loff_t written;
 	loff_t pos;
 	loff_t to_skip;

-- 
2.53.0

[PATCH 05/11] coredump: deduplicate the to_skip flush

[Christian Brauner]

dump_emit() and dump_emit_page() open-code the same flush of the
accumulated cprm->to_skip. Move it into a helper.

No functional change.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/coredump.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index e5463e3b3f4b..6de18bc49925 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -1249,13 +1249,21 @@ static int __dump_skip(struct coredump_params *cprm, size_t nr)
 	return __dump_emit(cprm, zeroes, nr);
 }
 
-int dump_emit(struct coredump_params *cprm, const void *addr, int nr)
+/* Flush the accumulated hole before writing data. */
+static int dump_flush(struct coredump_params *cprm)
 {
 	if (cprm->to_skip) {
 		if (!__dump_skip(cprm, cprm->to_skip))
 			return 0;
 		cprm->to_skip = 0;
 	}
+	return 1;
+}
+
+int dump_emit(struct coredump_params *cprm, const void *addr, int nr)
+{
+	if (!dump_flush(cprm))
+		return 0;
 	return __dump_emit(cprm, addr, nr);
 }
 EXPORT_SYMBOL(dump_emit);
@@ -1284,11 +1292,8 @@ static int dump_emit_page(struct coredump_params *cprm, struct page *page)
 	if (!page)
 		return 0;
 
-	if (cprm->to_skip) {
-		if (!__dump_skip(cprm, cprm->to_skip))
-			return 0;
-		cprm->to_skip = 0;
-	}
+	if (!dump_flush(cprm))
+		return 0;
 	if (cprm->written + PAGE_SIZE > cprm->limit)
 		return 0;
 	if (dump_interrupted())

-- 
2.53.0

[PATCH 06/11] coredump: add COREDUMP_HEADER to the coredump socket protocol

[Christian Brauner]

A coredump sent over a socket is a plain byte stream. The kernel knows
things about the bytes it is sending that a server might care about. For
example, it knows where the unpopulated parts of a mapping are. We can't
communicate this to userspace currently though.

Add a COREDUMP_HEADER feature bit and a struct coredump_frame_header.
Userspace can negotiate that feature. Instead of a byte stream it gets a
header plus data. Reassembling the frames yields the same coredump that
would have been sent without them. The next patch will introduce a first
feature.

The frame itself is also versioned and thus extensible with the same
protocol as the ack-req sync.

A kernel that doesn't know the bit doesn't raise it in
coredump_req->mask and a server may not raise a bit the kernel didn't
advertise. A server that doesn't know the bit never raises it and gets a
plain byte stream.

This just adds the infrastructure.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 include/uapi/linux/coredump.h | 52 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/include/uapi/linux/coredump.h b/include/uapi/linux/coredump.h
index 662e0468da6e..5252480d3eec 100644
--- a/include/uapi/linux/coredump.h
+++ b/include/uapi/linux/coredump.h
@@ -11,12 +11,16 @@
  * @COREDUMP_USERSPACE: userspace writes coredump
  * @COREDUMP_REJECT: don't generate coredump
  * @COREDUMP_WAIT: wait for coredump server
+ * @COREDUMP_HEADER: send the coredump as a sequence of frames instead of
+ *                   as a plain byte stream, see struct coredump_frame_header;
+ *                   requires COREDUMP_KERNEL
  */
 enum {
 	COREDUMP_KERNEL		= (1ULL << 0),
 	COREDUMP_USERSPACE	= (1ULL << 1),
 	COREDUMP_REJECT		= (1ULL << 2),
 	COREDUMP_WAIT		= (1ULL << 3),
+	COREDUMP_HEADER		= (1ULL << 4),
 };
 
 /**
@@ -101,4 +105,52 @@ enum coredump_mark {
 	__COREDUMP_MARK_MAX		= (1U << 31),
 };
 
+/**
+ * enum coredump_frame_type - Type of a coredump frame
+ *
+ * @COREDUMP_FRAME_DATA: the header is followed by ->len bytes of data
+ * @__COREDUMP_FRAME_MAX: the maximum coredump frame type value
+ */
+enum coredump_frame_type {
+	COREDUMP_FRAME_DATA	= 0U,
+	__COREDUMP_FRAME_MAX	= (1U << 31),
+};
+
+/**
+ * struct coredump_frame_header - header of a coredump frame
+ * @size: size of struct coredump_frame_header
+ * @type: one of enum coredump_frame_type
+ * @flags: modifiers for this frame
+ * @offset: offset of this frame in the coredump
+ * @len: length of this frame in the coredump
+ *
+ * If the coredump server raises COREDUMP_HEADER in coredump_ack->mask the
+ * kernel doesn't send the coredump as a plain byte stream. It sends a
+ * sequence of frames instead. A struct coredump_frame_header is followed by
+ * @len bytes of actual coredump data.
+ *
+ * The @size member is set to the size of struct coredump_frame_header the
+ * kernel knows and lets the header grow later. It comes first so it can be
+ * peeked. Userspace must consume @size bytes and discard anything beyond
+ * what it knows. The same way it deals with struct coredump_req. It must
+ * refuse a @size smaller than COREDUMP_FRAME_HEADER_SIZE_VER0.
+ *
+ * The @flags member carries modifiers that change how the frame is to be
+ * interpreted. No flags are defined yet. Userspace must refuse a frame
+ * carrying a flag it doesn't know.
+ *
+ * COREDUMP_HEADER must be combined with COREDUMP_KERNEL.
+ */
+struct coredump_frame_header {
+	__u32 size;
+	__u32 type;
+	__u64 flags;
+	__u64 offset;
+	__u64 len;
+};
+
+enum {
+	COREDUMP_FRAME_HEADER_SIZE_VER0 = 32U, /* size of first published struct */
+};
+
 #endif /* _UAPI_LINUX_COREDUMP_H */

-- 
2.53.0

[PATCH 07/11] coredump: add COREDUMP_SPARSE to the coredump socket protocol

[Christian Brauner]

A coredump with a lot of unpopulated mappings sends endless amounts of
zero data to userspace. This is nonsensical. While __dump_skip() can
seek over them when the target is a regular file a socket cannot do
this. COREDUMP_HEADER framed the zeroes but it didn't get rid of them.

Add a COREDUMP_SPARSE feature bit and a COREDUMP_FRAME_ZERO frame type.
A zero frame is a bare header that tells userspace how many zero bytes
where skipped.

So a hole crosses the socket as one header no matter how long it is. The
coredump server can recreate this sparsely. Zero frames only exist
inside a framed stream. So COREDUMP_SPARSE requires COREDUMP_HEADER.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 include/uapi/linux/coredump.h | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/coredump.h b/include/uapi/linux/coredump.h
index 5252480d3eec..312bafabb467 100644
--- a/include/uapi/linux/coredump.h
+++ b/include/uapi/linux/coredump.h
@@ -14,6 +14,8 @@
  * @COREDUMP_HEADER: send the coredump as a sequence of frames instead of
  *                   as a plain byte stream, see struct coredump_frame_header;
  *                   requires COREDUMP_KERNEL
+ * @COREDUMP_SPARSE: describe the holes in the coredump as zero frames
+ *                   instead of transferring them; requires COREDUMP_HEADER
  */
 enum {
 	COREDUMP_KERNEL		= (1ULL << 0),
@@ -21,6 +23,7 @@ enum {
 	COREDUMP_REJECT		= (1ULL << 2),
 	COREDUMP_WAIT		= (1ULL << 3),
 	COREDUMP_HEADER		= (1ULL << 4),
+	COREDUMP_SPARSE		= (1ULL << 5),
 };
 
 /**
@@ -109,10 +112,13 @@ enum coredump_mark {
  * enum coredump_frame_type - Type of a coredump frame
  *
  * @COREDUMP_FRAME_DATA: the header is followed by ->len bytes of data
+ * @COREDUMP_FRAME_ZERO: the header stands for ->len zero bytes and is not
+ *                       followed by any data
  * @__COREDUMP_FRAME_MAX: the maximum coredump frame type value
  */
 enum coredump_frame_type {
 	COREDUMP_FRAME_DATA	= 0U,
+	COREDUMP_FRAME_ZERO	= 1U,
 	__COREDUMP_FRAME_MAX	= (1U << 31),
 };
 
@@ -126,8 +132,10 @@ enum coredump_frame_type {
  *
  * If the coredump server raises COREDUMP_HEADER in coredump_ack->mask the
  * kernel doesn't send the coredump as a plain byte stream. It sends a
- * sequence of frames instead. A struct coredump_frame_header is followed by
- * @len bytes of actual coredump data.
+ * sequence of frames instead. A COREDUMP_FRAME_DATA frame is followed by
+ * @len bytes of actual coredump data. A COREDUMP_FRAME_ZERO frame is
+ * followed by nothing and stands for @len zero bytes. A server that didn't
+ * raise COREDUMP_SPARSE never sees a zero frame.
  *
  * The @size member is set to the size of struct coredump_frame_header the
  * kernel knows and lets the header grow later. It comes first so it can be
@@ -139,7 +147,8 @@ enum coredump_frame_type {
  * interpreted. No flags are defined yet. Userspace must refuse a frame
  * carrying a flag it doesn't know.
  *
- * COREDUMP_HEADER must be combined with COREDUMP_KERNEL.
+ * COREDUMP_HEADER must be combined with COREDUMP_KERNEL, and
+ * COREDUMP_SPARSE with COREDUMP_HEADER.
  */
 struct coredump_frame_header {
 	__u32 size;

-- 
2.53.0

[PATCH 08/11] tools: sync coredump.h header

[Christian Brauner]

Sync the headers for the selftests.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 tools/include/uapi/linux/coredump.h | 65 +++++++++++++++++++++++++++++++++++--
 1 file changed, 63 insertions(+), 2 deletions(-)

diff --git a/tools/include/uapi/linux/coredump.h b/tools/include/uapi/linux/coredump.h
index dc3789b78af0..312bafabb467 100644
--- a/tools/include/uapi/linux/coredump.h
+++ b/tools/include/uapi/linux/coredump.h
@@ -11,12 +11,19 @@
  * @COREDUMP_USERSPACE: userspace writes coredump
  * @COREDUMP_REJECT: don't generate coredump
  * @COREDUMP_WAIT: wait for coredump server
+ * @COREDUMP_HEADER: send the coredump as a sequence of frames instead of
+ *                   as a plain byte stream, see struct coredump_frame_header;
+ *                   requires COREDUMP_KERNEL
+ * @COREDUMP_SPARSE: describe the holes in the coredump as zero frames
+ *                   instead of transferring them; requires COREDUMP_HEADER
  */
 enum {
 	COREDUMP_KERNEL		= (1ULL << 0),
 	COREDUMP_USERSPACE	= (1ULL << 1),
 	COREDUMP_REJECT		= (1ULL << 2),
 	COREDUMP_WAIT		= (1ULL << 3),
+	COREDUMP_HEADER		= (1ULL << 4),
+	COREDUMP_SPARSE		= (1ULL << 5),
 };
 
 /**
@@ -30,11 +37,11 @@ enum {
  * member is set to the size of struct coredump_req and provides a hint
  * to userspace how much data can be read. Userspace may use MSG_PEEK to
  * peek the size of struct coredump_req and then choose to consume it in
- * one go. Userspace may also simply read a COREDUMP_ACK_SIZE_VER0
+ * one go. Userspace may also simply read a COREDUMP_REQ_SIZE_VER0
  * request. If the size the kernel sends is larger userspace simply
  * discards any remaining data.
  *
- * The coredump_req->mask member is set to the currently know features.
+ * The coredump_req->mask member is set to the currently known features.
  * Userspace may only set coredump_ack->mask to the bits raised by the
  * kernel in coredump_req->mask.
  *
@@ -101,4 +108,58 @@ enum coredump_mark {
 	__COREDUMP_MARK_MAX		= (1U << 31),
 };
 
+/**
+ * enum coredump_frame_type - Type of a coredump frame
+ *
+ * @COREDUMP_FRAME_DATA: the header is followed by ->len bytes of data
+ * @COREDUMP_FRAME_ZERO: the header stands for ->len zero bytes and is not
+ *                       followed by any data
+ * @__COREDUMP_FRAME_MAX: the maximum coredump frame type value
+ */
+enum coredump_frame_type {
+	COREDUMP_FRAME_DATA	= 0U,
+	COREDUMP_FRAME_ZERO	= 1U,
+	__COREDUMP_FRAME_MAX	= (1U << 31),
+};
+
+/**
+ * struct coredump_frame_header - header of a coredump frame
+ * @size: size of struct coredump_frame_header
+ * @type: one of enum coredump_frame_type
+ * @flags: modifiers for this frame
+ * @offset: offset of this frame in the coredump
+ * @len: length of this frame in the coredump
+ *
+ * If the coredump server raises COREDUMP_HEADER in coredump_ack->mask the
+ * kernel doesn't send the coredump as a plain byte stream. It sends a
+ * sequence of frames instead. A COREDUMP_FRAME_DATA frame is followed by
+ * @len bytes of actual coredump data. A COREDUMP_FRAME_ZERO frame is
+ * followed by nothing and stands for @len zero bytes. A server that didn't
+ * raise COREDUMP_SPARSE never sees a zero frame.
+ *
+ * The @size member is set to the size of struct coredump_frame_header the
+ * kernel knows and lets the header grow later. It comes first so it can be
+ * peeked. Userspace must consume @size bytes and discard anything beyond
+ * what it knows. The same way it deals with struct coredump_req. It must
+ * refuse a @size smaller than COREDUMP_FRAME_HEADER_SIZE_VER0.
+ *
+ * The @flags member carries modifiers that change how the frame is to be
+ * interpreted. No flags are defined yet. Userspace must refuse a frame
+ * carrying a flag it doesn't know.
+ *
+ * COREDUMP_HEADER must be combined with COREDUMP_KERNEL, and
+ * COREDUMP_SPARSE with COREDUMP_HEADER.
+ */
+struct coredump_frame_header {
+	__u32 size;
+	__u32 type;
+	__u64 flags;
+	__u64 offset;
+	__u64 len;
+};
+
+enum {
+	COREDUMP_FRAME_HEADER_SIZE_VER0 = 32U, /* size of first published struct */
+};
+
 #endif /* _UAPI_LINUX_COREDUMP_H */

-- 
2.53.0

[PATCH 09/11] coredump: frame the coredump when COREDUMP_HEADER is negotiated

[Christian Brauner]

When the coredump server raises COREDUMP_HEADER every write to the
socket is prefixed with a struct coredump_frame_header in front of it
describing what follows. A header and the bytes it describes go out in
one iovec.

Both emitters write through one helper that either writes the whole
iov_iter or fails. So a single place advances the file position.
cprm->pos stays the offset in the coredump and keeps ignoring the
framing overhead. dump_skip_to() and dump_align() compute from
it. cprm->written counts what was handed to the file and so picks the
headers up. The two are the same number when the coredump isn't framed.

A hole is flushed through __dump_emit() like before. So zeroes still are
sent on the socket as actual data frames. Making holes cheap is
COREDUMP_SPARSE's job.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/coredump.c                                      | 114 ++++++++++++++++-----
 include/linux/coredump.h                           |   5 +
 .../selftests/coredump/coredump_test_helpers.c     |   2 +-
 3 files changed, 94 insertions(+), 27 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index 6de18bc49925..364c89c5f82a 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -51,7 +51,6 @@
 #include <net/sock.h>
 #include <uapi/linux/pidfd.h>
 #include <uapi/linux/un.h>
-#include <uapi/linux/coredump.h>
 
 #include <linux/uaccess.h>
 #include <asm/mmu_context.h>
@@ -753,6 +752,7 @@ static inline bool coredump_sock_send(struct file *file, struct coredump_req *re
 
 static_assert(sizeof(struct coredump_req) == COREDUMP_REQ_SIZE_VER0);
 static_assert(sizeof(struct coredump_ack) == COREDUMP_ACK_SIZE_VER0);
+static_assert(sizeof(struct coredump_frame_header) == COREDUMP_FRAME_HEADER_SIZE_VER0);
 static_assert(sizeof(enum coredump_mark) == sizeof(__u32));
 
 static inline bool coredump_sock_mark(struct file *file, enum coredump_mark mark)
@@ -798,7 +798,8 @@ static bool coredump_sock_request(struct core_name *cn, struct coredump_params *
 	struct coredump_req req = {
 		.size		= sizeof(struct coredump_req),
 		.mask		= COREDUMP_KERNEL | COREDUMP_USERSPACE |
-				  COREDUMP_REJECT | COREDUMP_WAIT,
+				  COREDUMP_REJECT | COREDUMP_WAIT |
+				  COREDUMP_HEADER,
 		.size_ack	= sizeof(struct coredump_ack),
 	};
 	struct coredump_ack ack = {};
@@ -847,11 +848,24 @@ static bool coredump_sock_request(struct core_name *cn, struct coredump_params *
 		return false;
 	}
 
+	/* Framing only applies to a coredump the kernel writes. */
+	if ((ack.mask & COREDUMP_HEADER) && !(ack.mask & COREDUMP_KERNEL)) {
+		coredump_sock_mark(cprm->file, COREDUMP_MARK_CONFLICTING);
+		return false;
+	}
+
 	if (ack.spare) {
 		coredump_sock_mark(cprm->file, COREDUMP_MARK_UNSUPPORTED);
 		return false;
 	}
 
+	/* Frame header scratch; a bvec can't point at the stack. */
+	if (ack.mask & COREDUMP_HEADER) {
+		cprm->frame = kmalloc_obj(*cprm->frame);
+		if (!cprm->frame)
+			return false;
+	}
+
 	cprm->mask = ack.mask;
 	return coredump_sock_mark(cprm->file, COREDUMP_MARK_REQACK);
 }
@@ -1053,9 +1067,10 @@ static bool coredump_write(struct core_name *cn,
 	cn->core_dumped = binfmt->core_dump(cprm);
 	/*
 	 * Ensures that file size is big enough to contain the current
-	 * file postion. This prevents gdb from complaining about
+	 * file position. This prevents gdb from complaining about
 	 * a truncated file if the last "write" to the file was
-	 * dump_skip.
+	 * dump_skip. A framed coredump relies on it too: the flush
+	 * emits the frames that cover a trailing hole.
 	 */
 	if (cprm->to_skip) {
 		cprm->to_skip--;
@@ -1075,6 +1090,7 @@ static void coredump_cleanup(struct core_name *cn, struct coredump_params *cprm)
 		atomic_dec(&core_pipe_count);
 	}
 	kfree(cn->corename);
+	kfree(cprm->frame);
 	coredump_finish(cn->core_dumped);
 }
 
@@ -1208,24 +1224,72 @@ void vfs_coredump(const kernel_siginfo_t *siginfo)
  * do on a core-file: use only these functions to write out all the
  * necessary info.
  */
-static int __dump_emit(struct coredump_params *cprm, const void *addr, int nr)
+static bool dump_framed(const struct coredump_params *cprm)
+{
+	return cprm->mask & COREDUMP_HEADER;
+}
+
+/* Describe the next @len bytes of the coredump. Returns the header size. */
+static size_t dump_frame_init(struct coredump_params *cprm,
+			      enum coredump_frame_type type, u64 len)
+{
+	if (!dump_framed(cprm))
+		return 0;
+
+	*cprm->frame = (struct coredump_frame_header) {
+		.size	= sizeof(*cprm->frame),
+		.type	= type,
+		.offset	= cprm->pos,
+		.len	= len,
+	};
+
+	return sizeof(*cprm->frame);
+}
+
+/* Write @iter whole or fail. @len is what it advances the coredump by. */
+static bool dump_write_iter(struct coredump_params *cprm, struct iov_iter *iter,
+			    size_t len)
 {
 	struct file *file = cprm->file;
+	size_t count = iov_iter_count(iter);
 	loff_t pos = file->f_pos;
 	ssize_t n;
 
+	n = __kernel_write_iter(file, iter, &pos);
+	if (n < 0 || (size_t)n != count)
+		return false;
+	file->f_pos = pos;
+	cprm->written += count;
+	cprm->pos += len;
+
+	return true;
+}
+
+static int __dump_emit(struct coredump_params *cprm, const void *addr, int nr)
+{
+	struct kvec kvec[2];
+	struct iov_iter iter;
+	unsigned int nseg = 0;
+	size_t hdr;
+
 	if (cprm->written + nr > cprm->limit)
 		return 0;
 	if (dump_interrupted())
 		return 0;
-	n = __kernel_write(file, addr, nr, &pos);
-	if (n != nr)
-		return 0;
-	file->f_pos = pos;
-	cprm->written += n;
-	cprm->pos += n;
 
-	return 1;
+	hdr = dump_frame_init(cprm, COREDUMP_FRAME_DATA, nr);
+	if (hdr) {
+		kvec[nseg].iov_base = cprm->frame;
+		kvec[nseg].iov_len = hdr;
+		nseg++;
+	}
+	kvec[nseg].iov_base = (void *)addr;
+	kvec[nseg].iov_len = nr;
+	nseg++;
+
+	iov_iter_kvec(&iter, ITER_SOURCE, kvec, nseg, hdr + nr);
+
+	return dump_write_iter(cprm, &iter, nr);
 }
 
 static int __dump_skip(struct coredump_params *cprm, size_t nr)
@@ -1283,11 +1347,10 @@ EXPORT_SYMBOL(dump_skip);
 #ifdef CONFIG_ELF_CORE
 static int dump_emit_page(struct coredump_params *cprm, struct page *page)
 {
-	struct bio_vec bvec;
+	struct bio_vec bvec[2];
 	struct iov_iter iter;
-	struct file *file = cprm->file;
-	loff_t pos;
-	ssize_t n;
+	unsigned int nseg = 0;
+	size_t hdr;
 
 	if (!page)
 		return 0;
@@ -1298,17 +1361,16 @@ static int dump_emit_page(struct coredump_params *cprm, struct page *page)
 		return 0;
 	if (dump_interrupted())
 		return 0;
-	pos = file->f_pos;
-	bvec_set_page(&bvec, page, PAGE_SIZE, 0);
-	iov_iter_bvec(&iter, ITER_SOURCE, &bvec, 1, PAGE_SIZE);
-	n = __kernel_write_iter(cprm->file, &iter, &pos);
-	if (n != PAGE_SIZE)
-		return 0;
-	file->f_pos = pos;
-	cprm->written += PAGE_SIZE;
-	cprm->pos += PAGE_SIZE;
 
-	return 1;
+	/* Hand the frame to the same write as the page it describes. */
+	hdr = dump_frame_init(cprm, COREDUMP_FRAME_DATA, PAGE_SIZE);
+	if (hdr)
+		bvec_set_virt(&bvec[nseg++], cprm->frame, hdr);
+	bvec_set_page(&bvec[nseg++], page, PAGE_SIZE, 0);
+
+	iov_iter_bvec(&iter, ITER_SOURCE, bvec, nseg, hdr + PAGE_SIZE);
+
+	return dump_write_iter(cprm, &iter, PAGE_SIZE);
 }
 
 /*
diff --git a/include/linux/coredump.h b/include/linux/coredump.h
index dc7a05b1bb0a..06ccd3046a06 100644
--- a/include/linux/coredump.h
+++ b/include/linux/coredump.h
@@ -6,6 +6,7 @@
 #include <linux/mm.h>
 #include <linux/fs.h>
 #include <linux/sched/coredump.h>
+#include <uapi/linux/coredump.h>
 #include <asm/siginfo.h>
 
 #ifdef CONFIG_COREDUMP
@@ -28,7 +29,11 @@ struct coredump_params {
 	int cpu;
 	/* COREDUMP_* options negotiated with the coredump server. */
 	u64 mask;
+	/* Frame header scratch, NULL unless the coredump is framed. */
+	struct coredump_frame_header *frame;
+	/* Bytes handed to the file, frame headers included. */
 	loff_t written;
+	/* Offset in the coredump, frame headers excluded. */
 	loff_t pos;
 	loff_t to_skip;
 	int vma_count;
diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.c b/tools/testing/selftests/coredump/coredump_test_helpers.c
index d32d96436779..bfe10bc51618 100644
--- a/tools/testing/selftests/coredump/coredump_test_helpers.c
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.c
@@ -290,7 +290,7 @@ bool send_coredump_ack(int fd, const struct coredump_req *req,
 /* Every option the kernel is expected to advertise in coredump_req->mask. */
 #define COREDUMP_REQ_MASK_ALL					\
 	(COREDUMP_KERNEL | COREDUMP_USERSPACE |			\
-	 COREDUMP_REJECT | COREDUMP_WAIT)
+	 COREDUMP_REJECT | COREDUMP_WAIT | COREDUMP_HEADER)
 
 bool check_coredump_req(const struct coredump_req *req)
 {

-- 
2.53.0

[PATCH 10/11] coredump: describe the holes when COREDUMP_SPARSE is negotiated

[Christian Brauner]

Offer COREDUMP_SPARSE in coredump_req->mask now that there is something
behind it. Refuse it without COREDUMP_HEADER. A zero frame cannot exist
outside a framed stream.

A trailing hole is flushed by coredump_write() the way it always was. So
its last byte goes out as a one-byte data frame and the frames cover the
whole coredump.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/coredump.c                                      | 34 +++++++++++++++++++++-
 .../selftests/coredump/coredump_test_helpers.c     |  3 +-
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index 364c89c5f82a..bdf2eae948d0 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -799,7 +799,7 @@ static bool coredump_sock_request(struct core_name *cn, struct coredump_params *
 		.size		= sizeof(struct coredump_req),
 		.mask		= COREDUMP_KERNEL | COREDUMP_USERSPACE |
 				  COREDUMP_REJECT | COREDUMP_WAIT |
-				  COREDUMP_HEADER,
+				  COREDUMP_HEADER | COREDUMP_SPARSE,
 		.size_ack	= sizeof(struct coredump_ack),
 	};
 	struct coredump_ack ack = {};
@@ -854,6 +854,12 @@ static bool coredump_sock_request(struct core_name *cn, struct coredump_params *
 		return false;
 	}
 
+	/* Zero frames only exist inside a framed stream. */
+	if ((ack.mask & COREDUMP_SPARSE) && !(ack.mask & COREDUMP_HEADER)) {
+		coredump_sock_mark(cprm->file, COREDUMP_MARK_CONFLICTING);
+		return false;
+	}
+
 	if (ack.spare) {
 		coredump_sock_mark(cprm->file, COREDUMP_MARK_UNSUPPORTED);
 		return false;
@@ -1229,6 +1235,11 @@ static bool dump_framed(const struct coredump_params *cprm)
 	return cprm->mask & COREDUMP_HEADER;
 }
 
+static bool dump_sparse(const struct coredump_params *cprm)
+{
+	return cprm->mask & COREDUMP_SPARSE;
+}
+
 /* Describe the next @len bytes of the coredump. Returns the header size. */
 static size_t dump_frame_init(struct coredump_params *cprm,
 			      enum coredump_frame_type type, u64 len)
@@ -1292,11 +1303,32 @@ static int __dump_emit(struct coredump_params *cprm, const void *addr, int nr)
 	return dump_write_iter(cprm, &iter, nr);
 }
 
+/* Hand the server the length of the hole instead of the hole itself. */
+static int dump_skip_frame(struct coredump_params *cprm, size_t nr)
+{
+	struct kvec kvec;
+	struct iov_iter iter;
+	size_t hdr;
+
+	if (dump_interrupted())
+		return 0;
+
+	hdr = dump_frame_init(cprm, COREDUMP_FRAME_ZERO, nr);
+	kvec.iov_base = cprm->frame;
+	kvec.iov_len = hdr;
+	iov_iter_kvec(&iter, ITER_SOURCE, &kvec, 1, hdr);
+
+	return dump_write_iter(cprm, &iter, nr);
+}
+
 static int __dump_skip(struct coredump_params *cprm, size_t nr)
 {
 	static char zeroes[PAGE_SIZE];
 	struct file *file = cprm->file;
 
+	if (dump_sparse(cprm))
+		return dump_skip_frame(cprm, nr);
+
 	if (file->f_mode & FMODE_LSEEK) {
 		if (dump_interrupted() || vfs_llseek(file, nr, SEEK_CUR) < 0)
 			return 0;
diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.c b/tools/testing/selftests/coredump/coredump_test_helpers.c
index bfe10bc51618..ee669969605b 100644
--- a/tools/testing/selftests/coredump/coredump_test_helpers.c
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.c
@@ -290,7 +290,8 @@ bool send_coredump_ack(int fd, const struct coredump_req *req,
 /* Every option the kernel is expected to advertise in coredump_req->mask. */
 #define COREDUMP_REQ_MASK_ALL					\
 	(COREDUMP_KERNEL | COREDUMP_USERSPACE |			\
-	 COREDUMP_REJECT | COREDUMP_WAIT | COREDUMP_HEADER)
+	 COREDUMP_REJECT | COREDUMP_WAIT |			\
+	 COREDUMP_HEADER | COREDUMP_SPARSE)
 
 bool check_coredump_req(const struct coredump_req *req)
 {

-- 
2.53.0

[PATCH 11/11] selftests/coredump: test COREDUMP_HEADER and COREDUMP_SPARSE

[Christian Brauner]

Test the new COREDUMP_HEADER and COREDUMP_SPARSE flags.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 .../coredump/coredump_socket_protocol_test.c       | 379 +++++++++++++++++++++
 tools/testing/selftests/coredump/coredump_test.h   |   6 +
 .../selftests/coredump/coredump_test_helpers.c     | 156 ++++++++-
 3 files changed, 540 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/coredump/coredump_socket_protocol_test.c b/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
index 60a357e628eb..ef36e787a529 100644
--- a/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
+++ b/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 
+#include <elf.h>
 #include <sys/stat.h>
 #include <sys/epoll.h>
 #include <sys/socket.h>
@@ -1573,4 +1574,382 @@ TEST_F_TIMEOUT(coredump, socket_multiple_crashing_coredumps_epoll_workers, 500)
 	wait_and_check_coredump_server(pid_coredump_server, _metadata, self);
 }
 
+/*
+ * Reassemble a framed coredump and check that what comes out is an ELF
+ * core file. The frames themselves are validated by recv_coredump_frames().
+ */
+TEST_F(coredump, socket_request_sparse)
+{
+	unsigned char ehdr[EI_NIDENT + sizeof(Elf64_Half)];
+	int fd_core_file, pidfd, status;
+	pid_t pid, pid_coredump_server;
+	struct pidfd_info info = {};
+	int ipc_sockets[2];
+	Elf64_Half e_type;
+	struct stat st;
+	char c;
+
+	ASSERT_EQ(socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets), 0);
+	ASSERT_TRUE(set_core_pattern("@@/tmp/coredump.socket"));
+
+	pid_coredump_server = fork();
+	ASSERT_GE(pid_coredump_server, 0);
+	if (pid_coredump_server == 0) {
+		int fd_server = -1, fd_coredump = -1, fd_peer_pidfd = -1;
+		int fd_file = -1;
+		int exit_code = EXIT_FAILURE;
+		struct coredump_req req = {};
+
+		close(ipc_sockets[0]);
+
+		fd_server = create_and_listen_unix_socket("/tmp/coredump.socket");
+		if (fd_server < 0)
+			goto out;
+
+		if (write_nointr(ipc_sockets[1], "1", 1) < 0)
+			goto out;
+
+		close(ipc_sockets[1]);
+
+		fd_coredump = accept4(fd_server, NULL, NULL, SOCK_CLOEXEC);
+		if (fd_coredump < 0)
+			goto out;
+
+		fd_peer_pidfd = get_peer_pidfd(fd_coredump);
+		if (fd_peer_pidfd < 0)
+			goto out;
+
+		fd_file = creat("/tmp/coredump.file", 0644);
+		if (fd_file < 0)
+			goto out;
+
+		if (!read_coredump_req(fd_coredump, &req))
+			goto out;
+
+		if (!check_coredump_req(&req))
+			goto out;
+
+		if (!send_coredump_ack(fd_coredump, &req,
+				       COREDUMP_KERNEL | COREDUMP_HEADER |
+				       COREDUMP_SPARSE | COREDUMP_WAIT, 0))
+			goto out;
+
+		if (!read_marker(fd_coredump, COREDUMP_MARK_REQACK))
+			goto out;
+
+		if (recv_coredump_frames(fd_coredump, fd_file, NULL) < 0)
+			goto out;
+
+		exit_code = EXIT_SUCCESS;
+out:
+		if (fd_file >= 0)
+			close(fd_file);
+		if (fd_peer_pidfd >= 0)
+			close(fd_peer_pidfd);
+		if (fd_coredump >= 0)
+			close(fd_coredump);
+		if (fd_server >= 0)
+			close(fd_server);
+		_exit(exit_code);
+	}
+	self->pid_coredump_server = pid_coredump_server;
+
+	EXPECT_EQ(close(ipc_sockets[1]), 0);
+	ASSERT_EQ(read_nointr(ipc_sockets[0], &c, 1), 1);
+	EXPECT_EQ(close(ipc_sockets[0]), 0);
+
+	pid = fork();
+	ASSERT_GE(pid, 0);
+	if (pid == 0)
+		crashing_child();
+
+	pidfd = sys_pidfd_open(pid, 0);
+	ASSERT_GE(pidfd, 0);
+
+	waitpid(pid, &status, 0);
+	ASSERT_TRUE(WIFSIGNALED(status));
+	ASSERT_TRUE(WCOREDUMP(status));
+
+	ASSERT_TRUE(get_pidfd_info(pidfd, &info));
+	ASSERT_GT((info.mask & PIDFD_INFO_COREDUMP), 0);
+	ASSERT_GT((info.coredump_mask & PIDFD_COREDUMPED), 0);
+
+	wait_and_check_coredump_server(pid_coredump_server, _metadata, self);
+
+	ASSERT_EQ(stat("/tmp/coredump.file", &st), 0);
+	ASSERT_GT(st.st_size, (off_t)sizeof(ehdr));
+
+	/* What the frames reassemble into has to be an ELF core file. */
+	fd_core_file = open("/tmp/coredump.file", O_RDONLY | O_CLOEXEC);
+	ASSERT_GE(fd_core_file, 0);
+	ASSERT_EQ(read_nointr(fd_core_file, ehdr, sizeof(ehdr)), sizeof(ehdr));
+	EXPECT_EQ(close(fd_core_file), 0);
+
+	/* e_type sits right behind e_ident in both ELF32 and ELF64. */
+	ASSERT_EQ(memcmp(ehdr, ELFMAG, SELFMAG), 0);
+	memcpy(&e_type, ehdr + EI_NIDENT, sizeof(e_type));
+	ASSERT_EQ(e_type, ET_CORE);
+}
+
+/*
+ * Crash a child with a mostly-unpopulated mapping and reassemble its
+ * framed coredump, reporting what crossed the socket and the coredump
+ * size the frames describe.
+ */
+static void test_framed_hole(struct __test_metadata *const _metadata,
+			     FIXTURE_DATA(coredump) *self, __u64 ack_mask,
+			     ssize_t *received, off_t *coredump_size)
+{
+	int pidfd, status;
+	pid_t pid, pid_coredump_server;
+	struct pidfd_info info = {};
+	int ipc_sockets[2];
+	int pipefds[2];
+	char c;
+
+	ASSERT_EQ(socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets), 0);
+	ASSERT_EQ(pipe(pipefds), 0);
+	ASSERT_TRUE(set_core_pattern("@@/tmp/coredump.socket"));
+
+	pid_coredump_server = fork();
+	ASSERT_GE(pid_coredump_server, 0);
+	if (pid_coredump_server == 0) {
+		int fd_server = -1, fd_coredump = -1, fd_peer_pidfd = -1;
+		int fd_file = -1;
+		int exit_code = EXIT_FAILURE;
+		struct coredump_req req = {};
+		off_t size = 0;
+		ssize_t ret;
+
+		close(ipc_sockets[0]);
+		close(pipefds[0]);
+
+		fd_server = create_and_listen_unix_socket("/tmp/coredump.socket");
+		if (fd_server < 0)
+			goto out;
+
+		if (write_nointr(ipc_sockets[1], "1", 1) < 0)
+			goto out;
+
+		close(ipc_sockets[1]);
+
+		fd_coredump = accept4(fd_server, NULL, NULL, SOCK_CLOEXEC);
+		if (fd_coredump < 0)
+			goto out;
+
+		fd_peer_pidfd = get_peer_pidfd(fd_coredump);
+		if (fd_peer_pidfd < 0)
+			goto out;
+
+		/*
+		 * The reassembled coredump is bigger than the mapping the
+		 * child made, so keep it on the detached tmpfs and sparse.
+		 */
+		fd_file = open_coredump_tmpfile(self->fd_tmpfs_detached);
+		if (fd_file < 0)
+			goto out;
+
+		if (!read_coredump_req(fd_coredump, &req))
+			goto out;
+
+		if (!check_coredump_req(&req))
+			goto out;
+
+		if (!send_coredump_ack(fd_coredump, &req, ack_mask, 0))
+			goto out;
+
+		if (!read_marker(fd_coredump, COREDUMP_MARK_REQACK))
+			goto out;
+
+		ret = recv_coredump_frames(fd_coredump, fd_file, &size);
+		if (ret < 0)
+			goto out;
+
+		if (write_nointr(pipefds[1], &ret, sizeof(ret)) != sizeof(ret))
+			goto out;
+		if (write_nointr(pipefds[1], &size, sizeof(size)) != sizeof(size))
+			goto out;
+
+		exit_code = EXIT_SUCCESS;
+out:
+		close(pipefds[1]);
+		if (fd_file >= 0)
+			close(fd_file);
+		if (fd_peer_pidfd >= 0)
+			close(fd_peer_pidfd);
+		if (fd_coredump >= 0)
+			close(fd_coredump);
+		if (fd_server >= 0)
+			close(fd_server);
+		_exit(exit_code);
+	}
+	self->pid_coredump_server = pid_coredump_server;
+
+	EXPECT_EQ(close(ipc_sockets[1]), 0);
+	EXPECT_EQ(close(pipefds[1]), 0);
+	ASSERT_EQ(read_nointr(ipc_sockets[0], &c, 1), 1);
+	EXPECT_EQ(close(ipc_sockets[0]), 0);
+
+	pid = fork();
+	ASSERT_GE(pid, 0);
+	if (pid == 0)
+		crashing_child_sparse(SPARSE_MAPPING_SIZE);
+
+	pidfd = sys_pidfd_open(pid, 0);
+	ASSERT_GE(pidfd, 0);
+
+	waitpid(pid, &status, 0);
+	ASSERT_TRUE(WIFSIGNALED(status));
+	ASSERT_TRUE(WCOREDUMP(status));
+
+	ASSERT_TRUE(get_pidfd_info(pidfd, &info));
+	ASSERT_GT((info.mask & PIDFD_INFO_COREDUMP), 0);
+	ASSERT_GT((info.coredump_mask & PIDFD_COREDUMPED), 0);
+
+	ASSERT_EQ(read_nointr(pipefds[0], received, sizeof(*received)),
+		  sizeof(*received));
+	ASSERT_EQ(read_nointr(pipefds[0], coredump_size, sizeof(*coredump_size)),
+		  sizeof(*coredump_size));
+	EXPECT_EQ(close(pipefds[0]), 0);
+
+	wait_and_check_coredump_server(pid_coredump_server, _metadata, self);
+
+	/* The mapping is in the coredump, holes included. */
+	ASSERT_GT(*coredump_size, (off_t)SPARSE_MAPPING_SIZE);
+}
+
+/*
+ * A mapping that has been written to is dumped whole, including the parts
+ * of it that were never faulted in. With COREDUMP_SPARSE the holes stay
+ * off the wire.
+ */
+TEST_F(coredump, socket_request_sparse_hole)
+{
+	off_t coredump_size = 0;
+	ssize_t received = 0;
+
+	test_framed_hole(_metadata, self,
+			 COREDUMP_KERNEL | COREDUMP_HEADER |
+			 COREDUMP_SPARSE | COREDUMP_WAIT,
+			 &received, &coredump_size);
+
+	/* The holes didn't have to go over the socket. */
+	ASSERT_LT(received, coredump_size / 8);
+}
+
+/*
+ * COREDUMP_HEADER alone frames the stream but elides nothing: the holes
+ * cross the socket as data frames.
+ */
+TEST_F(coredump, socket_request_header_hole)
+{
+	off_t coredump_size = 0;
+	ssize_t received = 0;
+
+	test_framed_hole(_metadata, self,
+			 COREDUMP_KERNEL | COREDUMP_HEADER | COREDUMP_WAIT,
+			 &received, &coredump_size);
+
+	/* Framing alone elides nothing, so everything crossed the socket. */
+	ASSERT_GT(received, coredump_size);
+}
+
+/* Ack @ack_mask, expect the kernel to refuse it as conflicting. */
+static void test_conflicting_ack(struct __test_metadata *const _metadata,
+				 FIXTURE_DATA(coredump) *self, __u64 ack_mask)
+{
+	int pidfd, status;
+	pid_t pid, pid_coredump_server;
+	struct pidfd_info info = {};
+	int ipc_sockets[2];
+	char c;
+
+	ASSERT_EQ(socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, ipc_sockets), 0);
+	ASSERT_TRUE(set_core_pattern("@@/tmp/coredump.socket"));
+
+	pid_coredump_server = fork();
+	ASSERT_GE(pid_coredump_server, 0);
+	if (pid_coredump_server == 0) {
+		int fd_server = -1, fd_coredump = -1, fd_peer_pidfd = -1;
+		int exit_code = EXIT_FAILURE;
+		struct coredump_req req = {};
+
+		close(ipc_sockets[0]);
+
+		fd_server = create_and_listen_unix_socket("/tmp/coredump.socket");
+		if (fd_server < 0)
+			goto out;
+
+		if (write_nointr(ipc_sockets[1], "1", 1) < 0)
+			goto out;
+
+		close(ipc_sockets[1]);
+
+		fd_coredump = accept4(fd_server, NULL, NULL, SOCK_CLOEXEC);
+		if (fd_coredump < 0)
+			goto out;
+
+		fd_peer_pidfd = get_peer_pidfd(fd_coredump);
+		if (fd_peer_pidfd < 0)
+			goto out;
+
+		if (!read_coredump_req(fd_coredump, &req))
+			goto out;
+
+		if (!check_coredump_req(&req))
+			goto out;
+
+		if (!send_coredump_ack(fd_coredump, &req, ack_mask, 0))
+			goto out;
+
+		if (!read_marker(fd_coredump, COREDUMP_MARK_CONFLICTING))
+			goto out;
+
+		exit_code = EXIT_SUCCESS;
+out:
+		if (fd_peer_pidfd >= 0)
+			close(fd_peer_pidfd);
+		if (fd_coredump >= 0)
+			close(fd_coredump);
+		if (fd_server >= 0)
+			close(fd_server);
+		_exit(exit_code);
+	}
+	self->pid_coredump_server = pid_coredump_server;
+
+	EXPECT_EQ(close(ipc_sockets[1]), 0);
+	ASSERT_EQ(read_nointr(ipc_sockets[0], &c, 1), 1);
+	EXPECT_EQ(close(ipc_sockets[0]), 0);
+
+	pid = fork();
+	ASSERT_GE(pid, 0);
+	if (pid == 0)
+		crashing_child();
+
+	pidfd = sys_pidfd_open(pid, 0);
+	ASSERT_GE(pidfd, 0);
+
+	waitpid(pid, &status, 0);
+	ASSERT_TRUE(WIFSIGNALED(status));
+	ASSERT_FALSE(WCOREDUMP(status));
+
+	ASSERT_TRUE(get_pidfd_info(pidfd, &info));
+	ASSERT_GT((info.mask & PIDFD_INFO_COREDUMP), 0);
+	ASSERT_GT((info.coredump_mask & PIDFD_COREDUMPED), 0);
+
+	wait_and_check_coredump_server(pid_coredump_server, _metadata, self);
+}
+
+/* COREDUMP_HEADER frames a coredump the kernel writes, nothing else. */
+TEST_F(coredump, socket_request_header_without_kernel)
+{
+	test_conflicting_ack(_metadata, self, COREDUMP_USERSPACE | COREDUMP_HEADER);
+}
+
+/* A zero frame can't exist outside a framed stream. */
+TEST_F(coredump, socket_request_sparse_without_header)
+{
+	test_conflicting_ack(_metadata, self, COREDUMP_KERNEL | COREDUMP_SPARSE);
+}
+
 TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/coredump/coredump_test.h b/tools/testing/selftests/coredump/coredump_test.h
index a02809145e2d..ae1751298aaf 100644
--- a/tools/testing/selftests/coredump/coredump_test.h
+++ b/tools/testing/selftests/coredump/coredump_test.h
@@ -16,6 +16,9 @@
 
 #define NUM_THREAD_SPAWN 128
 
+/* Size of the mostly unpopulated mapping the sparse coredump test maps. */
+#define SPARSE_MAPPING_SIZE (256 * 1024 * 1024)
+
 /* Coredump fixture */
 FIXTURE(coredump)
 {
@@ -27,6 +30,9 @@ FIXTURE(coredump)
 /* Shared helper function declarations */
 void *do_nothing(void *arg);
 void crashing_child(void);
+void crashing_child_sparse(size_t size);
+ssize_t recv_coredump_frames(int fd_coredump, int fd_core_file,
+			     off_t *coredump_size);
 int create_detached_tmpfs(void);
 int create_and_listen_unix_socket(const char *path);
 bool set_core_pattern(const char *pattern);
diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.c b/tools/testing/selftests/coredump/coredump_test_helpers.c
index ee669969605b..dececebe15cb 100644
--- a/tools/testing/selftests/coredump/coredump_test_helpers.c
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.c
@@ -13,6 +13,7 @@
 #include <string.h>
 #include <sys/epoll.h>
 #include <sys/ioctl.h>
+#include <sys/mman.h>
 #include <sys/socket.h>
 #include <sys/types.h>
 #include <sys/un.h>
@@ -59,6 +60,156 @@ void crashing_child(void)
 	i = *(volatile int *)NULL;
 }
 
+void crashing_child_sparse(size_t size)
+{
+	char *p;
+
+	/*
+	 * Touch the first page only. The whole mapping is dumped because
+	 * it has been written to, but all of it save that one page is a
+	 * hole.
+	 */
+	p = mmap(NULL, size, PROT_READ | PROT_WRITE,
+		 MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0);
+	if (p != MAP_FAILED)
+		p[0] = 'x';
+
+	/* crash on purpose */
+	*(volatile int *)NULL = 0;
+}
+
+/* Read @len bytes off the socket, writing them at @offset if @fd_out >= 0. */
+static ssize_t recv_frame_bytes(int fd_coredump, __u64 len, int fd_out,
+				off_t offset)
+{
+	ssize_t received = 0;
+
+	while (len) {
+		char buffer[PAGE_SIZE];
+		size_t chunk = len < sizeof(buffer) ? len : sizeof(buffer);
+		ssize_t ret;
+
+		ret = recv(fd_coredump, buffer, chunk, MSG_WAITALL);
+		if (ret <= 0) {
+			fprintf(stderr, "%s: short read %zd: %m\n",
+				__func__, ret);
+			return -1;
+		}
+
+		if (fd_out >= 0 &&
+		    pwrite(fd_out, buffer, ret, offset + received) != ret) {
+			fprintf(stderr, "%s: pwrite failed: %m\n", __func__);
+			return -1;
+		}
+
+		received += ret;
+		len -= ret;
+	}
+
+	return received;
+}
+
+ssize_t recv_coredump_frames(int fd_coredump, int fd_core_file,
+			     off_t *coredump_size)
+{
+	ssize_t received = 0;
+	off_t size = 0;
+
+	for (;;) {
+		struct coredump_frame_header frame = {};
+		size_t known;
+		ssize_t ret;
+
+		/* Peek the header size the way read_coredump_req() does. */
+		ret = recv(fd_coredump, &frame, sizeof(frame.size),
+			   MSG_PEEK | MSG_WAITALL);
+		if (ret == 0)
+			break;
+		if (ret != sizeof(frame.size)) {
+			fprintf(stderr, "%s: short frame peek %zd: %m\n",
+				__func__, ret);
+			return -1;
+		}
+
+		if (frame.size < COREDUMP_FRAME_HEADER_SIZE_VER0) {
+			fprintf(stderr, "%s: header size %u below minimum %u\n",
+				__func__, frame.size,
+				COREDUMP_FRAME_HEADER_SIZE_VER0);
+			return -1;
+		}
+
+		/* Consume as much of the header as we know about. */
+		known = frame.size < sizeof(frame) ? frame.size : sizeof(frame);
+		ret = recv(fd_coredump, &frame, known, MSG_WAITALL);
+		if (ret != (ssize_t)known) {
+			fprintf(stderr, "%s: short frame read %zd: %m\n",
+				__func__, ret);
+			return -1;
+		}
+		received += ret;
+
+		/*
+		 * A flag changes what the frame means, so refuse one we
+		 * don't know rather than guess.
+		 */
+		if (frame.flags) {
+			fprintf(stderr, "%s: unknown header flags 0x%llx\n",
+				__func__, (unsigned long long)frame.flags);
+			return -1;
+		}
+
+		/* Discard any part of the header we have no use for. */
+		ret = recv_frame_bytes(fd_coredump, frame.size - known, -1, 0);
+		if (ret < 0)
+			return -1;
+		received += ret;
+
+		/* Frames are sent in order and they don't leave gaps. */
+		if (frame.offset != (__u64)size) {
+			fprintf(stderr, "%s: frame at %llu, expected %llu\n",
+				__func__, (unsigned long long)frame.offset,
+				(unsigned long long)size);
+			return -1;
+		}
+
+		switch (frame.type) {
+		case COREDUMP_FRAME_ZERO:
+			/* A hole. It comes with no data and needs none. */
+			break;
+		case COREDUMP_FRAME_DATA:
+			ret = recv_frame_bytes(fd_coredump, frame.len,
+					       fd_core_file, size);
+			if (ret < 0)
+				return -1;
+			received += ret;
+			break;
+		default:
+			fprintf(stderr, "%s: unknown frame type %u\n",
+				__func__, frame.type);
+			return -1;
+		}
+
+		size += frame.len;
+	}
+
+	/*
+	 * Nothing is written for a hole, so grow the file to the size the
+	 * frames describe in case the coredump ended in one.
+	 */
+	if (ftruncate(fd_core_file, size) < 0) {
+		fprintf(stderr, "%s: ftruncate to %llu failed: %m\n",
+			__func__, (unsigned long long)size);
+		return -1;
+	}
+
+	if (coredump_size)
+		*coredump_size = size;
+
+	fprintf(stderr, "Received %zd bytes for a coredump of %llu bytes\n",
+		received, (unsigned long long)size);
+	return received;
+}
+
 int create_detached_tmpfs(void)
 {
 	int fd_context, fd_tmpfs;
@@ -101,6 +252,7 @@ int create_and_listen_unix_socket(const char *path)
 	return fd;
 
 out:
+	fprintf(stderr, "%s: %s: %m\n", __func__, path);
 	if (fd >= 0)
 		close(fd);
 	return -1;
@@ -279,8 +431,10 @@ bool send_coredump_ack(int fd, const struct coredump_req *req,
 	large_ack.ack.mask = mask;
 	large_ack.ack.size = size_ack;
 	ret = send(fd, &large_ack, size_ack, MSG_NOSIGNAL);
-	if (ret != size_ack)
+	if (ret != size_ack) {
+		fprintf(stderr, "%s: short send %zd: %m\n", __func__, ret);
 		return false;
+	}
 
 	fprintf(stderr, "Sent coredump ack with size %zu and mask 0x%llx\n",
 		size_ack, (unsigned long long)mask);

-- 
2.53.0

Re: [PATCH 00/11] coredump: allow to create sparse coredumps on the coredump socket

[Jann Horn]

On Tue, Aug 11, 2026 at 5:27 PM Christian Brauner <brauner@kernel.org> wrote:
> A server that raises COREDUMP_HEADER in coredump_ack->mask doesn't get
> the coredump as a plain byte stream but as a sequence of frames. Each
> one a struct coredump_frame_header followed by what it describes. A data
> frame carries its bytes. If a server also raises COREDUMP_SPARSE, zero
> frames are sent for unpopulated mappings. They only indicate how many
> zero bytes need to be written and to not include data. Reassembling the
> frames gives back the same coredump. A debugger and everything else
> still see an ordinary core file and nothing outside the coredump server
> has to learn anything.

Hmm...

I think what you're doing is probably the easiest way to do this in
practice. I guess some design alternatives would be:

1. (overengineered, not generically useful enough): If the transport
was a pipe (which already has the concept of different types of pipe
buffers) instead of a unix domain socket, we could introduce a special
representation for zero-filled pipe buffers and some API for receiving
zeroed holes through lseek(pipefd, 0, SEEK_DATA), but that's probably
not sufficiently useful for stuff other than core dumping to be worth
the effort.
2. (somewhat overengineered) With some refactoring, we could maybe do
something like /proc/kcore and create a seekable virtual coredump file
that we send over the socket via SCM_RIGHTS?
3. We could leave the userspace memory dump out of the core dump data,
and let userspace take care of filling out the memory contents using
/proc/$pid/pagemap and /proc/$pid/mem? That would also avoid task
switches and SKB allocations, and probably reduce the number of data
copies involved in this by 1.

Re: [PATCH 00/11] coredump: allow to create sparse coredumps on the coredump socket

[Jann Horn]

On Tue, Aug 11, 2026 at 9:07 PM Jann Horn <jannh@google.com> wrote:
> On Tue, Aug 11, 2026 at 5:27 PM Christian Brauner <brauner@kernel.org> wrote:
> > A server that raises COREDUMP_HEADER in coredump_ack->mask doesn't get
> > the coredump as a plain byte stream but as a sequence of frames. Each
> > one a struct coredump_frame_header followed by what it describes. A data
> > frame carries its bytes. If a server also raises COREDUMP_SPARSE, zero
> > frames are sent for unpopulated mappings. They only indicate how many
> > zero bytes need to be written and to not include data. Reassembling the
> > frames gives back the same coredump. A debugger and everything else
> > still see an ordinary core file and nothing outside the coredump server
> > has to learn anything.
>
> Hmm...
>
> I think what you're doing is probably the easiest way to do this in
> practice. I guess some design alternatives would be:
>
> 1. (overengineered, not generically useful enough): If the transport
> was a pipe (which already has the concept of different types of pipe
> buffers) instead of a unix domain socket, we could introduce a special
> representation for zero-filled pipe buffers and some API for receiving
> zeroed holes through lseek(pipefd, 0, SEEK_DATA), but that's probably
> not sufficiently useful for stuff other than core dumping to be worth
> the effort.
> 2. (somewhat overengineered) With some refactoring, we could maybe do
> something like /proc/kcore and create a seekable virtual coredump file
> that we send over the socket via SCM_RIGHTS?
> 3. We could leave the userspace memory dump out of the core dump data,
> and let userspace take care of filling out the memory contents using
> /proc/$pid/pagemap and /proc/$pid/mem? That would also avoid task
> switches and SKB allocations, and probably reduce the number of data
> copies involved in this by 1.

I also wonder what userspace actually does with this data - does
userspace just want to write it to disk, potentially after compressing
it? Or does userspace actually do some fancy parsing of the data
stream to extract stack memory or something like that? Or does
userspace buffer the whole thing into RAM and then process it from
there (it kinda looks like systemd tries to do that but I might be
reading this wrong)?

Anyway, I've looked through your code and it does look fine to me.

Re: [PATCH 00/11] coredump: allow to create sparse coredumps on the coredump socket

[Christian Brauner]

On Tue, Aug 11, 2026 at 09:39:12PM +0200, Jann Horn wrote:
> On Tue, Aug 11, 2026 at 9:07 PM Jann Horn <jannh@google.com> wrote:
> > On Tue, Aug 11, 2026 at 5:27 PM Christian Brauner <brauner@kernel.org> wrote:
> > > A server that raises COREDUMP_HEADER in coredump_ack->mask doesn't get
> > > the coredump as a plain byte stream but as a sequence of frames. Each
> > > one a struct coredump_frame_header followed by what it describes. A data
> > > frame carries its bytes. If a server also raises COREDUMP_SPARSE, zero
> > > frames are sent for unpopulated mappings. They only indicate how many
> > > zero bytes need to be written and to not include data. Reassembling the
> > > frames gives back the same coredump. A debugger and everything else
> > > still see an ordinary core file and nothing outside the coredump server
> > > has to learn anything.
> >
> > Hmm...
> >
> > I think what you're doing is probably the easiest way to do this in
> > practice. I guess some design alternatives would be:
> >
> > 1. (overengineered, not generically useful enough): If the transport
> > was a pipe (which already has the concept of different types of pipe
> > buffers) instead of a unix domain socket, we could introduce a special
> > representation for zero-filled pipe buffers and some API for receiving
> > zeroed holes through lseek(pipefd, 0, SEEK_DATA), but that's probably
> > not sufficiently useful for stuff other than core dumping to be worth
> > the effort.
> > 2. (somewhat overengineered) With some refactoring, we could maybe do
> > something like /proc/kcore and create a seekable virtual coredump file
> > that we send over the socket via SCM_RIGHTS?
> > 3. We could leave the userspace memory dump out of the core dump data,
> > and let userspace take care of filling out the memory contents using
> > /proc/$pid/pagemap and /proc/$pid/mem? That would also avoid task
> > switches and SKB allocations, and probably reduce the number of data
> > copies involved in this by 1.
> 
> I also wonder what userspace actually does with this data - does
> userspace just want to write it to disk, potentially after compressing
> it? Or does userspace actually do some fancy parsing of the data

That would be the most straightforward use-case, yes.

> stream to extract stack memory or something like that? Or does
> userspace buffer the whole thing into RAM and then process it from
> there (it kinda looks like systemd tries to do that but I might be
> reading this wrong)?

I'm not sure about that. I think it's writing it to disk and then
parsing it. But it also wranges it into a socket to forward to
containers or services.

Note that systemd has a pull request for the coredump socket up:

https://github.com/systemd/systemd/pull/43330

This should kill the usermodehelper soon on kernels that support the
socket.

> Anyway, I've looked through your code and it does look fine to me.

Thanks.