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

[PATCH v2 00/22] 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_RECORDS in coredump_ack->mask doesn't get
the coredump as a plain byte stream but as a sequence of records. Each
one a struct coredump_record_header followed by what it describes. A
data record carries its bytes. If a server also raises COREDUMP_SPARSE,
zero records are sent for unpopulated mappings. They only indicate how
many zero bytes need to be written and do not include data. Reassembling
the records 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 selftests, on a kernel built from this series:

- a process with 128 threads: 1424153 bytes on the socket for a
  coredump of 1075150848 bytes
- a 256MB mapping with the first and last page touched: 188793 bytes on
  the socket for a coredump of 268890112 bytes
- the same 256MB mapping with COREDUMP_RECORDS alone: 271009312 bytes on
  the socket, so the record 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>
---
Changes in v2:
- Use standard naming aligning with other subsystems.
- Add a termination record to make this really clean.
- Link to v1: https://patch.msgid.link/20260811-work-coredump-sparse-v1-0-cd3e8b1e356d@kernel.org

---
Christian Brauner (22):
      powerpc/spufs: don't dump more than the note supports
      coredump: refuse negative skips
      coredump: set the minimum send buffer size
      selftests/coredump: discard the right amount after the coredump request
      selftests/coredump: collapse the expected request check into the helper
      selftests/coredump: add a separate helper header
      coredump: pin the protocol struct sizes
      coredump: move the negotiated mask into struct coredump_params
      coredump: deduplicate the to_skip flush
      coredump: make the dump helper return bool
      coredump: always chunk writes
      coredump: clean up coredump state handling
      coredump: add COREDUMP_RECORDS to the coredump socket protocol
      coredump: add COREDUMP_SPARSE to the coredump socket protocol
      tools: sync coredump.h header
      coredump: send the coredump in records if requested
      coredump: describe the holes when COREDUMP_SPARSE is negotiated
      selftests/coredump: test COREDUMP_RECORDS and COREDUMP_SPARSE
      selftests/coredump: hand the record stream to a sink
      selftests/coredump: put a hole in the middle of a sparse mapping
      selftests/coredump: simulate a blob store
      selftests/coredump: show how to inspect the task to decide how the coredump should be sent

 arch/powerpc/platforms/cell/spufs/file.c           |   18 +-
 fs/binfmt_elf.c                                    |   12 +-
 fs/binfmt_elf_fdpic.c                              |   12 +-
 fs/coredump.c                                      |  325 ++++--
 include/linux/binfmts.h                            |    3 +-
 include/linux/coredump.h                           |   33 +-
 include/uapi/linux/coredump.h                      |   79 +-
 tools/include/uapi/linux/coredump.h                |   79 +-
 .../coredump/coredump_socket_protocol_test.c       |  783 ++++++++++++-
 tools/testing/selftests/coredump/coredump_test.h   |   31 +-
 .../selftests/coredump/coredump_test_helpers.c     | 1171 +++++++++++++++++++-
 .../selftests/coredump/coredump_test_helpers.h     |   53 +
 12 files changed, 2399 insertions(+), 200 deletions(-)
---
base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
change-id: 20260811-work-coredump-sparse-18177d77b014

[PATCH v2 01/22] powerpc/spufs: don't dump more than the note supports

[Christian Brauner]

The spufs_arch_write_note() function puts notes in the header and uses
them to fin where the next note starts. The spufs_coredump_read[] array
provides the sizes of the notes:

	dump_skip_to(cprm, roundup(cprm->pos - ret + sz, 4));

In this call @ret is the amount of data the dump callback wrote. @sz is
the declared size. So the position moves backwards if the callback
wrote more data than the declared size.

For three note sizes that is the case:

(1) signal1 sets sizeof(u32) and dumps u64 via sizeof(ctx->csa.spu_chnldata_RW[3])
(2) signal2 sets sizeof(u32) and dumps u64 via sizeof(ctx->csa.spu_chnldata_RW[4])
(3) ibox_info sets sizeof(u32) and dumps a u64 via puint_mb_R

The note is 4 byte aligned. The dump_emit() call wrote the dump_align(4)
just before the note. So if @ret is 8 and @sz is 4 the position ends up
4 bytes before the current position which means cprm->to_skip is now
negative.

For __dump_skip() with size_t that means the pipe or socket gets 2^52
PAGE_SIZE zeroes. This also means a file seeks backwards and overwrites
the four bytes that it just wrote.

Before commit 5456ffdee666 ("powerpc/spufs: simplify spufs core
dumping") this was benign because this truncated (on purpose, I
presume):

	u32 data;

	data = ctx->csa.spu_chnldata_RW[3];
	...
	copy_to_user(buf, &data, 4)

and after said commit things became fscked. So let's truncate this
again. Not truncation means the wrong bits will be picked on big endian.

Afaict, spufs is effectively dead so the fix probably doesn't matter in
the grand scheme of things.

Fixes: 5456ffdee666 ("powerpc/spufs: simplify spufs core dumping")
Cc: stable@vger.kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 arch/powerpc/platforms/cell/spufs/file.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spufs/file.c b/arch/powerpc/platforms/cell/spufs/file.c
index de7494748fec..6f86d87e3749 100644
--- a/arch/powerpc/platforms/cell/spufs/file.c
+++ b/arch/powerpc/platforms/cell/spufs/file.c
@@ -956,10 +956,12 @@ spufs_signal1_release(struct inode *inode, struct file *file)
 static ssize_t spufs_signal1_dump(struct spu_context *ctx,
 		struct coredump_params *cprm)
 {
+	u32 data;
+
 	if (!ctx->csa.spu_chnlcnt_RW[3])
 		return 0;
-	return spufs_dump_emit(cprm, &ctx->csa.spu_chnldata_RW[3],
-			       sizeof(ctx->csa.spu_chnldata_RW[3]));
+	data = ctx->csa.spu_chnldata_RW[3];
+	return spufs_dump_emit(cprm, &data, sizeof(data));
 }
 
 static ssize_t __spufs_signal1_read(struct spu_context *ctx, char __user *buf,
@@ -1089,10 +1091,12 @@ spufs_signal2_release(struct inode *inode, struct file *file)
 static ssize_t spufs_signal2_dump(struct spu_context *ctx,
 		struct coredump_params *cprm)
 {
+	u32 data;
+
 	if (!ctx->csa.spu_chnlcnt_RW[4])
 		return 0;
-	return spufs_dump_emit(cprm, &ctx->csa.spu_chnldata_RW[4],
-			       sizeof(ctx->csa.spu_chnldata_RW[4]));
+	data = ctx->csa.spu_chnldata_RW[4];
+	return spufs_dump_emit(cprm, &data, sizeof(data));
 }
 
 static ssize_t __spufs_signal2_read(struct spu_context *ctx, char __user *buf,
@@ -1965,10 +1969,12 @@ static const struct file_operations spufs_mbox_info_fops = {
 static ssize_t spufs_ibox_info_dump(struct spu_context *ctx,
 		struct coredump_params *cprm)
 {
+	u32 data;
+
 	if (!(ctx->csa.prob.mb_stat_R & 0xff0000))
 		return 0;
-	return spufs_dump_emit(cprm, &ctx->csa.priv2.puint_mb_R,
-			       sizeof(ctx->csa.priv2.puint_mb_R));
+	data = ctx->csa.priv2.puint_mb_R;
+	return spufs_dump_emit(cprm, &data, sizeof(data));
 }
 
 static ssize_t spufs_ibox_info_read(struct file *file, char __user *buf,

-- 
2.53.0

[PATCH v2 02/22] coredump: refuse negative skips

[Christian Brauner]

The dump_skip_to() helper calculates a relative skip based on the
absolute positon of the coredump:

	cprm->to_skip = pos - cprm->pos;

That's easy to mess up for callers and one already did. This risk
endless zero PAGE_SIZE loops or overwriting already written coredump
data thereby corrupting the dump.

I don't think skipping backwards has any meaning. So warn and refuse.

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

diff --git a/fs/coredump.c b/fs/coredump.c
index e68a76ff92a3..ac4e922c9cb9 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -1261,6 +1261,8 @@ EXPORT_SYMBOL(dump_emit);
 
 void dump_skip_to(struct coredump_params *cprm, unsigned long pos)
 {
+	if (WARN_ON_ONCE(pos < cprm->pos))
+		return;
 	cprm->to_skip = pos - cprm->pos;
 }
 EXPORT_SYMBOL(dump_skip_to);

-- 
2.53.0

[PATCH v2 03/22] coredump: set the minimum send buffer size

[Christian Brauner]

The send buffer of the coredump socket is subject to the limit in
net.core.wmem_default. af_unix uses sk_sndbuf / 2 - 64 bytes for a
single skb. That means a send buffer below that would split a page-sized
write into multiple skbs. Raise the send buffer to leave room for a
page-sized write plus the header. The default value is well above that.
So we really change it when it's below our minimum.

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

diff --git a/fs/coredump.c b/fs/coredump.c
index ac4e922c9cb9..24a6405ea242 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -665,6 +665,9 @@ static int umh_coredump_setup(struct subprocess_info *info, struct cred *new)
 }
 
 #ifdef CONFIG_UNIX
+/* af_unix halves the send buffer to size a single skb. */
+#define COREDUMP_SOCK_SNDBUF_MIN (3 * PAGE_SIZE)
+
 static bool coredump_sock_connect(struct core_name *cn, struct coredump_params *cprm)
 {
 	struct file *file __free(fput) = NULL;
@@ -690,6 +693,10 @@ static bool coredump_sock_connect(struct core_name *cn, struct coredump_params *
 	if (retval < 0)
 		return false;
 
+	/* Don't let a page-sized write split into several skbs. */
+	socket->sk->sk_sndbuf = max_t(int, socket->sk->sk_sndbuf,
+				      COREDUMP_SOCK_SNDBUF_MIN);
+
 	file = sock_alloc_file(socket, 0, NULL);
 	if (IS_ERR(file))
 		return false;

-- 
2.53.0

[PATCH v2 04/22] 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...

It's 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 v2 05/22] 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..306711e1b24d 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_size, 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_size = user_size < kernel_size ? user_size : kernel_size;
+	ret = recv(fd, req, known_size, MSG_WAITALL);
+	if (ret != known_size)
 		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 TEST_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 != TEST_REQ_MASK_ALL) {
+		fprintf(stderr, "%s: mask 0x%llx, expected 0x%llx\n",
+			__func__, (unsigned long long)req->mask,
+			(unsigned long long)TEST_REQ_MASK_ALL);
 		return false;
+	}
 	return true;
 }
 

-- 
2.53.0

[PATCH v2 06/22] selftests/coredump: add a separate helper header

[Christian Brauner]

Right now we have coredump_test.h which pulls in the test harness.
So it can't be included in coredump_test_helpers.c and it hand-rolls a
bunch of stuff that is not needed.

Instead of this mess, split everything out into a separate
coredump_test_helpers.h header and make both coredump_test.h and
coredump_test_helpers.c include it.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 tools/testing/selftests/coredump/coredump_test.h   | 30 +-----------------
 .../selftests/coredump/coredump_test_helpers.c     | 17 +---------
 .../selftests/coredump/coredump_test_helpers.h     | 37 ++++++++++++++++++++++
 3 files changed, 39 insertions(+), 45 deletions(-)

diff --git a/tools/testing/selftests/coredump/coredump_test.h b/tools/testing/selftests/coredump/coredump_test.h
index a02809145e2d..8d99b5cb2f12 100644
--- a/tools/testing/selftests/coredump/coredump_test.h
+++ b/tools/testing/selftests/coredump/coredump_test.h
@@ -3,18 +3,9 @@
 #ifndef __COREDUMP_TEST_H
 #define __COREDUMP_TEST_H
 
-#include <stdbool.h>
-#include <sys/types.h>
-#include <linux/coredump.h>
-
 #include "../kselftest_harness.h"
-#include "../pidfd/pidfd.h"
-
-#ifndef PAGE_SIZE
-#define PAGE_SIZE 4096
-#endif
 
-#define NUM_THREAD_SPAWN 128
+#include "coredump_test_helpers.h"
 
 /* Coredump fixture */
 FIXTURE(coredump)
@@ -24,15 +15,6 @@ FIXTURE(coredump)
 	int fd_tmpfs_detached;
 };
 
-/* Shared helper function declarations */
-void *do_nothing(void *arg);
-void crashing_child(void);
-int create_detached_tmpfs(void);
-int create_and_listen_unix_socket(const char *path);
-bool set_core_pattern(const char *pattern);
-int get_peer_pidfd(int fd);
-bool get_pidfd_info(int fd_peer_pidfd, struct pidfd_info *info);
-
 /* Inline helper that uses harness types */
 static inline void wait_and_check_coredump_server(pid_t pid_coredump_server,
 						   struct __test_metadata *const _metadata,
@@ -45,14 +27,4 @@ static inline void wait_and_check_coredump_server(pid_t pid_coredump_server,
 	ASSERT_EQ(WEXITSTATUS(status), 0);
 }
 
-/* Protocol helper function declarations */
-ssize_t recv_marker(int fd);
-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);
-int open_coredump_tmpfile(int fd_tmpfs_detached);
-void process_coredump_worker(int fd_coredump, int fd_peer_pidfd, int fd_core_file);
-
 #endif /* __COREDUMP_TEST_H */
diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.c b/tools/testing/selftests/coredump/coredump_test_helpers.c
index 306711e1b24d..570fc2e005c2 100644
--- a/tools/testing/selftests/coredump/coredump_test_helpers.c
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.c
@@ -20,23 +20,8 @@
 #include <unistd.h>
 
 #include "../filesystems/wrappers.h"
-#include "../pidfd/pidfd.h"
 
-/* Forward declarations to avoid including harness header */
-struct __test_metadata;
-
-/* Match the fixture definition from coredump_test.h */
-struct _fixture_coredump_data {
-	char original_core_pattern[256];
-	pid_t pid_coredump_server;
-	int fd_tmpfs_detached;
-};
-
-#ifndef PAGE_SIZE
-#define PAGE_SIZE 4096
-#endif
-
-#define NUM_THREAD_SPAWN 128
+#include "coredump_test_helpers.h"
 
 void *do_nothing(void *arg)
 {
diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.h b/tools/testing/selftests/coredump/coredump_test_helpers.h
new file mode 100644
index 000000000000..45904bd177b8
--- /dev/null
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __COREDUMP_TEST_HELPERS_H
+#define __COREDUMP_TEST_HELPERS_H
+
+#include <stdbool.h>
+#include <sys/types.h>
+#include <linux/coredump.h>
+
+#include "../pidfd/pidfd.h"
+
+#ifndef PAGE_SIZE
+#define PAGE_SIZE 4096
+#endif
+
+#define NUM_THREAD_SPAWN 128
+
+/* Shared helper function declarations */
+void *do_nothing(void *arg);
+void crashing_child(void);
+int create_detached_tmpfs(void);
+int create_and_listen_unix_socket(const char *path);
+bool set_core_pattern(const char *pattern);
+int get_peer_pidfd(int fd);
+bool get_pidfd_info(int fd_peer_pidfd, struct pidfd_info *info);
+
+/* Protocol helper function declarations */
+ssize_t recv_marker(int fd);
+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);
+int open_coredump_tmpfile(int fd_tmpfs_detached);
+void process_coredump_worker(int fd_coredump, int fd_peer_pidfd, int fd_core_file);
+
+#endif /* __COREDUMP_TEST_HELPERS_H */

-- 
2.53.0

[PATCH v2 07/22] 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. While at it
fix some issues with docs.

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 24a6405ea242..c696b46c97ff 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -759,6 +759,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 v2 08/22] 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 to make them available.

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 c696b46c97ff..156688be3df0 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;
@@ -860,7 +859,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);
 }
 
