[v2] reftable: avoid undefined behaviour breaking t0032

Commit Message

Carlo Marcelo Arenas Belón April 15, 2022, 8:30 a.m. UTC

1214aa841bc (reftable: add blocksource, an abstraction for random
access reads, 2021-10-07), makes the assumption that it is ok to
free a reftable_block pointing to NULL if the size is also set to
0, but implements that using a memset call that at least in glibc
based system will trigger a runtime exception if called with a
NULL pointer as its first parameter.

Avoid doing so by adding a conditional to check for the size in all
three identically looking functions that were affected, and therefore,
still allow memset to help catch callers that might incorrectly pass
a NULL pointer with a non zero size, but avoiding the exception for
the valid cases.

Signed-off-by: Carlo Marcelo Arenas Belón <[e-mail]>
---
Changes since v1:
- Improved logic as suggested by Junio
- Hopefully also improved commit message

 reftable/blocksource.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Han-Wen Nienhuys April 25, 2022, 10:18 a.m. UTC | #1

On Fri, Apr 15, 2022 at 10:34 AM Carlo Marcelo Arenas Belón
<[e-mail]> wrote:
> 1214aa841bc (reftable: add blocksource, an abstraction for random
> access reads, 2021-10-07), makes the assumption that it is ok to
> free a reftable_block pointing to NULL if the size is also set to
>..

either patch (data or len) LGTM

Patch

diff --git a/reftable/blocksource.c b/reftable/blocksource.c
index 0044eecd9aa..db1b7dc966f 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->len)
+		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->len)
+		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->len)
+		memset(dest->data, 0xff, dest->len);
 	reftable_free(dest->data);
 }