| Message ID | 20220415070236.25280-1-carenas@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | reftable: avoid undefined behaviour breaking t0032 | expand |
Carlo Marcelo Arenas Belón <carenas@gmail.com> writes: > At least in glibc based systems, memset with a NULL first parameter > will cause a runtime exception. I take it to mean that the code assumes that it is OK to pass NULL as long as length is 0 (i.e. filling the range of memory whose size is 0 with the specified byte can happen safely no matter what the starting address of that range is, as size==0 by definition should mean a no-op). That would mean we have a rule on how members of dest must be set: .data is allowed to be NULL only when .len is 0. If so, I wonder if we want to guard with dest->len instead, i.e. if (dest->len) memset(dest->data, 0xff, dest->len); With the form in this patch, i.e. > - memset(dest->data, 0xff, dest->len); > + if (dest->data) > + memset(dest->data, 0xff, dest->len); we will fail to catch a bogus caller that violates the rule above that we have on <data, len>. But if we guard with dest->len, then a violator of <data, len> rule will be caught by memset(). Thanks.
diff --git a/reftable/blocksource.c b/reftable/blocksource.c index 0044eecd9aa..984bf07fc17 100644 --- a/reftable/blocksource.c +++ b/reftable/blocksource.c @@ -15,7 +15,8 @@ license that can be found in the LICENSE file or at static void strbuf_return_block(void *b, struct reftable_block *dest) { - memset(dest->data, 0xff, dest->len); + if (dest->data) + memset(dest->data, 0xff, dest->len); reftable_free(dest->data); } @@ -56,7 +57,8 @@ void block_source_from_strbuf(struct reftable_block_source *bs, static void malloc_return_block(void *b, struct reftable_block *dest) { - memset(dest->data, 0xff, dest->len); + if (dest->data) + memset(dest->data, 0xff, dest->len); reftable_free(dest->data); } @@ -85,7 +87,8 @@ static uint64_t file_size(void *b) static void file_return_block(void *b, struct reftable_block *dest) { - memset(dest->data, 0xff, dest->len); + if (dest->data) + memset(dest->data, 0xff, dest->len); reftable_free(dest->data); }
At least in glibc based systems, memset with a NULL first parameter will cause a runtime exception. Avoid doing so by adding a conditional to check for NULL in all three identically looking functions that were affected. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> --- Bug was introduced with the original code in 1214aa841bc (reftable: add blocksource, an abstraction for random access reads, 2021-10-07), so not to be considered a regression for this release. reftable/blocksource.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)