| Message ID | 603bd1d4f6e4ceda02a5165eedd3ae5b92b8929e.1638899124.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Reftable coverity fixes | expand |
On 12/7/2021 12:45 PM, Han-Wen Nienhuys via GitGitGadget wrote: > From: Han-Wen Nienhuys <hanwen@google.com> > > 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@google.com> > --- > 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
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; - 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); - return REFTABLE_ZLIB_ERROR; + err = REFTABLE_ZLIB_ERROR; + goto done; } - if (dst_len + block_header_skip != sz) - return REFTABLE_FORMAT_ERROR; + if (dst_len + block_header_skip != sz) { + err = REFTABLE_FORMAT_ERROR; + goto done; + } /* We're done with the input data. */ reftable_block_done(block); block->data = uncompressed; + uncompressed = NULL; block->len = sz; block->source = malloc_block_source(); full_block_size = src_len + block_header_skip; @@ -251,7 +258,11 @@ int block_reader_init(struct block_reader *br, struct reftable_block *block, br->restart_count = restart_count; br->restart_bytes = restart_bytes; - return 0; +done: + if (uncompressed) { + reftable_free(uncompressed); + } + return err; } static uint32_t block_reader_restart_offset(struct block_reader *br, int i)