On 12/7/2021 12:45 PM, Han-Wen Nienhuys via GitGitGadget wrote:
> From: Han-Wen Nienhuys <hanwen@xxxxxxxxxx>
>
> This would be triggered by corrupt files, so it doesn't have test coverage. This
> was discovered by a Coverity scan.
>
> Signed-off-by: Han-Wen Nienhuys <hanwen@xxxxxxxxxx>
> ---
> reftable/block.c | 25 ++++++++++++++++++-------
> 1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/reftable/block.c b/reftable/block.c
> index 855e3f5c947..d7347bb3152 100644
> --- a/reftable/block.c
> +++ b/reftable/block.c
> @@ -188,13 +188,16 @@ int block_reader_init(struct block_reader *br, struct reftable_block *block,
> uint32_t full_block_size = table_block_size;
> uint8_t typ = block->data[header_off];
> uint32_t sz = get_be24(block->data + header_off + 1);
> -
> + int err = 0;
> uint16_t restart_count = 0;
> uint32_t restart_start = 0;
> uint8_t *restart_bytes = NULL;
> + uint8_t *uncompressed = NULL;
You define this here...
>
> - if (!reftable_is_block_type(typ))
> - return REFTABLE_FORMAT_ERROR;
> + if (!reftable_is_block_type(typ)) {
> + err = REFTABLE_FORMAT_ERROR;
> + goto done;
> + }
>
> if (typ == BLOCK_TYPE_LOG) {
> int block_header_skip = 4 + header_off;
> @@ -213,15 +216,19 @@ int block_reader_init(struct block_reader *br, struct reftable_block *block,
> uncompress2(uncompressed + block_header_skip, &dst_len,
> block->data + block_header_skip, &src_len)) {
> reftable_free(uncompressed);
But it is already used here, because it is defined within the if()
block. You need to remove that definition, too, or your change here
does nothing.
...
> /* We're done with the input data. */
> reftable_block_done(block);
> block->data = uncompressed;
> + uncompressed = NULL;
For example, this nullifies the local version of uncompressed.
(I had to double-check that this even compiled, which was
surprising to me.)
> +done:
> + if (uncompressed) {
> + reftable_free(uncompressed);
> + }
> + return err;
Remove that local declaration and this will be a good patch.
Thanks,
-Stolee
