Re: [PATCH v11 02/10] btrfs-progs: receive: dynamically allocate sctx->read_buf

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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>




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux