On 1.09.21 г. 20:01, Omar Sandoval wrote:
> From: Boris Burkov <boris@xxxxxx>
>
> In send stream v2, write commands can now be an arbitrary size. For that
> 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 <boris@xxxxxx>
> ---
> common/send-stream.c | 55 ++++++++++++++++++++++++++++----------------
> send.h | 2 +-
> 2 files changed, 36 insertions(+), 21 deletions(-)
>
<snip>
> @@ -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>