@@ -1129,7 +1128,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 */
@@ -1137,13 +1136,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;
 
 	/*
@@ -1151,7 +1150,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 v2 09/22] 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 156688be3df0..cd36a72be445 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -1256,13 +1256,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_skip(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_skip(cprm))
+		return 0;
 	return __dump_emit(cprm, addr, nr);
 }
 EXPORT_SYMBOL(dump_emit);
@@ -1293,11 +1301,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_skip(cprm))
+		return 0;
 	if (cprm->written + PAGE_SIZE > cprm->limit)
 		return 0;
 	if (dump_interrupted())

-- 
2.53.0

[PATCH v2 10/22] coredump: make the dump helper return bool

[Christian Brauner]

The various dump helpers return one and zero. Every caller just does a
boolean test. Convert them to return an actual bool.

While at it, drop the externs.

No functional changes.

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

diff --git a/fs/coredump.c b/fs/coredump.c
index cd36a72be445..d837819031ff 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -1215,41 +1215,41 @@ 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_emit(struct coredump_params *cprm, const void *addr, int nr)
 {
 	struct file *file = cprm->file;
 	loff_t pos = file->f_pos;
 	ssize_t n;
 
 	if (cprm->written + nr > cprm->limit)
-		return 0;
+		return false;
 	if (dump_interrupted())
-		return 0;
+		return false;
 	n = __kernel_write(file, addr, nr, &pos);
 	if (n != nr)
-		return 0;
+		return false;
 	file->f_pos = pos;
 	cprm->written += n;
 	cprm->pos += n;
 
-	return 1;
+	return true;
 }
 
-static int __dump_skip(struct coredump_params *cprm, size_t nr)
+static bool __dump_skip(struct coredump_params *cprm, size_t nr)
 {
 	static char zeroes[PAGE_SIZE];
 	struct file *file = cprm->file;
 
 	if (file->f_mode & FMODE_LSEEK) {
 		if (dump_interrupted() || vfs_llseek(file, nr, SEEK_CUR) < 0)
-			return 0;
+			return false;
 		cprm->pos += nr;
-		return 1;
+		return true;
 	}
 
 	while (nr > PAGE_SIZE) {
 		if (!__dump_emit(cprm, zeroes, PAGE_SIZE))
-			return 0;
+			return false;
 		nr -= PAGE_SIZE;
 	}
 
@@ -1257,20 +1257,20 @@ static int __dump_skip(struct coredump_params *cprm, size_t nr)
 }
 
 /* Flush the accumulated hole before writing data. */
-static int dump_flush_skip(struct coredump_params *cprm)
+static bool dump_flush_skip(struct coredump_params *cprm)
 {
 	if (cprm->to_skip) {
 		if (!__dump_skip(cprm, cprm->to_skip))
-			return 0;
+			return false;
 		cprm->to_skip = 0;
 	}
-	return 1;
+	return true;
 }
 
-int dump_emit(struct coredump_params *cprm, const void *addr, int nr)
+bool dump_emit(struct coredump_params *cprm, const void *addr, int nr)
 {
 	if (!dump_flush_skip(cprm))
-		return 0;
+		return false;
 	return __dump_emit(cprm, addr, nr);
 }
 EXPORT_SYMBOL(dump_emit);
@@ -1290,7 +1290,7 @@ void dump_skip(struct coredump_params *cprm, size_t nr)
 EXPORT_SYMBOL(dump_skip);
 
 #ifdef CONFIG_ELF_CORE
-static int dump_emit_page(struct coredump_params *cprm, struct page *page)
+static bool dump_emit_page(struct coredump_params *cprm, struct page *page)
 {
 	struct bio_vec bvec;
 	struct iov_iter iter;
@@ -1299,25 +1299,25 @@ static int dump_emit_page(struct coredump_params *cprm, struct page *page)
 	ssize_t n;
 
 	if (!page)
-		return 0;
+		return false;
 
 	if (!dump_flush_skip(cprm))
-		return 0;
+		return false;
 	if (cprm->written + PAGE_SIZE > cprm->limit)
-		return 0;
+		return false;
 	if (dump_interrupted())
-		return 0;
+		return false;
 	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;
+		return false;
 	file->f_pos = pos;
 	cprm->written += PAGE_SIZE;
 	cprm->pos += PAGE_SIZE;
 
-	return 1;
+	return true;
 }
 
 /*
@@ -1349,18 +1349,19 @@ static inline struct page *dump_page_copy(struct page *src, struct page *dst)
 }
 #endif
 
-int dump_user_range(struct coredump_params *cprm, unsigned long start,
-		    unsigned long len)
+bool dump_user_range(struct coredump_params *cprm, unsigned long start,
+		     unsigned long len)
 {
 	unsigned long addr;
 	struct page *dump_page;
-	int locked, ret;
+	int locked;
+	bool ret;
 
 	dump_page = dump_page_alloc();
 	if (!dump_page)
-		return 0;
+		return false;
 
-	ret = 0;
+	ret = false;
 	locked = 0;
 	for (addr = start; addr < start + len; addr += PAGE_SIZE) {
 		struct page *page;
@@ -1384,7 +1385,7 @@ int dump_user_range(struct coredump_params *cprm, unsigned long start,
 				mmap_read_unlock(current->mm);
 				locked = 0;
 			}
-			int stop = !dump_emit_page(cprm, dump_page_copy(page, dump_page));
+			bool stop = !dump_emit_page(cprm, dump_page_copy(page, dump_page));
 			put_page(page);
 			if (stop)
 				goto out;
@@ -1403,7 +1404,7 @@ int dump_user_range(struct coredump_params *cprm, unsigned long start,
 		}
 		cond_resched();
 	}
-	ret = 1;
+	ret = true;
 out:
 	if (locked)
 		mmap_read_unlock(current->mm);
@@ -1413,14 +1414,14 @@ int dump_user_range(struct coredump_params *cprm, unsigned long start,
 }
 #endif
 
-int dump_align(struct coredump_params *cprm, int align)
+bool dump_align(struct coredump_params *cprm, int align)
 {
 	unsigned mod = (cprm->pos + cprm->to_skip) & (align - 1);
 	if (align & (align - 1))
-		return 0;
+		return false;
 	if (mod)
 		cprm->to_skip += align - mod;
-	return 1;
+	return true;
 }
 EXPORT_SYMBOL(dump_align);
 
diff --git a/include/linux/coredump.h b/include/linux/coredump.h
index dc7a05b1bb0a..943bddfb22bf 100644
--- a/include/linux/coredump.h
+++ b/include/linux/coredump.h
@@ -43,13 +43,13 @@ extern unsigned int core_file_note_size_limit;
  * These are the only things you should do on a core-file: use only these
  * functions to write out all the necessary info.
  */
-extern void dump_skip_to(struct coredump_params *cprm, unsigned long to);
-extern void dump_skip(struct coredump_params *cprm, size_t nr);
-extern int dump_emit(struct coredump_params *cprm, const void *addr, int nr);
-extern int dump_align(struct coredump_params *cprm, int align);
-int dump_user_range(struct coredump_params *cprm, unsigned long start,
-		    unsigned long len);
-extern void vfs_coredump(const kernel_siginfo_t *siginfo);
+void dump_skip_to(struct coredump_params *cprm, unsigned long to);
+void dump_skip(struct coredump_params *cprm, size_t nr);
+bool dump_emit(struct coredump_params *cprm, const void *addr, int nr);
+bool dump_align(struct coredump_params *cprm, int align);
+bool dump_user_range(struct coredump_params *cprm, unsigned long start,
+		     unsigned long len);
+void vfs_coredump(const kernel_siginfo_t *siginfo);
 
 /*
  * Logging for the coredump code, ratelimited.

-- 
2.53.0

[PATCH v2 11/22] coredump: always chunk writes

[Christian Brauner]

Right now dump_emit() is the only coredump helper that writes buffers
larger than a page in one call. For elf notes that can easily blow past
PAGE_SIZE. That's annoying because neither pipes nor af_unix sockets
take such writes in one piece.

If a signal arrives while the writer is waiting they drop a short write.
With the coredump records work coming up that means header and its data
are desynchronized. A write that fits in one pipe buffer or one skb
doesn't suffer from this.

So split all writes up, including elf notes, and cap every write at a
page. The coredump socket already raises sk_sndbuf far enough for a page
to fit a single skb and pipes always work that way.

That means dump_interrupted() is now checked once per page. So a large
coredump stops earlier (good). An empty write no longer issues a
zero-length write. The rlimit core check stays where it was. It
continues refusing whole writes.

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

diff --git a/fs/coredump.c b/fs/coredump.c
index d837819031ff..d61f36239f91 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -1215,19 +1215,21 @@ 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 bool __dump_emit(struct coredump_params *cprm, const void *addr, int nr)
+/* One write, never more than a page. See __dump_emit(). */
+static bool dump_emit_chunk(struct coredump_params *cprm, const void *addr,
+			    int nr)
 {
 	struct file *file = cprm->file;
 	loff_t pos = file->f_pos;
 	ssize_t n;
 
-	if (cprm->written + nr > cprm->limit)
-		return false;
 	if (dump_interrupted())
 		return false;
+
 	n = __kernel_write(file, addr, nr, &pos);
 	if (n != nr)
 		return false;
+
 	file->f_pos = pos;
 	cprm->written += n;
 	cprm->pos += n;
@@ -1235,6 +1237,24 @@ static bool __dump_emit(struct coredump_params *cprm, const void *addr, int nr)
 	return true;
 }
 
+static bool __dump_emit(struct coredump_params *cprm, const void *addr, int nr)
+{
+	if (cprm->written + nr > cprm->limit)
+		return false;
+
+	while (nr) {
+		int chunk = min_t(int, nr, PAGE_SIZE);
+
+		if (!dump_emit_chunk(cprm, addr, chunk))
+			return false;
+
+		addr += chunk;
+		nr -= chunk;
+	}
+
+	return true;
+}
+
 static bool __dump_skip(struct coredump_params *cprm, size_t nr)
 {
 	static char zeroes[PAGE_SIZE];
@@ -1247,13 +1267,16 @@ static bool __dump_skip(struct coredump_params *cprm, size_t nr)
 		return true;
 	}
 
-	while (nr > PAGE_SIZE) {
-		if (!__dump_emit(cprm, zeroes, PAGE_SIZE))
+	while (nr) {
+		size_t chunk = min_t(size_t, nr, PAGE_SIZE);
+
+		if (!__dump_emit(cprm, zeroes, chunk))
 			return false;
-		nr -= PAGE_SIZE;
+
+		nr -= chunk;
 	}
 
-	return __dump_emit(cprm, zeroes, nr);
+	return true;
 }
 
 /* Flush the accumulated hole before writing data. */

-- 
2.53.0

[PATCH v2 12/22] coredump: clean up coredump state handling

[Christian Brauner]

Right now coredump state handling is messy. The binfmt->core_dump::
methods return 1 when the coredump method did anything at all which
means that a partial write counts as having dumped core. This is fine as
the state is really only used to indicate that a coredump event occurred
in the exit status of the task. That should obviously be indicated even
if the actual writeout of the coredump failed.

But it means the coredump method of the binary formats is different from
all the other coredump helpers. And there's no way to communicate to
userspace that a coredump was truncated. We'll add support for that in a
second. For now, clean this up.

Add a flag member into struct coredump_params. Let the coredump method
raise COREDUMP_STATE_STARTED. This is what coredump_finish() will end up
using to splice in the coredump bit into the exit status.

This allows us to let the return value mean success or failure and align
it with the other coredump helpers. We also start raising
COREDUMP_STATE_TRUNCATED. This will be used in the next patches to
communicate truncation to userspace via the coredump socket.

No functional changes.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/binfmt_elf.c          | 12 +++++++-----
 fs/binfmt_elf_fdpic.c    | 12 +++++++-----
 fs/coredump.c            | 31 +++++++++++++++++--------------
 include/linux/binfmts.h  |  3 ++-
 include/linux/coredump.h | 12 ++++++++++++
 5 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 16a56b6b3f6c..f85b2137bdb9 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -74,7 +74,7 @@ static int load_elf_binary(struct linux_binprm *bprm);
  * don't even try.
  */
 #ifdef CONFIG_ELF_CORE
-static int elf_core_dump(struct coredump_params *cprm);
+static bool elf_core_dump(struct coredump_params *cprm);
 #else
 #define elf_core_dump	NULL
 #endif
@@ -1990,9 +1990,9 @@ static void fill_extnum_info(struct elfhdr *elf, struct elf_shdr *shdr4extnum,
  * and then they are actually written out.  If we run out of core limit
  * we just truncate.
  */
-static int elf_core_dump(struct coredump_params *cprm)
+static bool elf_core_dump(struct coredump_params *cprm)
 {
-	int has_dumped = 0;
+	bool ret = false;
 	int segs, i;
 	struct elfhdr elf;
 	loff_t offset = 0, dataoff;
@@ -2023,7 +2023,7 @@ static int elf_core_dump(struct coredump_params *cprm)
 	if (!fill_note_info(&elf, e_phnum, &info, cprm))
 		goto end_coredump;
 
-	has_dumped = 1;
+	cprm->state |= COREDUMP_STATE_STARTED;
 
 	offset += sizeof(elf);				/* ELF header */
 	offset += segs * sizeof(struct elf_phdr);	/* Program headers */
@@ -2118,11 +2118,13 @@ static int elf_core_dump(struct coredump_params *cprm)
 			goto end_coredump;
 	}
 
+	ret = true;
+
 end_coredump:
 	free_note_info(&info);
 	kfree(shdr4extnum);
 	kfree(phdr4note);
-	return has_dumped;
+	return ret;
 }
 
 #endif		/* CONFIG_ELF_CORE */
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index fe0b5c5ed2bc..252eb155965b 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -75,7 +75,7 @@ static int elf_fdpic_map_file_by_direct_mmap(struct elf_fdpic_params *,
 					     struct file *, struct mm_struct *);
 
 #ifdef CONFIG_ELF_CORE
