On 20.10.21 г. 17:09, Nikolay Borisov wrote:

On 1.09.21 г. 20:01, Omar Sandoval wrote:

quoted

From: Boris Burkov <redacted>

In send stream v2, write commands can now be an arbitrary size. For that

nit: Actually can't commands really be up-to BTRFS_MAX_COMPRESSED + 16K
really  or are we going to leave this as an implementation detail? I'm
fine either way but looking at the changelog of patch 12 in the kernel
series doesn't really mention of arbitrary size, instead it explicitly
is talking about sending the max compressed extent size (128K) + some
space for metadata (the 16K above).

quoted

reason, we can no longer allocate a fixed array in sctx for read_cmd.
Instead, read_cmd dynamically allocates sctx->read_buf. To avoid
needless reallocations, we reuse read_buf between read_cmd calls by also
keeping track of the size of the allocated buffer in sctx->read_buf_sz.

We do the first allocation of the old default size at the start of
processing the stream, and we only reallocate if we encounter a command
that needs a larger buffer.

Signed-off-by: Boris Burkov <redacted>
---
 common/send-stream.c | 55 ++++++++++++++++++++++++++++----------------
 send.h               |  2 +-
 2 files changed, 36 insertions(+), 21 deletions(-)

<snip>

quoted

@@ -124,18 +125,22 @@ static int read_cmd(struct btrfs_send_stream *sctx)
 		goto out;
 	}
 
-	sctx->cmd_hdr = (struct btrfs_cmd_header *)sctx->read_buf;
-	cmd = le16_to_cpu(sctx->cmd_hdr->cmd);
-	cmd_len = le32_to_cpu(sctx->cmd_hdr->len);
-
-	if (cmd_len + sizeof(*sctx->cmd_hdr) >= sizeof(sctx->read_buf)) {
-		ret = -EINVAL;
-		error("command length %u too big for buffer %zu",
-				cmd_len, sizeof(sctx->read_buf));
-		goto out;
+	cmd_hdr = (struct btrfs_cmd_header *)sctx->read_buf;
+	cmd_len = le32_to_cpu(cmd_hdr->len);
+	cmd = le16_to_cpu(cmd_hdr->cmd);
+	buf_len = sizeof(*cmd_hdr) + cmd_len;
+	if (sctx->read_buf_sz < buf_len) {
+		sctx->read_buf = realloc(sctx->read_buf, buf_len);
+		if (!sctx->read_buf) {

nit: This is prone to a memory leak, because according to
https://en.cppreference.com/w/c/memory/realloc

If there is not enough memory, the old memory block is not freed and
null pointer is returned.


This means if realloc fails it will overwrite sctx->read_buf with NULL,
yet the old memory won't be freed which will cause a memory leak. It can
be argued that's not critical since we'll very quickly terminate the
program afterwards but still.


<snip>