-static int elf_fdpic_core_dump(struct coredump_params *cprm);
+static bool elf_fdpic_core_dump(struct coredump_params *cprm);
 #endif
 
 static struct linux_binfmt elf_fdpic_format = {
@@ -1476,9 +1476,9 @@ static bool elf_fdpic_dump_segments(struct coredump_params *cprm,
  * and then they are actually written out.  If we run out of core limit
  * we just truncate.
  */
-static int elf_fdpic_core_dump(struct coredump_params *cprm)
+static bool elf_fdpic_core_dump(struct coredump_params *cprm)
 {
-	int has_dumped = 0;
+	bool ret = false;
 	int segs;
 	int i;
 	struct elfhdr *elf = NULL;
@@ -1535,7 +1535,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 	/* Set up header */
 	fill_elf_fdpic_header(elf, e_phnum);
 
-	has_dumped = 1;
+	cprm->state |= COREDUMP_STATE_STARTED;
 	/*
 	 * Set up the notes in similar form to SVR4 core dumps made
 	 * with info from their /proc.
@@ -1655,6 +1655,8 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 		       cprm->file->f_pos, offset);
 	}
 
+	ret = true;
+
 end_coredump:
 	while (thread_list) {
 		tmp = thread_list;
@@ -1665,7 +1667,7 @@ static int elf_fdpic_core_dump(struct coredump_params *cprm)
 	kfree(elf);
 	kfree(psinfo);
 	kfree(shdr4extnum);
-	return has_dumped;
+	return ret;
 }
 
 #endif		/* CONFIG_ELF_CORE */
diff --git a/fs/coredump.c b/fs/coredump.c
index d61f36239f91..6af3ff0e19a6 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -98,7 +98,6 @@ struct core_name {
 	char *corename __counted_by_ptr(size);
 	int used, size;
 	unsigned int core_pipe_limit;
-	bool core_dumped;
 	enum coredump_type_t core_type;
 };
 
@@ -250,7 +249,6 @@ static bool coredump_parse(struct core_name *cn, struct coredump_params *cprm,
 	cn->used = 0;
 	cn->corename = NULL;
 	cn->core_pipe_limit = 0;
-	cn->core_dumped = false;
 	if (*pat_ptr == '|')
 		cn->core_type = COREDUMP_PIPE;
 	else if (*pat_ptr == '@')
@@ -549,13 +547,13 @@ static int coredump_wait(int exit_code, struct core_state *core_state)
 	return core_waiters;
 }
 
-static void coredump_finish(bool core_dumped)
+static void coredump_finish(enum coredump_state state)
 {
 	struct core_thread *curr, *next;
 	struct task_struct *task;
 
 	spin_lock_irq(&current->sighand->siglock);
-	if (core_dumped && !__fatal_signal_pending(current))
+	if ((state & COREDUMP_STATE_STARTED) && !__fatal_signal_pending(current))
 		current->signal->group_exit_code |= 0x80;
 	next = current->signal->core_state->dumper.next;
 	current->signal->core_state = NULL;
@@ -1045,19 +1043,23 @@ static bool coredump_pipe(struct core_name *cn, struct coredump_params *cprm,
 	return true;
 }
 
-static bool coredump_write(struct core_name *cn,
-			  struct coredump_params *cprm,
-			  const struct linux_binfmt *binfmt)
+static bool coredump_write(struct coredump_params *cprm,
+			   const struct linux_binfmt *binfmt)
 {
 
-	if (dump_interrupted())
+	if (dump_interrupted()) {
+		cprm->state |= COREDUMP_STATE_TRUNCATED;
 		return true;
+	}
 
-	if (!dump_vma_snapshot(cprm))
+	if (!dump_vma_snapshot(cprm)) {
+		cprm->state |= COREDUMP_STATE_TRUNCATED;
 		return false;
+	}
 
 	file_start_write(cprm->file);
-	cn->core_dumped = binfmt->core_dump(cprm);
+	if (!binfmt->core_dump(cprm))
+		cprm->state |= COREDUMP_STATE_TRUNCATED;
 	/*
 	 * Ensures that file size is big enough to contain the current
 	 * file postion. This prevents gdb from complaining about
@@ -1066,7 +1068,8 @@ static bool coredump_write(struct core_name *cn,
 	 */
 	if (cprm->to_skip) {
 		cprm->to_skip--;
-		dump_emit(cprm, "", 1);
+		if (!dump_emit(cprm, "", 1))
+			cprm->state |= COREDUMP_STATE_TRUNCATED;
 	}
 	file_end_write(cprm->file);
 	free_vma_snapshot(cprm);
@@ -1082,7 +1085,7 @@ static void coredump_cleanup(struct core_name *cn, struct coredump_params *cprm)
 		atomic_dec(&core_pipe_count);
 	}
 	kfree(cn->corename);
-	coredump_finish(cn->core_dumped);
+	coredump_finish(cprm->state);
 }
 
 static inline bool coredump_skip(const struct coredump_params *cprm,
@@ -1136,14 +1139,14 @@ static void do_coredump(struct core_name *cn, struct coredump_params *cprm,
 	if (unshare_files())
 		return;
 
-	if ((cprm->mask & COREDUMP_KERNEL) && !coredump_write(cn, cprm, binfmt))
+	if ((cprm->mask & COREDUMP_KERNEL) && !coredump_write(cprm, binfmt))
 		return;
 
 	coredump_sock_shutdown(cprm->file);
 
 	/* Let the parent know that a coredump was generated. */
 	if (cprm->mask & COREDUMP_USERSPACE)
-		cn->core_dumped = true;
+		cprm->state |= COREDUMP_STATE_STARTED;
 
 	/*
 	 * When core_pipe_limit is set we wait for the coredump server
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 2c77e383e737..64f82fb642bf 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -94,7 +94,8 @@ struct linux_binfmt {
 	struct module *module;
 	int (*load_binary)(struct linux_binprm *);
 #ifdef CONFIG_COREDUMP
-	int (*core_dump)(struct coredump_params *cprm);
+	/* Returns true if the whole coredump was written. */
+	bool (*core_dump)(struct coredump_params *cprm);
 	unsigned long min_coredump;	/* minimal dump size */
 #endif
 } __randomize_layout;
diff --git a/include/linux/coredump.h b/include/linux/coredump.h
index 943bddfb22bf..709388dd5659 100644
--- a/include/linux/coredump.h
+++ b/include/linux/coredump.h
@@ -9,6 +9,16 @@
 #include <asm/siginfo.h>
 
 #ifdef CONFIG_COREDUMP
+/**
+ * enum coredump_state - what happened while the coredump was written
+ * @COREDUMP_STATE_STARTED: the dumper committed to writing a coredump
+ * @COREDUMP_STATE_TRUNCATED: the dumper stopped before it had written all of it
+ */
+enum coredump_state {
+	COREDUMP_STATE_STARTED		= (1U << 0),
+	COREDUMP_STATE_TRUNCATED	= (1U << 1),
+};
+
 struct core_vma_metadata {
 	unsigned long start, end;
 	vm_flags_t flags;
@@ -28,6 +38,8 @@ struct coredump_params {
 	int cpu;
 	/* COREDUMP_* options negotiated with the coredump server. */
 	u64 mask;
+	/* COREDUMP_STATE_* raised while the coredump is written. */
+	enum coredump_state state;
 	loff_t written;
 	loff_t pos;
 	loff_t to_skip;

-- 
2.53.0

[PATCH v2 13/22] coredump: add COREDUMP_RECORDS to the coredump socket protocol

[Christian Brauner]

Currently a coredump sent over a socket is raw data. The kernel knows
things about the data it's sending that are useful for a coredump
server. For example, it knows where the unpopulated parts of a mapping
are. We can't communicate this to userspace currently though.

Add a COREDUMP_RECORDS feature bit and a struct coredump_record_header
so userspace can negotiate that feature. Instead of a byte stream it
gets a header plus data. Reassembling the records yields the same
coredump that would have been sent without them.

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

A record stream ends explicitly. A COREDUMP_RECORD_END record closes it,
carries no data and reports the size of the coredump. It is only sent
once the whole coredump has been written. If it's missing the coredump
should be treated as truncated.

This just adds the infrastructure.

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

diff --git a/include/uapi/linux/coredump.h b/include/uapi/linux/coredump.h
index 662e0468da6e..0bd5c8662ebe 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_RECORDS: send the coredump as a sequence of records instead of
+ *                    as a plain byte stream, see struct coredump_record_header;
+ *                    requires COREDUMP_KERNEL
  */
 enum {
 	COREDUMP_KERNEL		= (1ULL << 0),
 	COREDUMP_USERSPACE	= (1ULL << 1),
 	COREDUMP_REJECT		= (1ULL << 2),
 	COREDUMP_WAIT		= (1ULL << 3),
+	COREDUMP_RECORDS	= (1ULL << 4),
 };
 
 /**
@@ -101,4 +105,66 @@ enum coredump_mark {
 	__COREDUMP_MARK_MAX		= (1U << 31),
 };
 
+/**
+ * enum coredump_record_type - Type of a coredump record
+ *
+ * @COREDUMP_RECORD_DATA: the header is followed by ->len bytes of data
+ * @COREDUMP_RECORD_END: the coredump ends here, the header is not followed
+ *                       by any data and no further record is sent
+ * @__COREDUMP_RECORD_TYPE_MAX: the maximum coredump record type value
+ */
+enum coredump_record_type {
+	COREDUMP_RECORD_DATA		= 0U,
+	COREDUMP_RECORD_END		= 1U,
+	__COREDUMP_RECORD_TYPE_MAX	= (1U << 31),
+};
+
+/**
+ * struct coredump_record_header - header of a coredump record
+ * @size: size of struct coredump_record_header
+ * @type: one of enum coredump_record_type
+ * @flags: modifiers for this record
+ * @offset: offset in the coredump this record starts at
+ * @len: number of coredump bytes this record accounts for
+ *
+ * If the coredump server raises COREDUMP_RECORDS in coredump_ack->mask
+ * the kernel doesn't send the coredump as a plain byte stream. It sends
+ * a sequence of records instead. A COREDUMP_RECORD_DATA record is
+ * followed by @len bytes of actual coredump data. Records arrive in
+ * order and leave no gaps. So @offset is the sum of the @len of all
+ * records before it.
+ *
+ * The last record is a COREDUMP_RECORD_END record. It is followed by
+ * nothing. Its @len is zero. Its @offset is the size of the coredump.
+ * The kernel only sends it once it has written the whole coredump. A
+ * server that hits end-of-file without having seen an end record must
+ * treat the coredump as incomplete.
+ *
+ * The @size member is set to the size of struct coredump_record_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. It must refuse a @size smaller than
+ * COREDUMP_RECORD_HEADER_SIZE_VER0. @size covers the header alone.
+ * @offset and @len count coredump bytes.
+ *
+ * The @flags member carries modifiers that change how the record is to
+ * be interpreted. No flag is defined yet. Userspace must refuse a
+ * record carrying a flag or a type it doesn't know. Every new record
+ * type is raised in coredump_req->mask as a feature of its own. A
+ * server only ever sees the types it asked for.
+ *
+ * COREDUMP_RECORDS must be combined with COREDUMP_KERNEL.
+ */
+struct coredump_record_header {
+	__u32 size;
+	__u32 type;
+	__u64 flags;
+	__u64 offset;
+	__u64 len;
+};
+
+enum {
+	COREDUMP_RECORD_HEADER_SIZE_VER0 = 32U, /* size of first published struct */
+};
+
 #endif /* _UAPI_LINUX_COREDUMP_H */

-- 
2.53.0

[PATCH v2 14/22] coredump: add COREDUMP_SPARSE to the coredump socket protocol

[Christian Brauner]

A coredump with a lot of unpopulated mappings sends useless 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_RECORDS put the zeroes in records but it didn't get rid
of them.

Add a COREDUMP_SPARSE feature bit and a COREDUMP_RECORD_ZERO record
type. A zero record is a bare header that tells userspace how many zero
bytes were skipped.

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

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

diff --git a/include/uapi/linux/coredump.h b/include/uapi/linux/coredump.h
index 0bd5c8662ebe..f3771861ca48 100644
--- a/include/uapi/linux/coredump.h
+++ b/include/uapi/linux/coredump.h
@@ -14,6 +14,8 @@
  * @COREDUMP_RECORDS: send the coredump as a sequence of records instead of
  *                    as a plain byte stream, see struct coredump_record_header;
  *                    requires COREDUMP_KERNEL
+ * @COREDUMP_SPARSE: describe the holes in the coredump as zero records
+ *                   instead of transferring them; requires COREDUMP_RECORDS
  */
 enum {
 	COREDUMP_KERNEL		= (1ULL << 0),
@@ -21,6 +23,7 @@ enum {
 	COREDUMP_REJECT		= (1ULL << 2),
 	COREDUMP_WAIT		= (1ULL << 3),
 	COREDUMP_RECORDS	= (1ULL << 4),
+	COREDUMP_SPARSE		= (1ULL << 5),
 };
 
 /**
@@ -111,11 +114,14 @@ enum coredump_mark {
  * @COREDUMP_RECORD_DATA: the header is followed by ->len bytes of data
  * @COREDUMP_RECORD_END: the coredump ends here, the header is not followed
  *                       by any data and no further record is sent
+ * @COREDUMP_RECORD_ZERO: the header stands for ->len zero bytes and is not
+ *                        followed by any data
  * @__COREDUMP_RECORD_TYPE_MAX: the maximum coredump record type value
  */
 enum coredump_record_type {
 	COREDUMP_RECORD_DATA		= 0U,
 	COREDUMP_RECORD_END		= 1U,
+	COREDUMP_RECORD_ZERO		= 2U,
 	__COREDUMP_RECORD_TYPE_MAX	= (1U << 31),
 };
 
@@ -130,9 +136,11 @@ enum coredump_record_type {
  * If the coredump server raises COREDUMP_RECORDS in coredump_ack->mask
  * the kernel doesn't send the coredump as a plain byte stream. It sends
  * a sequence of records instead. A COREDUMP_RECORD_DATA record is
- * followed by @len bytes of actual coredump data. Records arrive in
- * order and leave no gaps. So @offset is the sum of the @len of all
- * records before it.
+ * followed by @len bytes of actual coredump data. A
+ * COREDUMP_RECORD_ZERO record is followed by nothing and stands for
+ * @len zero bytes. A server that didn't raise COREDUMP_SPARSE never
+ * sees a zero record. Records arrive in order and leave no gaps. So
+ * @offset is the sum of the @len of all records before it.
  *
  * The last record is a COREDUMP_RECORD_END record. It is followed by
  * nothing. Its @len is zero. Its @offset is the size of the coredump.
@@ -153,7 +161,8 @@ enum coredump_record_type {
  * type is raised in coredump_req->mask as a feature of its own. A
  * server only ever sees the types it asked for.
  *
- * COREDUMP_RECORDS must be combined with COREDUMP_KERNEL.
+ * COREDUMP_RECORDS must be combined with COREDUMP_KERNEL, and
+ * COREDUMP_SPARSE with COREDUMP_RECORDS.
  */
 struct coredump_record_header {
 	__u32 size;

-- 
2.53.0

[PATCH v2 15/22] 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 | 79 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 77 insertions(+), 2 deletions(-)

diff --git a/tools/include/uapi/linux/coredump.h b/tools/include/uapi/linux/coredump.h
index dc3789b78af0..f3771861ca48 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_RECORDS: send the coredump as a sequence of records instead of
+ *                    as a plain byte stream, see struct coredump_record_header;
+ *                    requires COREDUMP_KERNEL
+ * @COREDUMP_SPARSE: describe the holes in the coredump as zero records
+ *                   instead of transferring them; requires COREDUMP_RECORDS
  */
 enum {
 	COREDUMP_KERNEL		= (1ULL << 0),
 	COREDUMP_USERSPACE	= (1ULL << 1),
 	COREDUMP_REJECT		= (1ULL << 2),
 	COREDUMP_WAIT		= (1ULL << 3),
+	COREDUMP_RECORDS	= (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,72 @@ enum coredump_mark {
 	__COREDUMP_MARK_MAX		= (1U << 31),
 };
 
+/**
+ * enum coredump_record_type - Type of a coredump record
+ *
+ * @COREDUMP_RECORD_DATA: the header is followed by ->len bytes of data
+ * @COREDUMP_RECORD_END: the coredump ends here, the header is not followed
+ *                       by any data and no further record is sent
+ * @COREDUMP_RECORD_ZERO: the header stands for ->len zero bytes and is not
+ *                        followed by any data
+ * @__COREDUMP_RECORD_TYPE_MAX: the maximum coredump record type value
+ */
+enum coredump_record_type {
+	COREDUMP_RECORD_DATA		= 0U,
+	COREDUMP_RECORD_END		= 1U,
+	COREDUMP_RECORD_ZERO		= 2U,
+	__COREDUMP_RECORD_TYPE_MAX	= (1U << 31),
+};
+
+/**
+ * struct coredump_record_header - header of a coredump record
+ * @size: size of struct coredump_record_header
+ * @type: one of enum coredump_record_type
+ * @flags: modifiers for this record
+ * @offset: offset in the coredump this record starts at
+ * @len: number of coredump bytes this record accounts for
+ *
+ * If the coredump server raises COREDUMP_RECORDS in coredump_ack->mask
+ * the kernel doesn't send the coredump as a plain byte stream. It sends
+ * a sequence of records instead. A COREDUMP_RECORD_DATA record is
+ * followed by @len bytes of actual coredump data. A
+ * COREDUMP_RECORD_ZERO record is followed by nothing and stands for
+ * @len zero bytes. A server that didn't raise COREDUMP_SPARSE never
+ * sees a zero record. Records arrive in order and leave no gaps. So
+ * @offset is the sum of the @len of all records before it.
+ *
+ * The last record is a COREDUMP_RECORD_END record. It is followed by
+ * nothing. Its @len is zero. Its @offset is the size of the coredump.
+ * The kernel only sends it once it has written the whole coredump. A
+ * server that hits end-of-file without having seen an end record must
+ * treat the coredump as incomplete.
+ *
+ * The @size member is set to the size of struct coredump_record_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. It must refuse a @size smaller than
+ * COREDUMP_RECORD_HEADER_SIZE_VER0. @size covers the header alone.
+ * @offset and @len count coredump bytes.
+ *
+ * The @flags member carries modifiers that change how the record is to
+ * be interpreted. No flag is defined yet. Userspace must refuse a
+ * record carrying a flag or a type it doesn't know. Every new record
+ * type is raised in coredump_req->mask as a feature of its own. A
+ * server only ever sees the types it asked for.
+ *
+ * COREDUMP_RECORDS must be combined with COREDUMP_KERNEL, and
+ * COREDUMP_SPARSE with COREDUMP_RECORDS.
+ */
+struct coredump_record_header {
+	__u32 size;
+	__u32 type;
+	__u64 flags;
+	__u64 offset;
+	__u64 len;
+};
+
+enum {
+	COREDUMP_RECORD_HEADER_SIZE_VER0 = 32U, /* size of first published struct */
+};
+
 #endif /* _UAPI_LINUX_COREDUMP_H */

-- 
2.53.0

[PATCH v2 16/22] coredump: send the coredump in records if requested

[Christian Brauner]

When the coredump server raises COREDUMP_RECORDS send the coredump in
records. A record consists of a struct coredump_record_header and data.
A header and the bytes it describes go out in one iovec.

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

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

diff --git a/fs/coredump.c b/fs/coredump.c
index 6af3ff0e19a6..b1679930094c 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>
@@ -68,6 +67,7 @@
 
 static bool dump_vma_snapshot(struct coredump_params *cprm);
 static void free_vma_snapshot(struct coredump_params *cprm);
+static void dump_end_record(struct coredump_params *cprm);
 
 #define CORE_FILE_NOTE_SIZE_DEFAULT (4*1024*1024)
 /* Define a reasonable max cap */
@@ -661,6 +661,8 @@ static int umh_coredump_setup(struct subprocess_info *info, struct cred *new)
 	return 0;
 }
 
+static_assert(sizeof(struct coredump_record_header) == COREDUMP_RECORD_HEADER_SIZE_VER0);
+
 #ifdef CONFIG_UNIX
 /* af_unix halves the send buffer to size a single skb. */
 #define COREDUMP_SOCK_SNDBUF_MIN (3 * PAGE_SIZE)
@@ -803,7 +805,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_RECORDS,
 		.size_ack	= sizeof(struct coredump_ack),
 	};
 	struct coredump_ack ack = {};
@@ -857,6 +860,19 @@ static bool coredump_sock_request(struct core_name *cn, struct coredump_params *
 		return false;
 	}
 
+	/* Records only describe a coredump the kernel writes. */
+	if ((ack.mask & COREDUMP_RECORDS) && !(ack.mask & COREDUMP_KERNEL)) {
+		coredump_sock_mark(cprm->file, COREDUMP_MARK_CONFLICTING);
+		return false;
+	}
+
+	/* Record header scratch; a bvec can't point at the stack. */
+	if (ack.mask & COREDUMP_RECORDS) {
+		cprm->record_hdr = kmalloc_obj(*cprm->record_hdr);
+		if (!cprm->record_hdr)
+			return false;
+	}
+
 	cprm->mask = ack.mask;
 	return coredump_sock_mark(cprm->file, COREDUMP_MARK_REQACK);
 }
@@ -1046,7 +1062,6 @@ static bool coredump_pipe(struct core_name *cn, struct coredump_params *cprm,
 static bool coredump_write(struct coredump_params *cprm,
 			   const struct linux_binfmt *binfmt)
 {
-
 	if (dump_interrupted()) {
 		cprm->state |= COREDUMP_STATE_TRUNCATED;
 		return true;
@@ -1062,15 +1077,17 @@ static bool coredump_write(struct coredump_params *cprm,
 		cprm->state |= COREDUMP_STATE_TRUNCATED;
 	/*
 	 * 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 record stream relies on it too: the flush
+	 * emits the records that cover a trailing hole.
 	 */
 	if (cprm->to_skip) {
 		cprm->to_skip--;
 		if (!dump_emit(cprm, "", 1))
 			cprm->state |= COREDUMP_STATE_TRUNCATED;
 	}
+	dump_end_record(cprm);
 	file_end_write(cprm->file);
 	free_vma_snapshot(cprm);
 	return true;
@@ -1085,6 +1102,7 @@ static void coredump_cleanup(struct core_name *cn, struct coredump_params *cprm)
 		atomic_dec(&core_pipe_count);
 	}
 	kfree(cn->corename);
+	kfree(cprm->record_hdr);
 	coredump_finish(cprm->state);
 }
 
@@ -1218,26 +1236,74 @@ void vfs_coredump(const kernel_siginfo_t *siginfo)
  * do on a core-file: use only these functions to write out all the
  * necessary info.
  */
-/* One write, never more than a page. See __dump_emit(). */
-static bool dump_emit_chunk(struct coredump_params *cprm, const void *addr,
-			    int nr)
+static bool dump_records(const struct coredump_params *cprm)
+{
+	return cprm->mask & COREDUMP_RECORDS;
+}
+
+/* Describe the next @len bytes of the coredump. Returns the header size. */
+static size_t dump_record_init(struct coredump_params *cprm,
+			       enum coredump_record_type type, u64 flags,
+			       u64 len)
+{
+	if (!dump_records(cprm))
+		return 0;
+
+	*cprm->record_hdr = (struct coredump_record_header) {
+		.size	= sizeof(*cprm->record_hdr),
+		.type	= type,
+		.flags	= flags,
+		.offset	= cprm->pos,
+		.len	= len,
+	};
+
+	return sizeof(*cprm->record_hdr);
+}
+
+/* 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;
 
-	if (dump_interrupted())
+	n = __kernel_write_iter(file, iter, &pos);
+	if (n != (ssize_t)count)
 		return false;
+	file->f_pos = pos;
+	cprm->written += count;
+	cprm->pos += len;
+
+	return true;
+}
+
+/* One record, never more than a page. See __dump_emit(). */
+static bool dump_emit_chunk(struct coredump_params *cprm, const void *addr,
+			    int nr)
+{
+	struct kvec kvec[2];
+	struct iov_iter iter;
+	unsigned int nseg = 0;
+	size_t hdrlen;
 
-	n = __kernel_write(file, addr, nr, &pos);
-	if (n != nr)
+	if (dump_interrupted())
 		return false;
 
-	file->f_pos = pos;
-	cprm->written += n;
-	cprm->pos += n;
+	hdrlen = dump_record_init(cprm, COREDUMP_RECORD_DATA, 0, nr);
+	if (hdrlen) {
+		kvec[nseg].iov_base = cprm->record_hdr;
+		kvec[nseg].iov_len = hdrlen;
+		nseg++;
+	}
+	kvec[nseg].iov_base = (void *)addr;
+	kvec[nseg].iov_len = nr;
+	nseg++;
 
-	return true;
+	iov_iter_kvec(&iter, ITER_SOURCE, kvec, nseg, hdrlen + nr);
+
+	return dump_write_iter(cprm, &iter, nr);
 }
 
 static bool __dump_emit(struct coredump_params *cprm, const void *addr, int nr)
@@ -1258,6 +1324,34 @@ static bool __dump_emit(struct coredump_params *cprm, const void *addr, int nr)
 	return true;
 }
 
+/* Send a record that stands on its own: a header and nothing else. */
+static bool dump_emit_record(struct coredump_params *cprm,
+			     enum coredump_record_type type, u64 flags, u64 len)
+{
+	struct kvec kvec;
+	struct iov_iter iter;
+	size_t hdrlen;
+
+	hdrlen = dump_record_init(cprm, type, flags, len);
+	if (!hdrlen)
+		return false;
+
+	kvec.iov_base = cprm->record_hdr;
+	kvec.iov_len = hdrlen;
+	iov_iter_kvec(&iter, ITER_SOURCE, &kvec, 1, hdrlen);
+
+	return dump_write_iter(cprm, &iter, len);
+}
+
+/* Close the record stream. Only a whole coredump gets an end record. */
+static void dump_end_record(struct coredump_params *cprm)
+{
+	if (cprm->state & COREDUMP_STATE_TRUNCATED)
+		return;
+
+	dump_emit_record(cprm, COREDUMP_RECORD_END, 0, 0);
+}
+
 static bool __dump_skip(struct coredump_params *cprm, size_t nr)
 {
 	static char zeroes[PAGE_SIZE];
@@ -1318,11 +1412,10 @@ EXPORT_SYMBOL(dump_skip);
 #ifdef CONFIG_ELF_CORE
 static bool 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 hdrlen;
 
 	if (!page)
 		return false;
@@ -1333,17 +1426,16 @@ static bool dump_emit_page(struct coredump_params *cprm, struct page *page)
 		return false;
 	if (dump_interrupted())
 		return false;
-	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 false;
-	file->f_pos = pos;
-	cprm->written += PAGE_SIZE;
-	cprm->pos += PAGE_SIZE;
 
-	return true;
+	/* Hand the record header to the same write as the page it describes. */
+	hdrlen = dump_record_init(cprm, COREDUMP_RECORD_DATA, 0, PAGE_SIZE);
+	if (hdrlen)
+		bvec_set_virt(&bvec[nseg++], cprm->record_hdr, hdrlen);
+	bvec_set_page(&bvec[nseg++], page, PAGE_SIZE, 0);
+
+	iov_iter_bvec(&iter, ITER_SOURCE, bvec, nseg, hdrlen + PAGE_SIZE);
+
+	return dump_write_iter(cprm, &iter, PAGE_SIZE);
 }
 
 /*
diff --git a/include/linux/coredump.h b/include/linux/coredump.h
index 709388dd5659..b252bb2843b3 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
@@ -40,7 +41,11 @@ struct coredump_params {
 	u64 mask;
 	/* COREDUMP_STATE_* raised while the coredump is written. */
 	enum coredump_state state;
+	/* Record header scratch, NULL unless the coredump is a record stream. */
+	struct coredump_record_header *record_hdr;
+	/* Bytes handed to the file, record headers included. */
 	loff_t written;
+	/* Offset in the coredump, record 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 570fc2e005c2..1c8658f35735 100644
--- a/tools/testing/selftests/coredump/coredump_test_helpers.c
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.c
@@ -275,7 +275,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 TEST_REQ_MASK_ALL					\
 	(COREDUMP_KERNEL | COREDUMP_USERSPACE |			\
-	 COREDUMP_REJECT | COREDUMP_WAIT)
+	 COREDUMP_REJECT | COREDUMP_WAIT | COREDUMP_RECORDS)
 
 bool check_coredump_req(const struct coredump_req *req)
 {

-- 
2.53.0

[PATCH v2 17/22] coredump: describe the holes when COREDUMP_SPARSE is negotiated

[Christian Brauner]

Make use of COREDUMP_SPARSE. Refuse it without COREDUMP_RECORDS.

Actual holes are sent as a record with length indicating how much zero
data there was.

coredump_write() flushes a trailing hole if the coredump is done.
Instead of writing the actual byte for pipes and sockets, collapse it.
This stops wasting a header with coredump records for a single byte. So
we now only write it when the coredump can be seeked. TL;DR a trailing
hole is a zero record like any other and the records still cover the
whole coredump.

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

diff --git a/fs/coredump.c b/fs/coredump.c
index b1679930094c..7b568d25887c 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -68,6 +68,7 @@
 static bool dump_vma_snapshot(struct coredump_params *cprm);
 static void free_vma_snapshot(struct coredump_params *cprm);
 static void dump_end_record(struct coredump_params *cprm);
+static bool dump_flush_skip(struct coredump_params *cprm);
 
 #define CORE_FILE_NOTE_SIZE_DEFAULT (4*1024*1024)
 /* Define a reasonable max cap */
@@ -806,7 +807,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_RECORDS,
+				  COREDUMP_RECORDS | COREDUMP_SPARSE,
 		.size_ack	= sizeof(struct coredump_ack),
 	};
 	struct coredump_ack ack = {};
@@ -866,6 +867,12 @@ static bool coredump_sock_request(struct core_name *cn, struct coredump_params *
 		return false;
 	}
 
+	/* Zero records only exist inside a record stream. */
+	if ((ack.mask & COREDUMP_SPARSE) && !(ack.mask & COREDUMP_RECORDS)) {
+		coredump_sock_mark(cprm->file, COREDUMP_MARK_CONFLICTING);
+		return false;
+	}
+
 	/* Record header scratch; a bvec can't point at the stack. */
 	if (ack.mask & COREDUMP_RECORDS) {
 		cprm->record_hdr = kmalloc_obj(*cprm->record_hdr);
@@ -1076,15 +1083,21 @@ static bool coredump_write(struct coredump_params *cprm,
 	if (!binfmt->core_dump(cprm))
 		cprm->state |= COREDUMP_STATE_TRUNCATED;
 	/*
-	 * Ensures that file size is big enough to contain the current
-	 * file position. This prevents gdb from complaining about
-	 * a truncated file if the last "write" to the file was
-	 * dump_skip. A record stream relies on it too: the flush
-	 * emits the records that cover a trailing hole.
+	 * A trailing hole still has to land in the coredump. Seeking over
+	 * it doesn't grow the file, so the last byte of it is written
+	 * instead and gdb doesn't see a truncated file. Everything else
+	 * puts the hole on the wire as it flushes it.
 	 */
 	if (cprm->to_skip) {
-		cprm->to_skip--;
-		if (!dump_emit(cprm, "", 1))
+		bool flushed;
+
+		if (cprm->file->f_mode & FMODE_LSEEK) {
+			cprm->to_skip--;
+			flushed = dump_emit(cprm, "", 1);
+		} else {
+			flushed = dump_flush_skip(cprm);
+		}
+		if (!flushed)
 			cprm->state |= COREDUMP_STATE_TRUNCATED;
 	}
 	dump_end_record(cprm);
@@ -1241,6 +1254,11 @@ static bool dump_records(const struct coredump_params *cprm)
 	return cprm->mask & COREDUMP_RECORDS;
 }
 
+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_record_init(struct coredump_params *cprm,
 			       enum coredump_record_type type, u64 flags,
@@ -1357,6 +1375,13 @@ static bool __dump_skip(struct coredump_params *cprm, size_t nr)
 	static char zeroes[PAGE_SIZE];
 	struct file *file = cprm->file;
 
+	if (dump_sparse(cprm)) {
+		/* Hand the server the length of the hole instead of the hole itself. */
+		if (dump_interrupted())
+			return false;
+		return dump_emit_record(cprm, COREDUMP_RECORD_ZERO, 0, nr);
+	}
+
 	if (file->f_mode & FMODE_LSEEK) {
 		if (dump_interrupted() || vfs_llseek(file, nr, SEEK_CUR) < 0)
 			return false;
diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.c b/tools/testing/selftests/coredump/coredump_test_helpers.c
index 1c8658f35735..a5b9cde47239 100644
--- a/tools/testing/selftests/coredump/coredump_test_helpers.c
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.c
@@ -275,7 +275,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 TEST_REQ_MASK_ALL					\
 	(COREDUMP_KERNEL | COREDUMP_USERSPACE |			\
-	 COREDUMP_REJECT | COREDUMP_WAIT | COREDUMP_RECORDS)
+	 COREDUMP_REJECT | COREDUMP_WAIT |			\
+	 COREDUMP_RECORDS | COREDUMP_SPARSE)
 
 bool check_coredump_req(const struct coredump_req *req)
 {

-- 
2.53.0

[PATCH v2 18/22] selftests/coredump: test COREDUMP_RECORDS and COREDUMP_SPARSE

[Christian Brauner]

Test the new COREDUMP_RECORDS and COREDUMP_SPARSE flags.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 .../coredump/coredump_socket_protocol_test.c       | 407 +++++++++++++++++++++
 .../selftests/coredump/coredump_test_helpers.c     | 236 +++++++++++-
 .../selftests/coredump/coredump_test_helpers.h     |   8 +
 3 files changed, 650 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..abf6e2c4c354 100644
--- a/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
+++ b/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
@@ -1573,4 +1573,411 @@ TEST_F_TIMEOUT(coredump, socket_multiple_crashing_coredumps_epoll_workers, 500)
 	wait_and_check_coredump_server(pid_coredump_server, _metadata, self);
 }
 
+/*
+ * Reassemble a record stream and check that what comes out is an ELF
+ * core file. The records themselves are validated by recv_coredump_records().
+ */
+TEST_F(coredump, socket_request_sparse_reassemble)
+{
+	int fd_core_file, 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 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_RECORDS |
+				       COREDUMP_SPARSE | COREDUMP_WAIT, 0))
+			goto out;
+
+		if (!read_marker(fd_coredump, COREDUMP_MARK_REQACK))
+			goto out;
+
+		if (recv_coredump_records(fd_coredump, fd_file, NULL, NULL, -1) < 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);
+
+	/* What the records 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_TRUE(is_elf_core(fd_core_file));
+	EXPECT_EQ(close(fd_core_file), 0);
+}
+
+/*
+ * Crash a child with a mostly-unpopulated mapping and reassemble its
+ * record stream, reporting what crossed the socket and the coredump
+ * size the records describe. With @kill_peer the server kills the task
+ * once the coredump is under way so the kernel has to cut it short.
+ */
+static void check_record_dump(struct __test_metadata *const _metadata,
+			      FIXTURE_DATA(coredump) *self, __u64 ack_mask,
+			      bool kill_peer, ssize_t *received,
+			      off_t *coredump_size)
+{
+	bool truncated = false;
+	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 = {};
+		bool is_truncated = false;
+		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_records(fd_coredump, fd_file, &size, &is_truncated,
+					    kill_peer ? fd_peer_pidfd : -1);
+		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;
+		if (write_nointr(pipefds[1], &is_truncated,
+				 sizeof(is_truncated)) != sizeof(is_truncated))
+			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_EQ(read_nointr(pipefds[0], received, sizeof(*received)),
+		  sizeof(*received));
+	ASSERT_EQ(read_nointr(pipefds[0], coredump_size, sizeof(*coredump_size)),
+		  sizeof(*coredump_size));
+	ASSERT_EQ(read_nointr(pipefds[0], &truncated, sizeof(truncated)),
+		  sizeof(truncated));
+	EXPECT_EQ(close(pipefds[0]), 0);
+
+	wait_and_check_coredump_server(pid_coredump_server, _metadata, self);
+
+	if (kill_peer) {
+		/* The kernel gave up partway, so no end record closed the stream. */
+		ASSERT_TRUE(truncated);
+		ASSERT_FALSE(WCOREDUMP(status));
+		ASSERT_LT(*coredump_size, (off_t)SPARSE_MAPPING_SIZE);
+		return;
+	}
+
+	ASSERT_FALSE(truncated);
+	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);
+
+	/* 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;
+
+	check_record_dump(_metadata, self,
+			  COREDUMP_KERNEL | COREDUMP_RECORDS |
+			  COREDUMP_SPARSE | COREDUMP_WAIT,
+			  false, &received, &coredump_size);
+
+	/* The holes didn't have to go over the socket. */
+	ASSERT_LT(received, coredump_size / 8);
+}
+
+/*
+ * COREDUMP_RECORDS alone splits the stream into records but elides
+ * nothing: the holes cross the socket as data records.
+ */
+TEST_F(coredump, socket_request_records_hole)
+{
+	off_t coredump_size = 0;
+	ssize_t received = 0;
+
+	check_record_dump(_metadata, self,
+			  COREDUMP_KERNEL | COREDUMP_RECORDS | COREDUMP_WAIT,
+			  false, &received, &coredump_size);
+
+	/* Records alone elide nothing, so everything crossed the socket. */
+	ASSERT_GT(received, coredump_size);
+}
+
+/*
+ * A coredump the kernel gives up on halfway still ends in an end record,
+ * and that record says the coredump is incomplete. COREDUMP_SPARSE is left
+ * out on purpose: the holes have to cross the socket so the coredump is
+ * far larger than the socket buffer and the kernel is still writing it
+ * when the kill lands.
+ */
+TEST_F(coredump, socket_request_records_truncated)
+{
+	off_t coredump_size = 0;
+	ssize_t received = 0;
+
+	check_record_dump(_metadata, self,
+			  COREDUMP_KERNEL | COREDUMP_RECORDS | COREDUMP_WAIT,
+			  true, &received, &coredump_size);
+
+	/* The end record crossed the socket even though the task was killed. */
+	ASSERT_GT(received, 0);
+}
+
+/* Ack @ack_mask, expect the kernel to refuse it as conflicting. */
+static void check_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_RECORDS applies to a coredump the kernel writes, nothing else. */
+TEST_F(coredump, socket_request_records_without_kernel)
+{
+	check_conflicting_ack(_metadata, self, COREDUMP_USERSPACE | COREDUMP_RECORDS);
+}
+
+/* A zero record can't exist outside a record stream. */
+TEST_F(coredump, socket_request_sparse_without_records)
+{
+	check_conflicting_ack(_metadata, self, COREDUMP_KERNEL | COREDUMP_SPARSE);
+}
+
 TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.c b/tools/testing/selftests/coredump/coredump_test_helpers.c
index a5b9cde47239..5b2ffe17f7b7 100644
--- a/tools/testing/selftests/coredump/coredump_test_helpers.c
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.c
@@ -1,9 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0
 
 #include <assert.h>
+#include <elf.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
+#include <link.h>
 #include <linux/coredump.h>
 #include <linux/fs.h>
 #include <pthread.h>
@@ -13,6 +15,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>
@@ -23,6 +26,12 @@
 
 #include "coredump_test_helpers.h"
 
+#if __ELF_NATIVE_CLASS == 64
+#define COREDUMP_ELFCLASS ELFCLASS64
+#else
+#define COREDUMP_ELFCLASS ELFCLASS32
+#endif
+
 void *do_nothing(void *arg)
 {
 	(void)arg;
@@ -44,6 +53,228 @@ 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_record_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;
+}
+
+/*
+ * Reassemble a record stream. If @fd_peer_pidfd is valid the task behind
+ * it is killed once a data record has arrived, so the kernel has to cut
+ * the coredump short with the stream already under way.
+ */
+ssize_t recv_coredump_records(int fd_coredump, int fd_core_file,
+			      off_t *coredump_size, bool *truncated,
+			      int fd_peer_pidfd)
+{
+	ssize_t received = 0;
+	off_t size = 0;
+	bool is_truncated = false;
+	bool ended = false;
+	char trailing;
+
+	while (!ended) {
+		struct coredump_record_header record = {};
+		size_t known_size;
+		ssize_t ret;
+
+		/* Peek the header size the way read_coredump_req() does. */
+		ret = recv(fd_coredump, &record, sizeof(record.size),
+			   MSG_PEEK | MSG_WAITALL);
+		if (ret == 0) {
+			/* Nothing closed the stream, so the coredump was cut short. */
+			if (truncated) {
+				is_truncated = true;
+				break;
+			}
+			fprintf(stderr, "%s: stream ended without an end record\n",
+				__func__);
+			return -1;
+		}
+		if (ret != sizeof(record.size)) {
+			fprintf(stderr, "%s: short record peek %zd: %m\n",
+				__func__, ret);
+			return -1;
+		}
+
+		if (record.size < COREDUMP_RECORD_HEADER_SIZE_VER0) {
+			fprintf(stderr, "%s: header size %u below minimum %u\n",
+				__func__, record.size,
+				COREDUMP_RECORD_HEADER_SIZE_VER0);
+			return -1;
+		}
+
+		/* Consume as much of the header as we know about. */
+		known_size = record.size < sizeof(record) ? record.size : sizeof(record);
+		ret = recv(fd_coredump, &record, known_size, MSG_WAITALL);
+		if (ret != (ssize_t)known_size) {
+			fprintf(stderr, "%s: short record read %zd: %m\n",
+				__func__, ret);
+			return -1;
+		}
+		received += ret;
+
+		/*
+		 * A flag changes what the record means, so refuse one we
+		 * don't know rather than guess.
+		 */
+		if (record.flags) {
+			fprintf(stderr, "%s: unknown header flags 0x%llx\n",
+				__func__, (unsigned long long)record.flags);
+			return -1;
+		}
+
+		/* Discard any part of the header we have no use for. */
+		ret = recv_record_bytes(fd_coredump, record.size - known_size, -1, 0);
+		if (ret < 0)
+			return -1;
+		received += ret;
+
+		/* Records are sent in order and they don't leave gaps. */
+		if (record.offset != (__u64)size) {
+			fprintf(stderr, "%s: record at %llu, expected %llu\n",
+				__func__, (unsigned long long)record.offset,
+				(unsigned long long)size);
+			return -1;
+		}
+
+		switch (record.type) {
+		case COREDUMP_RECORD_ZERO:
+			/* A hole. It comes with no data and needs none. */
+			break;
+		case COREDUMP_RECORD_DATA:
+			ret = recv_record_bytes(fd_coredump, record.len,
+						fd_core_file, size);
+			if (ret < 0)
+				return -1;
+			received += ret;
+			if (fd_peer_pidfd >= 0) {
+				if (sys_pidfd_send_signal(fd_peer_pidfd, SIGKILL,
+							  NULL, 0)) {
+					fprintf(stderr, "%s: kill failed: %m\n",
+						__func__);
+					return -1;
+				}
+				fd_peer_pidfd = -1;
+			}
+			break;
+		case COREDUMP_RECORD_END:
+			/* The coredump ends here and nothing follows it. */
+			if (record.len) {
+				fprintf(stderr, "%s: end record covers %llu bytes\n",
+					__func__,
+					(unsigned long long)record.len);
+				return -1;
+			}
+			ended = true;
+			break;
+		default:
+			fprintf(stderr, "%s: unknown record type %u\n",
+				__func__, record.type);
+			return -1;
+		}
+
+		size += record.len;
+	}
+
+	/* The end record is the last thing on the wire. */
+	if (recv(fd_coredump, &trailing, sizeof(trailing), MSG_DONTWAIT) > 0) {
+		fprintf(stderr, "%s: data after the end record\n", __func__);
+		return -1;
+	}
+
+	if (truncated)
+		*truncated = is_truncated;
+
+	/*
+	 * Nothing is written for a hole, so grow the file to the size the
+	 * records 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 %s coredump of %llu bytes\n",
+		received, is_truncated ? "truncated" : "complete",
+		(unsigned long long)size);
+	return received;
+}
+
+/* The ELF header of a native core file. */
+static bool is_core_ehdr(const ElfW(Ehdr) *ehdr)
+{
+	return !memcmp(ehdr->e_ident, ELFMAG, SELFMAG) &&
+	       ehdr->e_ident[EI_CLASS] == COREDUMP_ELFCLASS &&
+	       ehdr->e_type == ET_CORE;
+}
+
+/* Whatever the server ends up with has to be an ELF core file. */
+bool is_elf_core(int fd)
+{
+	ElfW(Ehdr) ehdr;
+
+	if (pread(fd, &ehdr, sizeof(ehdr), 0) != sizeof(ehdr)) {
+		fprintf(stderr, "%s: short read: %m\n", __func__);
+		return false;
+	}
+
+	if (!is_core_ehdr(&ehdr)) {
+		fprintf(stderr, "%s: not an ELF core file\n", __func__);
+		return false;
+	}
+
+	return true;
+}
+
 int create_detached_tmpfs(void)
 {
 	int fd_context, fd_tmpfs;
@@ -86,6 +317,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;
@@ -264,8 +496,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);
diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.h b/tools/testing/selftests/coredump/coredump_test_helpers.h
index 45904bd177b8..fe0a88a71b05 100644
--- a/tools/testing/selftests/coredump/coredump_test_helpers.h
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.h
@@ -15,9 +15,17 @@
 
 #define NUM_THREAD_SPAWN 128
 
+/* Size of the mostly unpopulated mapping the sparse coredump test maps. */
+#define SPARSE_MAPPING_SIZE (256 * 1024 * 1024)
+
 /* Shared helper function declarations */
 void *do_nothing(void *arg);
 void crashing_child(void);
+void crashing_child_sparse(size_t size);
+ssize_t recv_coredump_records(int fd_coredump, int fd_core_file,
+			      off_t *coredump_size, bool *truncated,
+			      int fd_peer_pidfd);
+bool is_elf_core(int fd);
 int create_detached_tmpfs(void);
 int create_and_listen_unix_socket(const char *path);
 bool set_core_pattern(const char *pattern);

-- 
2.53.0

[PATCH v2 19/22] selftests/coredump: hand the record stream to a sink

[Christian Brauner]

Currently recv_coredump_records() parses the record stream and dumps it
into a file. A coredump server may want to process the data it gets. So
split the parsing from the processing.

No functional changes.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 .../selftests/coredump/coredump_test_helpers.c     | 91 +++++++++++++++++-----
 1 file changed, 72 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.c b/tools/testing/selftests/coredump/coredump_test_helpers.c
index 5b2ffe17f7b7..45d76fa0f469 100644
--- a/tools/testing/selftests/coredump/coredump_test_helpers.c
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.c
@@ -71,9 +71,19 @@ void crashing_child_sparse(size_t size)
 	*(volatile int *)NULL = 0;
 }
 
-/* Read @len bytes off the socket, writing them at @offset if @fd_out >= 0. */
-static ssize_t recv_record_bytes(int fd_coredump, __u64 len, int fd_out,
-				 off_t offset)
+/* Sink a reassembled record stream is handed to, record by record. */
+struct coredump_record_sink {
+	/* @len bytes of coredump data that belong at @offset. */
+	int (*data)(void *ctx, const void *buf, size_t len, __u64 offset);
+	/* @len zero bytes that belong at @offset. */
+	int (*zero)(void *ctx, __u64 offset, __u64 len);
+	void *ctx;
+};
+
+/* Read @len bytes off the socket and hand them to @sink, if there is one. */
+static ssize_t recv_record_bytes(int fd_coredump, __u64 len,
+				 const struct coredump_record_sink *sink,
+				 __u64 offset)
 {
 	ssize_t received = 0;
 
@@ -89,11 +99,8 @@ static ssize_t recv_record_bytes(int fd_coredump, __u64 len, int fd_out,
 			return -1;
 		}
 
-		if (fd_out >= 0 &&
-		    pwrite(fd_out, buffer, ret, offset + received) != ret) {
-			fprintf(stderr, "%s: pwrite failed: %m\n", __func__);
+		if (sink && sink->data(sink->ctx, buffer, ret, offset + received))
 			return -1;
-		}
 
 		received += ret;
 		len -= ret;
@@ -102,14 +109,34 @@ static ssize_t recv_record_bytes(int fd_coredump, __u64 len, int fd_out,
 	return received;
 }
 
+/* Put the data where the records say it goes and leave the holes alone. */
+static int file_sink_data(void *ctx, const void *buf, size_t len, __u64 offset)
+{
+	int fd = *(int *)ctx;
+
+	if (pwrite(fd, buf, len, offset) != (ssize_t)len) {
+		fprintf(stderr, "%s: pwrite failed: %m\n", __func__);
+		return -1;
+	}
+
+	return 0;
+}
+
+static int file_sink_zero(void *ctx, __u64 offset, __u64 len)
+{
+	/* Nothing has to be written for a hole. */
+	return 0;
+}
+
 /*
- * Reassemble a record stream. If @fd_peer_pidfd is valid the task behind
- * it is killed once a data record has arrived, so the kernel has to cut
- * the coredump short with the stream already under way.
+ * Read a coredump strea and funnel it into @sink. Allow to pass in a
+ * @fd_peer_pidfd to simulate coredump truncation by killing it after having
+ * received a coredump record.
  */
-ssize_t recv_coredump_records(int fd_coredump, int fd_core_file,
-			      off_t *coredump_size, bool *truncated,
-			      int fd_peer_pidfd)
+static ssize_t __recv_coredump_records(int fd_coredump,
+				       const struct coredump_record_sink *sink,
+				       off_t *coredump_size, bool *truncated,
+				       int fd_peer_pidfd)
 {
 	ssize_t received = 0;
 	off_t size = 0;
@@ -169,7 +196,8 @@ ssize_t recv_coredump_records(int fd_coredump, int fd_core_file,
 		}
 
 		/* Discard any part of the header we have no use for. */
-		ret = recv_record_bytes(fd_coredump, record.size - known_size, -1, 0);
+		ret = recv_record_bytes(fd_coredump, record.size - known_size,
+					NULL, 0);
 		if (ret < 0)
 			return -1;
 		received += ret;
@@ -185,10 +213,12 @@ ssize_t recv_coredump_records(int fd_coredump, int fd_core_file,
 		switch (record.type) {
 		case COREDUMP_RECORD_ZERO:
 			/* A hole. It comes with no data and needs none. */
+			if (sink->zero(sink->ctx, record.offset, record.len))
+				return -1;
 			break;
 		case COREDUMP_RECORD_DATA:
-			ret = recv_record_bytes(fd_coredump, record.len,
-						fd_core_file, size);
+			ret = recv_record_bytes(fd_coredump, record.len, sink,
+						record.offset);
 			if (ret < 0)
 				return -1;
 			received += ret;
@@ -230,6 +260,32 @@ ssize_t recv_coredump_records(int fd_coredump, int fd_core_file,
 	if (truncated)
 		*truncated = is_truncated;
 
+	*coredump_size = size;
+
+	fprintf(stderr, "Received %zd bytes for a %s coredump of %llu bytes\n",
+		received, is_truncated ? "truncated" : "complete",
+		(unsigned long long)size);
+	return received;
+}
+
+/* Reassemble a record stream into the coredump it describes. */
+ssize_t recv_coredump_records(int fd_coredump, int fd_core_file,
+			      off_t *coredump_size, bool *truncated,
+			      int fd_peer_pidfd)
+{
+	struct coredump_record_sink sink = {
+		.data	= file_sink_data,
+		.zero	= file_sink_zero,
+		.ctx	= &fd_core_file,
+	};
+	ssize_t received;
+	off_t size = 0;
+
+	received = __recv_coredump_records(fd_coredump, &sink, &size, truncated,
+					   fd_peer_pidfd);
+	if (received < 0)
+		return -1;
+
 	/*
 	 * Nothing is written for a hole, so grow the file to the size the
 	 * records describe in case the coredump ended in one.
@@ -243,9 +299,6 @@ ssize_t recv_coredump_records(int fd_coredump, int fd_core_file,
 	if (coredump_size)
 		*coredump_size = size;
 
-	fprintf(stderr, "Received %zd bytes for a %s coredump of %llu bytes\n",
-		received, is_truncated ? "truncated" : "complete",
-		(unsigned long long)size);
 	return received;
 }
 

-- 
2.53.0

[PATCH v2 20/22] selftests/coredump: put a hole in the middle of a sparse mapping

[Christian Brauner]

The crashing_child_sparse() helper touches the first page of the
mapping. That forces everything behind it to be a trailing hole. This is
easy to handle. Make the test more difficult meaningful by also touchin
the last page. This causes the hole to sit between two populated pages.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 tools/testing/selftests/coredump/coredump_test_helpers.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.c b/tools/testing/selftests/coredump/coredump_test_helpers.c
index 45d76fa0f469..89f3954c5607 100644
--- a/tools/testing/selftests/coredump/coredump_test_helpers.c
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.c
@@ -58,14 +58,16 @@ 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.
+	 * Touch the first and the last page. This will cause the whole mapping
+	 * to be dumped because it has been written to. Everything between
+	 * those two pages is a hole though.
 	 */
 	p = mmap(NULL, size, PROT_READ | PROT_WRITE,
 		 MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0);
-	if (p != MAP_FAILED)
+	if (p != MAP_FAILED) {
 		p[0] = 'x';
+		p[size - 1] = 'x';
+	}
 
 	/* crash on purpose */
 	*(volatile int *)NULL = 0;

-- 
2.53.0

[PATCH v2 21/22] selftests/coredump: simulate a blob store

[Christian Brauner]

A coredump server that uploads to a blob store must redescribe the
coredump and fixup the phdr.

A segment is split wherever a hole was left out and everything a segment
covers past p_filesz is zeroes anyway. So the blob store ends up with an
ordinary ELF core file that is missing nothing but holes. Nothing
downstream of the server has to learn a container format.

The coredump with its holes still in it is reassembled alongside the
object so the two can be compared.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 .../coredump/coredump_socket_protocol_test.c       | 156 +++++
 .../selftests/coredump/coredump_test_helpers.c     | 765 +++++++++++++++++++++
 .../selftests/coredump/coredump_test_helpers.h     |   3 +
 3 files changed, 924 insertions(+)

diff --git a/tools/testing/selftests/coredump/coredump_socket_protocol_test.c b/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
index abf6e2c4c354..f33eaf2fa93d 100644
--- a/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
+++ b/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
@@ -1882,6 +1882,162 @@ TEST_F(coredump, socket_request_records_truncated)
 	ASSERT_GT(received, 0);
 }
 
+/*
+ * A coredump server that uploads to a blob store can't upload a sparse
+ * file. It doesn't have to: it streams the data records into the object
+ * as they arrive, leaves the holes out, and uploads the corrected
+ * program header table last. What it ends up with is an ordinary ELF
+ * core file that describes the same memory as the coredump the records
+ * came from, minus the holes.
+ */
+TEST_F(coredump, socket_request_sparse_blob_upload)
+{
+	int fd_core_file, pidfd, status;
+	pid_t pid, pid_coredump_server;
+	struct pidfd_info info = {};
+	off_t coredump_size = 0;
+	ssize_t received = 0;
+	int ipc_sockets[2];
+	int pipefds[2];
+	struct stat st;
+	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_object = -1, fd_reference = -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 object is a plain file. It never sees a hole. */
+		fd_object = open("/tmp/coredump.file",
+				 O_RDWR | O_CREAT | O_TRUNC | O_CLOEXEC, 0600);
+		if (fd_object < 0)
+			goto out;
+
+		/*
+		 * The coredump with its holes still in it is bigger than
+		 * the mapping the child made, so keep it on the detached
+		 * tmpfs and sparse.
+		 */
+		fd_reference = open_coredump_tmpfile(self->fd_tmpfs_detached);
+		if (fd_reference < 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_RECORDS |
+				       COREDUMP_SPARSE | COREDUMP_WAIT, 0))
+			goto out;
+
+		if (!read_marker(fd_coredump, COREDUMP_MARK_REQACK))
+			goto out;
+
+		ret = recv_coredump_compact(fd_coredump, fd_object,
+					    fd_reference, &size);
+		if (ret < 0)
+			goto out;
+
+		if (check_compact_coredump(fd_object, fd_reference))
+			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_reference >= 0)
+			close(fd_reference);
+		if (fd_object >= 0)
+			close(fd_object);
+		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_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);
+
+	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);
+
+	/* The mapping is in the coredump, holes included. */
+	ASSERT_GT(coredump_size, (off_t)SPARSE_MAPPING_SIZE);
+
+	/* The object isn't sparse and doesn't carry them. */
+	ASSERT_EQ(stat("/tmp/coredump.file", &st), 0);
+	ASSERT_LT(st.st_size, coredump_size / 8);
+
+	/* And a debugger still sees an ordinary ELF core file. */
+	fd_core_file = open("/tmp/coredump.file", O_RDONLY | O_CLOEXEC);
+	ASSERT_GE(fd_core_file, 0);
+	ASSERT_TRUE(is_elf_core(fd_core_file));
+	EXPECT_EQ(close(fd_core_file), 0);
+}
+
 /* Ack @ack_mask, expect the kernel to refuse it as conflicting. */
 static void check_conflicting_ack(struct __test_metadata *const _metadata,
 				  FIXTURE_DATA(coredump) *self, __u64 ack_mask)
diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.c b/tools/testing/selftests/coredump/coredump_test_helpers.c
index 89f3954c5607..9346b8f688e2 100644
--- a/tools/testing/selftests/coredump/coredump_test_helpers.c
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.c
@@ -330,6 +330,771 @@ bool is_elf_core(int fd)
 	return true;
 }
 
+/*
+ * A coredump server that uploads to a blob store can't upload a sparse
+ * file and can't seek in the object it is uploading. It streams the data
+ * records into the object as they arrive, remembers the holes it left
+ * out, and uploads the program header table that describes the result
+ * last. What comes out is an ordinary ELF core file without the holes.
+ */
+
+/* A run of the coredump the object doesn't carry. */
+struct compact_hole {
+	__u64 offset;
+	__u64 len;
+};
+
+/* A program header of the object and where its bytes sat in the coredump. */
+struct compact_piece {
+	ElfW(Phdr) phdr;
+	__u64 src;
+};
+
+struct compact_ctx {
+	int fd_body;			/* the object's payload, append only */
+	int fd_reference;		/* the coredump with its holes, for the test */
+	unsigned char *head;		/* everything ahead of the segment data */
+	size_t head_len;
+	size_t head_cap;
+	__u64 data_offset;		/* where the segment data starts, 0 while unknown */
+	struct compact_hole *holes;
+	size_t nr_holes;
+	size_t holes_cap;
+	__u64 body_len;
+};
+
+/* Write @len bytes out, short writes and all. */
+static int compact_write(int fd, const void *buf, size_t len)
+{
+	const unsigned char *pos = buf;
+
+	while (len) {
+		ssize_t ret = write(fd, pos, len);
+
+		if (ret <= 0) {
+			fprintf(stderr, "%s: write failed: %m\n", __func__);
+			return -1;
+		}
+
+		pos += ret;
+		len -= ret;
+	}
+
+	return 0;
+}
+
+/* Keep @len bytes of the head, or @len zeroes if @buf is NULL. */
+static int compact_head_append(struct compact_ctx *ctx, const void *buf,
+			       size_t len)
+{
+	if (ctx->head_len + len > ctx->head_cap) {
+		size_t cap = ctx->head_cap ? ctx->head_cap : PAGE_SIZE;
+		unsigned char *head;
+
+		while (cap < ctx->head_len + len)
+			cap *= 2;
+
+		head = realloc(ctx->head, cap);
+		if (!head) {
+			fprintf(stderr, "%s: out of memory\n", __func__);
+			return -1;
+		}
+		ctx->head = head;
+		ctx->head_cap = cap;
+	}
+
+	if (buf)
+		memcpy(ctx->head + ctx->head_len, buf, len);
+	else
+		memset(ctx->head + ctx->head_len, 0, len);
+	ctx->head_len += len;
+
+	return 0;
+}
+
+/* Remember a hole so the program header table can account for it later. */
+static int compact_keep_hole(struct compact_ctx *ctx, __u64 offset, __u64 len)
+{
+	if (ctx->nr_holes == ctx->holes_cap) {
+		size_t cap = ctx->holes_cap ? ctx->holes_cap * 2 : 64;
+		struct compact_hole *holes;
+
+		holes = realloc(ctx->holes, cap * sizeof(*holes));
+		if (!holes) {
+			fprintf(stderr, "%s: out of memory\n", __func__);
+			return -1;
+		}
+		ctx->holes = holes;
+		ctx->holes_cap = cap;
+	}
+
+	ctx->holes[ctx->nr_holes].offset = offset;
+	ctx->holes[ctx->nr_holes].len = len;
+	ctx->nr_holes++;
+
+	return 0;
+}
+
+/* The segment data starts where the first PT_LOAD points. */
+static int compact_probe(struct compact_ctx *ctx)
+{
+	const ElfW(Ehdr) *ehdr = (const ElfW(Ehdr) *)ctx->head;
+	const ElfW(Phdr) *phdr;
+	size_t i;
+
+	if (ctx->data_offset || ctx->head_len < sizeof(*ehdr))
+		return 0;
+
+	if (!is_core_ehdr(ehdr)) {
+		fprintf(stderr, "%s: not an ELF core file\n", __func__);
+		return -1;
+	}
+
+	if (ehdr->e_phoff != sizeof(*ehdr) ||
+	    ehdr->e_phentsize != sizeof(ElfW(Phdr)) ||
+	    ehdr->e_phnum == 0 || ehdr->e_phnum == PN_XNUM) {
+		fprintf(stderr, "%s: unhandled program header table\n", __func__);
+		return -1;
+	}
+
+	if (ctx->head_len < ehdr->e_phoff +
+			    (size_t)ehdr->e_phnum * ehdr->e_phentsize)
+		return 0;
+
+	phdr = (const ElfW(Phdr) *)(ctx->head + ehdr->e_phoff);
+	for (i = 0; i < ehdr->e_phnum; i++) {
+		if (phdr[i].p_type != PT_LOAD)
+			continue;
+		if (!ctx->data_offset || phdr[i].p_offset < ctx->data_offset)
+			ctx->data_offset = phdr[i].p_offset;
+	}
+
+	if (!ctx->data_offset) {
+		fprintf(stderr, "%s: coredump without a single segment\n",
+			__func__);
+		return -1;
+	}
+
+	return 0;
+}
+
+/*
+ * Take whatever of [@offset, @offset + @len) still belongs to the head.
+ * @buf is NULL for a hole. Returns how much was taken.
+ */
+static ssize_t compact_head_take(struct compact_ctx *ctx, const void *buf,
+				 __u64 offset, __u64 len)
+{
+	__u64 chunk;
+
+	if (!len || (ctx->data_offset && offset >= ctx->data_offset))
+		return 0;
+
+	chunk = len;
+	if (ctx->data_offset && offset + chunk > ctx->data_offset)
+		chunk = ctx->data_offset - offset;
+
+	if (offset != ctx->head_len) {
+		fprintf(stderr, "%s: head has a gap at %llu\n", __func__,
+			(unsigned long long)offset);
+		return -1;
+	}
+
+	if (compact_head_append(ctx, buf, chunk))
+		return -1;
+
+	return chunk;
+}
+
+static int compact_data(void *arg, const void *buf, size_t len, __u64 offset)
+{
+	struct compact_ctx *ctx = arg;
+	const unsigned char *pos = buf;
+	ssize_t head;
+
+	/* Only the test needs a coredump with the holes still in it. */
+	if (pwrite(ctx->fd_reference, pos, len, offset) != (ssize_t)len) {
+		fprintf(stderr, "%s: pwrite failed: %m\n", __func__);
+		return -1;
+	}
+
+	/* The head has to be rewritten at the end, so hold on to it. */
+	head = compact_head_take(ctx, pos, offset, len);
+	if (head < 0)
+		return -1;
+	if (head && compact_probe(ctx))
+		return -1;
+
+	pos += head;
+	len -= head;
+	if (!len)
+		return 0;
+
+	/* Everything else goes into the object as it arrives. */
+	if (compact_write(ctx->fd_body, pos, len))
+		return -1;
+	ctx->body_len += len;
+
+	return 0;
+}
+
+static int compact_zero(void *arg, __u64 offset, __u64 len)
+{
+	struct compact_ctx *ctx = arg;
+	ssize_t head;
+
+	/* A hole in the head is alignment padding. Write it out. */
+	head = compact_head_take(ctx, NULL, offset, len);
+	if (head < 0)
+		return -1;
+
+	offset += head;
+	len -= head;
+	if (!len)
+		return 0;
+
+	/* This is what the object doesn't have to carry. */
+	return compact_keep_hole(ctx, offset, len);
+}
+
+/* Where @offset ends up in the object once the holes ahead of it are gone. */
+static __u64 compact_offset(const struct compact_ctx *ctx, __u64 body_start,
+			    __u64 offset)
+{
+	__u64 elided = 0;
+	size_t i;
+
+	for (i = 0; i < ctx->nr_holes; i++) {
+		__u64 len = ctx->holes[i].len;
+
+		if (ctx->holes[i].offset >= offset)
+			break;
+		if (ctx->holes[i].offset + len > offset)
+			len = offset - ctx->holes[i].offset;
+		elided += len;
+	}
+
+	return body_start + (offset - ctx->data_offset) - elided;
+}
+
+/* A run of segment data that made it into the object. */
+static void compact_add_data(struct compact_piece *pieces, size_t *nr,
+			     const ElfW(Phdr) *phdr, __u64 start, __u64 end)
+{
+	struct compact_piece *piece = &pieces[(*nr)++];
+
+	piece->phdr = *phdr;
+	piece->phdr.p_vaddr = phdr->p_vaddr + (start - phdr->p_offset);
+	piece->phdr.p_paddr = 0;
+	piece->phdr.p_filesz = end - start;
+	piece->phdr.p_memsz = end - start;
+	piece->src = start;
+}
+
+/*
+ * A run of @len bytes the object doesn't carry. It grows the piece in
+ * front of it if this segment already has one, because everything a
+ * segment covers past p_filesz is zeroes anyway.
+ */
+static void compact_add_zero(struct compact_piece *pieces, size_t *nr,
+			     size_t first, const ElfW(Phdr) *phdr, __u64 vaddr,
+			     __u64 len)
+{
+	struct compact_piece *piece;
+
+	if (*nr > first) {
+		pieces[*nr - 1].phdr.p_memsz += len;
+		return;
+	}
+
+	piece = &pieces[(*nr)++];
+	piece->phdr = *phdr;
+	piece->phdr.p_vaddr = vaddr;
+	piece->phdr.p_paddr = 0;
+	piece->phdr.p_filesz = 0;
+	piece->phdr.p_memsz = len;
+	piece->src = 0;
+}
+
+/* Split the segments at the holes and write out what the object became. */
+static int compact_build(struct compact_ctx *ctx, int fd_object)
+{
+	__u64 note_offset = 0, note_len = 0, note_new;
+	__u64 align = 0, head_len, body_start, pos;
+	size_t nr_old, nr_new = 0, note_piece = 0, i;
+	struct compact_piece *pieces;
+	char buffer[PAGE_SIZE];
+	const ElfW(Phdr) *old;
+	ElfW(Ehdr) ehdr;
+	int ret = -1;
+
+	if (!ctx->data_offset) {
+		fprintf(stderr, "%s: coredump without segment data\n", __func__);
+		return -1;
+	}
+
+	memcpy(&ehdr, ctx->head, sizeof(ehdr));
+	if (ehdr.e_shoff) {
+		fprintf(stderr, "%s: section headers are not handled\n",
+			__func__);
+		return -1;
+	}
+
+	old = (const ElfW(Phdr) *)(ctx->head + ehdr.e_phoff);
+	nr_old = ehdr.e_phnum;
+
+	pieces = calloc(nr_old + 2 * ctx->nr_holes + 1, sizeof(*pieces));
+	if (!pieces) {
+		fprintf(stderr, "%s: out of memory\n", __func__);
+		return -1;
+	}
+
+	for (i = 0; i < nr_old; i++) {
+		ElfW(Phdr) phdr = old[i];
+		__u64 end = phdr.p_offset + phdr.p_filesz;
+		__u64 cur = phdr.p_offset;
+		size_t first = nr_new, h;
+
+		/* The notes move because the table in front of them grows. */
+		if (phdr.p_type == PT_NOTE) {
+			if (note_len) {
+				fprintf(stderr, "%s: more than one note segment\n",
+					__func__);
+				goto out;
+			}
+			note_offset = phdr.p_offset;
+			note_len = phdr.p_filesz;
+			note_piece = nr_new;
+			pieces[nr_new].phdr = phdr;
+			pieces[nr_new++].src = 0;
+			continue;
+		}
+
+		if (phdr.p_type != PT_LOAD) {
+			if (phdr.p_filesz && phdr.p_offset < ctx->data_offset) {
+				fprintf(stderr, "%s: segment %zu is in the head\n",
+					__func__, i);
+				goto out;
+			}
+			pieces[nr_new].phdr = phdr;
+			pieces[nr_new++].src = phdr.p_offset;
+			continue;
+		}
+
+		if (!align)
+			align = phdr.p_align;
+
+		for (h = 0; h < ctx->nr_holes && cur < end; h++) {
+			__u64 start = ctx->holes[h].offset;
+			__u64 stop = start + ctx->holes[h].len;
+
+			if (stop <= cur)
+				continue;
+			if (start >= end)
+				break;
+
+			/* A hole can span more than this one segment. */
+			if (start < cur)
+				start = cur;
+			if (stop > end)
+				stop = end;
+
+			if (start > cur) {
+				compact_add_data(pieces, &nr_new, &phdr, cur,
+						 start);
+				cur = start;
+			}
+			compact_add_zero(pieces, &nr_new, first, &phdr,
+					 phdr.p_vaddr + (cur - phdr.p_offset),
+					 stop - cur);
+			cur = stop;
+		}
+
+		if (cur < end)
+			compact_add_data(pieces, &nr_new, &phdr, cur, end);
+
+		/* Whatever the kernel didn't dump of this mapping. */
+		if (phdr.p_memsz > phdr.p_filesz)
+			compact_add_zero(pieces, &nr_new, first, &phdr,
+					 phdr.p_vaddr + phdr.p_filesz,
+					 phdr.p_memsz - phdr.p_filesz);
+	}
+
+	if (!note_len || note_offset + note_len > ctx->head_len) {
+		fprintf(stderr, "%s: notes aren't where they should be\n",
+			__func__);
+		goto out;
+	}
+
+	if (nr_new >= PN_XNUM) {
+		fprintf(stderr, "%s: %zu program headers don't fit\n", __func__,
+			nr_new);
+		goto out;
+	}
+
+	if (!align || (align & (align - 1)))
+		align = sysconf(_SC_PAGESIZE);
+
+	note_new = sizeof(ehdr) + (__u64)nr_new * sizeof(ElfW(Phdr));
+	head_len = note_new + note_len;
+	body_start = (head_len + align - 1) & ~(align - 1);
+
+	for (i = 0; i < nr_new; i++) {
+		struct compact_piece *piece = &pieces[i];
+
+		if (i == note_piece)
+			piece->phdr.p_offset = note_new;
+		else if (piece->phdr.p_filesz)
+			piece->phdr.p_offset = compact_offset(ctx, body_start,
+							      piece->src);
+		else
+			piece->phdr.p_offset = 0;
+	}
+
+	/* Only now is the head known. That's why it is uploaded last. */
+	ehdr.e_phnum = nr_new;
+	if (compact_write(fd_object, &ehdr, sizeof(ehdr)))
+		goto out;
+
+	for (i = 0; i < nr_new; i++)
+		if (compact_write(fd_object, &pieces[i].phdr,
+				  sizeof(pieces[i].phdr)))
+			goto out;
+
+	if (compact_write(fd_object, ctx->head + note_offset, note_len))
+		goto out;
+
+	/* Keep the segments aligned the way a debugger expects them. */
+	memset(buffer, 0, sizeof(buffer));
+	for (pos = head_len; pos < body_start; ) {
+		__u64 chunk = body_start - pos;
+
+		if (chunk > sizeof(buffer))
+			chunk = sizeof(buffer);
+		if (compact_write(fd_object, buffer, chunk))
+			goto out;
+		pos += chunk;
+	}
+
+	/* Putting the parts together is the blob store's job. Do it here. */
+	for (pos = 0; pos < ctx->body_len; ) {
+		ssize_t chunk = pread(ctx->fd_body, buffer, sizeof(buffer), pos);
+
+		if (chunk <= 0) {
+			fprintf(stderr, "%s: short read %zd: %m\n", __func__,
+				chunk);
+			goto out;
+		}
+		if (compact_write(fd_object, buffer, chunk))
+			goto out;
+		pos += chunk;
+	}
+
+	fprintf(stderr, "Object is %llu bytes in %zu program headers, %zu holes left out\n",
+		(unsigned long long)(body_start + ctx->body_len), nr_new,
+		ctx->nr_holes);
+	ret = 0;
+out:
+	free(pieces);
+	return ret;
+}
+
+/*
+ * Reassemble a record stream into an ELF core file that has no holes in
+ * it, the way a coredump server that uploads to a blob store has to. If
+ * @fd_reference is valid it gets the coredump the records describe,
+ * holes and all, so the test can compare the two.
+ */
+ssize_t recv_coredump_compact(int fd_coredump, int fd_object, int fd_reference,
+			      off_t *coredump_size)
+{
+	struct compact_ctx ctx = {
+		.fd_body	= -1,
+		.fd_reference	= fd_reference,
+	};
+	struct coredump_record_sink sink = {
+		.data	= compact_data,
+		.zero	= compact_zero,
+		.ctx	= &ctx,
+	};
+	ssize_t received;
+	off_t size = 0;
+	FILE *body;
+
+	body = tmpfile();
+	if (!body) {
+		fprintf(stderr, "%s: tmpfile failed: %m\n", __func__);
+		return -1;
+	}
+	ctx.fd_body = fileno(body);
+
+	/* An upload is appended to. Make sure nothing here can seek. */
+	if (fcntl(ctx.fd_body, F_SETFL, O_APPEND)) {
+		fprintf(stderr, "%s: F_SETFL failed: %m\n", __func__);
+		received = -1;
+		goto out;
+	}
+
+	received = __recv_coredump_records(fd_coredump, &sink, &size, NULL, -1);
+	if (received < 0)
+		goto out;
+
+	/*
+	 * Nothing is written for a hole, so grow the reference to the size
+	 * the records describe in case the coredump ended in one.
+	 */
+	if (ftruncate(fd_reference, size) < 0) {
+		fprintf(stderr, "%s: ftruncate to %llu failed: %m\n",
+			__func__, (unsigned long long)size);
+		received = -1;
+		goto out;
+	}
+
+	if (compact_build(&ctx, fd_object)) {
+		received = -1;
+		goto out;
+	}
+
+	if (coredump_size)
+		*coredump_size = size;
+out:
+	fclose(body);
+	free(ctx.head);
+	free(ctx.holes);
+	return received;
+}
+
+/* Read the ELF header and the program header table of @fd. */
+static ElfW(Phdr) *read_phdrs(int fd, size_t *nr)
+{
+	ElfW(Ehdr) ehdr;
+	ElfW(Phdr) *phdr;
+	size_t size;
+
+	if (pread(fd, &ehdr, sizeof(ehdr), 0) != sizeof(ehdr)) {
+		fprintf(stderr, "%s: no ELF header: %m\n", __func__);
+		return NULL;
+	}
+
+	if (!is_core_ehdr(&ehdr) || !ehdr.e_phnum ||
+	    ehdr.e_phentsize != sizeof(*phdr)) {
+		fprintf(stderr, "%s: not an ELF core file\n", __func__);
+		return NULL;
+	}
+
+	size = (size_t)ehdr.e_phnum * ehdr.e_phentsize;
+	phdr = malloc(size);
+	if (!phdr) {
+		fprintf(stderr, "%s: out of memory\n", __func__);
+		return NULL;
+	}
+
+	if (pread(fd, phdr, size, ehdr.e_phoff) != (ssize_t)size) {
+		fprintf(stderr, "%s: short program header table: %m\n", __func__);
+		free(phdr);
+		return NULL;
+	}
+
+	*nr = ehdr.e_phnum;
+	return phdr;
+}
+
+/* The segment @vaddr falls into. */
+static const ElfW(Phdr) *find_segment(const ElfW(Phdr) *phdr, size_t nr,
+				      __u64 vaddr)
+{
+	size_t i;
+
+	for (i = 0; i < nr; i++) {
+		if (phdr[i].p_type != PT_LOAD)
+			continue;
+		if (vaddr >= phdr[i].p_vaddr &&
+		    vaddr < phdr[i].p_vaddr + phdr[i].p_memsz)
+			return &phdr[i];
+	}
+
+	return NULL;
+}
+
+/* The next stretch of memory the segments cover, split ones merged back. */
+static bool next_range(const ElfW(Phdr) *phdr, size_t nr, size_t *i,
+		       __u64 *start, __u64 *end)
+{
+	while (*i < nr && phdr[*i].p_type != PT_LOAD)
+		(*i)++;
+
+	if (*i >= nr)
+		return false;
+
+	*start = phdr[*i].p_vaddr;
+	*end = phdr[*i].p_vaddr + phdr[*i].p_memsz;
+	(*i)++;
+
+	while (*i < nr) {
+		if (phdr[*i].p_type != PT_LOAD) {
+			(*i)++;
+			continue;
+		}
+		if (phdr[*i].p_vaddr != *end)
+			break;
+		*end = phdr[*i].p_vaddr + phdr[*i].p_memsz;
+		(*i)++;
+	}
+
+	return true;
+}
+
+/* Compare @len bytes at @offset against @len bytes at @offset_ref. */
+static int compare_range(int fd, __u64 offset, int fd_ref, __u64 offset_ref,
+			 __u64 len)
+{
+	char buffer[PAGE_SIZE], buffer_ref[PAGE_SIZE];
+
+	while (len) {
+		size_t chunk = len < sizeof(buffer) ? len : sizeof(buffer);
+
+		if (pread(fd, buffer, chunk, offset) != (ssize_t)chunk ||
+		    pread(fd_ref, buffer_ref, chunk, offset_ref) != (ssize_t)chunk) {
+			fprintf(stderr, "%s: short read at %llu: %m\n",
+				__func__, (unsigned long long)offset);
+			return -1;
+		}
+
+		if (memcmp(buffer, buffer_ref, chunk)) {
+			fprintf(stderr, "%s: %llu differs from %llu\n", __func__,
+				(unsigned long long)offset,
+				(unsigned long long)offset_ref);
+			return -1;
+		}
+
+		offset += chunk;
+		offset_ref += chunk;
+		len -= chunk;
+	}
+
+	return 0;
+}
+
+/* The @len bytes at @offset the object left out have to have been zeroes. */
+static int check_zero_range(int fd, __u64 offset, __u64 len)
+{
+	static const char zeroes[PAGE_SIZE];
+	char buffer[PAGE_SIZE];
+
+	while (len) {
+		size_t chunk = len < sizeof(buffer) ? len : sizeof(buffer);
+
+		if (pread(fd, buffer, chunk, offset) != (ssize_t)chunk) {
+			fprintf(stderr, "%s: short read at %llu: %m\n",
+				__func__, (unsigned long long)offset);
+			return -1;
+		}
+
+		if (memcmp(buffer, zeroes, chunk)) {
+			fprintf(stderr, "%s: %llu isn't a hole\n", __func__,
+				(unsigned long long)offset);
+			return -1;
+		}
+
+		offset += chunk;
+		len -= chunk;
+	}
+
+	return 0;
+}
+
+/*
+ * The object has to describe the same memory as the coredump it was built
+ * from, and it has to describe it correctly.
+ */
+int check_compact_coredump(int fd_object, int fd_reference)
+{
+	ElfW(Phdr) *object = NULL, *reference = NULL;
+	size_t nr_object, nr_reference, i;
+	size_t io = 0, ir = 0;
+	int ret = -1;
+
+	object = read_phdrs(fd_object, &nr_object);
+	reference = read_phdrs(fd_reference, &nr_reference);
+	if (!object || !reference)
+		goto out;
+
+	/* Nothing may have been dropped and nothing may have been added. */
+	for (;;) {
+		__u64 start = 0, end = 0, start_ref = 0, end_ref = 0;
+		bool has, has_ref;
+
+		has = next_range(object, nr_object, &io, &start, &end);
+		has_ref = next_range(reference, nr_reference, &ir, &start_ref,
+				     &end_ref);
+		if (!has && !has_ref)
+			break;
+
+		if (has != has_ref || start != start_ref || end != end_ref) {
+			fprintf(stderr, "%s: object covers 0x%llx-0x%llx, coredump 0x%llx-0x%llx\n",
+				__func__, (unsigned long long)start,
+				(unsigned long long)end,
+				(unsigned long long)start_ref,
+				(unsigned long long)end_ref);
+			goto out;
+		}
+	}
+
+	for (i = 0; i < nr_object; i++) {
+		const ElfW(Phdr) *segment;
+		__u64 offset, dumped;
+
+		if (object[i].p_type != PT_LOAD || !object[i].p_memsz)
+			continue;
+
+		segment = find_segment(reference, nr_reference,
+				       object[i].p_vaddr);
+		if (!segment) {
+			fprintf(stderr, "%s: 0x%llx isn't in the coredump\n",
+				__func__,
+				(unsigned long long)object[i].p_vaddr);
+			goto out;
+		}
+
+		offset = object[i].p_vaddr - segment->p_vaddr;
+		dumped = offset < segment->p_filesz ?
+				 segment->p_filesz - offset : 0;
+
+		/* What the object carries is what the coredump had. */
+		if (object[i].p_filesz > dumped) {
+			fprintf(stderr, "%s: object carries %llu bytes the coredump doesn't have\n",
+				__func__,
+				(unsigned long long)(object[i].p_filesz - dumped));
+			goto out;
+		}
+
+		if (compare_range(fd_object, object[i].p_offset, fd_reference,
+				  segment->p_offset + offset,
+				  object[i].p_filesz))
+			goto out;
+
+		/* And what it left out was a hole. */
+		if (object[i].p_memsz > object[i].p_filesz &&
+		    dumped > object[i].p_filesz) {
+			__u64 left_out = dumped - object[i].p_filesz;
+
+			if (left_out > object[i].p_memsz - object[i].p_filesz)
+				left_out = object[i].p_memsz - object[i].p_filesz;
+
+			if (check_zero_range(fd_reference,
+					     segment->p_offset + offset +
+					     object[i].p_filesz, left_out))
+				goto out;
+		}
+	}
+
+	ret = 0;
+out:
+	free(object);
+	free(reference);
+	return ret;
+}
+
 int create_detached_tmpfs(void)
 {
 	int fd_context, fd_tmpfs;
diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.h b/tools/testing/selftests/coredump/coredump_test_helpers.h
index fe0a88a71b05..00d695b67b3f 100644
--- a/tools/testing/selftests/coredump/coredump_test_helpers.h
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.h
@@ -26,6 +26,9 @@ ssize_t recv_coredump_records(int fd_coredump, int fd_core_file,
 			      off_t *coredump_size, bool *truncated,
 			      int fd_peer_pidfd);
 bool is_elf_core(int fd);
+ssize_t recv_coredump_compact(int fd_coredump, int fd_object, int fd_reference,
+			      off_t *coredump_size);
+int check_compact_coredump(int fd_object, int fd_reference);
 int create_detached_tmpfs(void);
 int create_and_listen_unix_socket(const char *path);
 bool set_core_pattern(const char *pattern);

-- 
2.53.0

[PATCH v2 22/22] selftests/coredump: show how to inspect the task to decide how the coredump should be sent

[Christian Brauner]

The kernel blocks in the coredump req until the coredump ack is sent by
the coredump server. This allows the coredump server to decide how the
kernel is supposed to send the coredump.

Let's show how that can work:

- a task that has a large memory mapping gets sent as a sparse record
  stream

- a task with a trivial memory mapping gets sent as a plain byte stream

Since the threads are parked in coredump_task_exit() with their mm
around we can look at /proc/<pid>/statm to figure out what the task has
mapped.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 .../coredump/coredump_socket_protocol_test.c       | 184 +++++++++++++++++++++
 .../selftests/coredump/coredump_test_helpers.c     |  58 +++++++
 .../selftests/coredump/coredump_test_helpers.h     |   7 +-
 3 files changed, 248 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 f33eaf2fa93d..daff908232a2 100644
--- a/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
+++ b/tools/testing/selftests/coredump/coredump_socket_protocol_test.c
@@ -2136,4 +2136,188 @@ TEST_F(coredump, socket_request_sparse_without_records)
 	check_conflicting_ack(_metadata, self, COREDUMP_KERNEL | COREDUMP_SPARSE);
 }
 
+/* What the server reports back about the coredump it decided to take. */
+struct stream_choice {
+	bool sparse;
+	ssize_t received;
+	off_t size;
+	ssize_t vm_size;
+};
+
+/*
+ * The kernel blocks in the coredump request until the ack arrives, so a
+ * coredump server gets to look at the task before it commits to a
+ * stream. Take the record stream only for a task whose mappings are
+ * worth it and the plain byte stream for everything else.
+ */
+static void check_stream_choice(struct __test_metadata *const _metadata,
+				FIXTURE_DATA(coredump) *self, bool big,
+				struct stream_choice *choice)
+{
+	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 = {};
+		struct stream_choice got = {};
+		__u64 mask;
+
+		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;
+
+		/*
+		 * Nothing is on the wire yet and the kernel is waiting for
+		 * the ack, so there is all the time in the world to look at
+		 * the task and decide what to ask it for.
+		 */
+		got.vm_size = peer_vm_size(fd_peer_pidfd);
+		if (got.vm_size < 0)
+			goto out;
+		got.sparse = got.vm_size >= SPARSE_STREAM_THRESHOLD;
+
+		fprintf(stderr, "Peer maps %zd bytes, asking for %s\n",
+			got.vm_size,
+			got.sparse ? "a sparse record stream" : "a byte stream");
+
+		mask = COREDUMP_KERNEL | COREDUMP_WAIT;
+		if (got.sparse)
+			mask |= COREDUMP_RECORDS | COREDUMP_SPARSE;
+
+		if (!send_coredump_ack(fd_coredump, &req, mask, 0))
+			goto out;
+
+		if (!read_marker(fd_coredump, COREDUMP_MARK_REQACK))
+			goto out;
+
+		if (got.sparse) {
+			got.received = recv_coredump_records(fd_coredump, fd_file,
+							     &got.size, NULL, -1);
+		} else {
+			got.received = recv_coredump_bytes(fd_coredump, fd_file);
+			got.size = got.received;
+		}
+		if (got.received < 0)
+			goto out;
+
+		/* Either way a debugger has to see an ordinary core file. */
+		if (!is_elf_core(fd_file))
+			goto out;
+
+		if (write_nointr(pipefds[1], &got, sizeof(got)) != sizeof(got))
+			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(big ? SPARSE_MAPPING_SIZE : PAGE_SIZE);
+
+	pidfd = sys_pidfd_open(pid, 0);
+	ASSERT_GE(pidfd, 0);
+
+	waitpid(pid, &status, 0);
+	ASSERT_TRUE(WIFSIGNALED(status));
+	ASSERT_TRUE(WCOREDUMP(status));
+
+	ASSERT_EQ(read_nointr(pipefds[0], choice, sizeof(*choice)),
+		  sizeof(*choice));
+	EXPECT_EQ(close(pipefds[0]), 0);
+
+	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);
+}
+
+/* A task with little mapped isn't worth a record stream. */
+TEST_F(coredump, socket_request_stream_choice_small)
+{
+	struct stream_choice choice = {};
+
+	check_stream_choice(_metadata, self, false, &choice);
+
+	ASSERT_LT(choice.vm_size, (ssize_t)SPARSE_STREAM_THRESHOLD);
+	ASSERT_FALSE(choice.sparse);
+	ASSERT_GT(choice.received, 0);
+}
+
+/* A task sitting on a big mapping is. */
+TEST_F(coredump, socket_request_stream_choice_large)
+{
+	struct stream_choice choice = {};
+
+	check_stream_choice(_metadata, self, true, &choice);
+
+	ASSERT_GE(choice.vm_size, (ssize_t)SPARSE_STREAM_THRESHOLD);
+	ASSERT_TRUE(choice.sparse);
+	ASSERT_GT(choice.size, (off_t)SPARSE_MAPPING_SIZE);
+
+	/* The holes didn't have to go over the socket. */
+	ASSERT_LT(choice.received, choice.size / 8);
+}
+
 TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.c b/tools/testing/selftests/coredump/coredump_test_helpers.c
index 9346b8f688e2..d7cc448eeaf4 100644
--- a/tools/testing/selftests/coredump/coredump_test_helpers.c
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.c
@@ -1095,6 +1095,33 @@ int check_compact_coredump(int fd_object, int fd_reference)
 	return ret;
 }
 
+/* Read a plain coredump byte stream to end-of-file. */
+ssize_t recv_coredump_bytes(int fd_coredump, int fd_core_file)
+{
+	ssize_t received = 0;
+
+	for (;;) {
+		char buffer[PAGE_SIZE];
+		ssize_t ret = read_nointr(fd_coredump, buffer, sizeof(buffer));
+
+		if (ret < 0) {
+			fprintf(stderr, "%s: read failed: %m\n", __func__);
+			return -1;
+		}
+		if (ret == 0)
+			break;
+
+		if (write_nointr(fd_core_file, buffer, ret) != ret) {
+			fprintf(stderr, "%s: write failed: %m\n", __func__);
+			return -1;
+		}
+		received += ret;
+	}
+
+	fprintf(stderr, "Received %zd bytes of coredump\n", received);
+	return received;
+}
+
 int create_detached_tmpfs(void)
 {
 	int fd_context, fd_tmpfs;
@@ -1190,6 +1217,37 @@ bool get_pidfd_info(int fd_peer_pidfd, struct pidfd_info *info)
 	return true;
 }
 
+/*
+ * How much the peer has mapped. The task is parked in the coredump
+ * handshake, so its mm is still there to be looked at.
+ */
+ssize_t peer_vm_size(int fd_peer_pidfd)
+{
+	struct pidfd_info info = {};
+	unsigned long pages;
+	char path[64];
+	FILE *f;
+
+	if (!get_pidfd_info(fd_peer_pidfd, &info))
+		return -1;
+
+	snprintf(path, sizeof(path), "/proc/%d/statm", info.pid);
+	f = fopen(path, "r");
+	if (!f) {
+		fprintf(stderr, "%s: %s: %m\n", __func__, path);
+		return -1;
+	}
+
+	if (fscanf(f, "%lu", &pages) != 1) {
+		fprintf(stderr, "%s: %s: no size\n", __func__, path);
+		fclose(f);
+		return -1;
+	}
+	fclose(f);
+
+	return (ssize_t)pages * sysconf(_SC_PAGESIZE);
+}
+
 /* Protocol helper functions */
 
 ssize_t recv_marker(int fd)
diff --git a/tools/testing/selftests/coredump/coredump_test_helpers.h b/tools/testing/selftests/coredump/coredump_test_helpers.h
index 00d695b67b3f..97ad5cfeae92 100644
--- a/tools/testing/selftests/coredump/coredump_test_helpers.h
+++ b/tools/testing/selftests/coredump/coredump_test_helpers.h
@@ -18,6 +18,9 @@
 /* Size of the mostly unpopulated mapping the sparse coredump test maps. */
 #define SPARSE_MAPPING_SIZE (256 * 1024 * 1024)
 
+/* A task mapping at least this much is worth a record stream. */
+#define SPARSE_STREAM_THRESHOLD (SPARSE_MAPPING_SIZE / 2)
+
 /* Shared helper function declarations */
 void *do_nothing(void *arg);
 void crashing_child(void);
@@ -25,9 +28,11 @@ void crashing_child_sparse(size_t size);
 ssize_t recv_coredump_records(int fd_coredump, int fd_core_file,
 			      off_t *coredump_size, bool *truncated,
 			      int fd_peer_pidfd);
-bool is_elf_core(int fd);
 ssize_t recv_coredump_compact(int fd_coredump, int fd_object, int fd_reference,
 			      off_t *coredump_size);
+ssize_t recv_coredump_bytes(int fd_coredump, int fd_core_file);
+ssize_t peer_vm_size(int fd_peer_pidfd);
+bool is_elf_core(int fd);
 int check_compact_coredump(int fd_object, int fd_reference);
 int create_detached_tmpfs(void);
 int create_and_listen_unix_socket(const char *path);

-- 
2.53.0