Use dynamic memory allocation instead of variable-length arrays

Since it is easy to trigger a stack overflow with variable-length arrays,
use dynamic memory allocation instead of VLAs. Add -Wvla to the compiler
options such that no new VLAs get introduced.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>

configure.ac

@@ -28,7 +28,7 @@ AC_ARG_ENABLE([werror], [AS_HELP_STRING([--disable-werror],
               [Disables building with -Werror by default])])
 
 if test "$ac_cv_prog_gcc" = yes; then
-   WARN_CFLAGS="-Wall -W -Wshadow -Wstrict-prototypes -Wpointer-arith -Wcast-align -Wno-strict-aliasing"
+   WARN_CFLAGS="-Wall -W -Wshadow -Wstrict-prototypes -Wpointer-arith -Wcast-align -Wno-strict-aliasing -Wvla"
    WARN_CFLAGS="$WARN_CFLAGS -Wno-unknown-warning-option -Wno-stringop-truncation"
    if test "x$enable_werror" != "xno"; then
        WARN_CFLAGS="$WARN_CFLAGS -Werror"

test-tool/iscsi-support.c

@@ -329,8 +329,8 @@ static struct scsi_task *send_scsi_command(struct scsi_device *sdev, struct scsi
 #ifdef HAVE_SG_IO
         if (sdev->sgio_dev) {
                 sg_io_hdr_t io_hdr;
-                unsigned int sense_len=32;
-                unsigned char sense[sense_len];
+                unsigned char sense[32];
+                const unsigned int sense_len = sizeof(sense);
                 char buf[1024];
 
                 memset(sense, 0, sizeof(sense));

test-tool/test_async_abort_simple.c

@@ -94,7 +94,7 @@ test_async_abort_simple(void)
 	struct tests_async_abort_state state = { NULL, 0, 0, 0, 0 };
 	int blocksize = 512;
 	int blocks_per_io = 8;
-	unsigned char buf[blocksize * blocks_per_io];
+	unsigned char *buf;
 	uint64_t timeout_sec;
 
 	CHECK_FOR_DATALOSS;
@@ -107,7 +107,10 @@ test_async_abort_simple(void)
 		return;
 	}
 
-	memset(buf, 0, blocksize * blocks_per_io);
+	buf = calloc(blocksize, blocks_per_io);
+	CU_ASSERT(buf != NULL);
+	if (!buf)
+		return;
 
 	/* queue and dispatch write before the abort */
 	state.wtask = scsi_cdb_write10(0, blocks_per_io * blocksize,
@@ -205,4 +208,6 @@ test_async_abort_simple(void)
         iscsi_logout_sync(sd->iscsi_ctx);
         iscsi_destroy_context(sd->iscsi_ctx);
 	sd->iscsi_ctx = NULL;
+
+	free(buf);
 }

test-tool/test_async_lu_reset_simple.c

@@ -93,7 +93,7 @@ test_async_lu_reset_simple(void)
 	struct tests_async_reset_state state = { NULL, 0, 0, 0, };
 	int blocksize = 512;
 	int blocks_per_io = 8;
-	unsigned char buf[blocksize * blocks_per_io];
+	unsigned char *buf;
 	uint64_t timeout_sec;
 
 	CHECK_FOR_DATALOSS;
@@ -106,7 +106,10 @@ test_async_lu_reset_simple(void)
 		return;
 	}
 
-	memset(buf, 0, blocksize * blocks_per_io);
+	buf = calloc(blocksize, blocks_per_io);
+	CU_ASSERT(buf != NULL);
+	if (!buf)
+		return;
 
 	/* queue and dispatch write before the reset */
 	state.wtask = scsi_cdb_write10(0, blocks_per_io * blocksize,
@@ -202,4 +205,6 @@ out:
         iscsi_logout_sync(sd->iscsi_ctx);
         iscsi_destroy_context(sd->iscsi_ctx);
 	sd->iscsi_ctx = NULL;
+
+	free(buf);
 }

test-tool/test_async_write.c

@@ -62,7 +62,7 @@ test_async_write(void)
 	int blocks_per_io = 8;
 	int num_ios = 1000;
 	/* IOs in flight concurrently, but all using the same src buffer */
-	unsigned char buf[block_size * blocks_per_io];
+	unsigned char *buf;
 
 	CHECK_FOR_DATALOSS;
 	CHECK_FOR_SBC;
@@ -74,7 +74,10 @@ test_async_write(void)
 		return;
 	}
 
-	memset(buf, 0, block_size * blocks_per_io);
+	buf = calloc(block_size, blocks_per_io);
+	CU_ASSERT(buf != NULL);
+	if (!buf)
+		return;
 
 	for (i = 0; i < num_ios; i++) {
 		uint32_t lba = i * blocks_per_io;
@@ -111,4 +114,6 @@ test_async_write(void)
 		ret = iscsi_service(sd->iscsi_ctx, pfd.revents);
 		CU_ASSERT_EQUAL(ret, 0);
 	}
+
+	free(buf);
 }

test-tool/test_compareandwrite_unwritten.c

@@ -31,6 +31,7 @@
 void
 test_compareandwrite_unwritten(void)
 {
+	unsigned char *read_buf;
 	int i, n;
 
 	CHECK_FOR_DATALOSS;
@@ -39,13 +40,17 @@ test_compareandwrite_unwritten(void)
         n = 256;
         if (n + 0U > num_blocks)
                 n = num_blocks;
-        
+
+	read_buf = malloc(block_size);
+	CU_ASSERT(read_buf != NULL);
+	if (!read_buf)
+		return;
+
 	logging(LOG_VERBOSE, LOG_BLANK_LINE);
 	logging(LOG_VERBOSE, "Test COMPARE_AND_WRITE of 1-%d blocks at the "
 		"start of the LUN, 1 block at a time", n);
 	for (i = 1; i <= n; i++) {
 		int caw_ret;
-		unsigned char read_buf[block_size];
 
 		/* semi-unique 'compare' buffer to trigger miscompare */
 		memset(scratch, '%', block_size);
@@ -78,4 +83,6 @@ test_compareandwrite_unwritten(void)
 					       block_size), 0);
 		}
 	}
+
+	free(read_buf);
 }

test-tool/test_multipathio_async_caw.c

@@ -105,14 +105,28 @@ test_mpio_async_caw(void)
 	struct test_mpio_async_caw_state state = { 0, 0, 0 };
 	int num_ios = 1000;
 	uint32_t lba = 0;
-	unsigned char cmp_buf[block_size * mp_num_sds];
-	unsigned char wr_buf[block_size * mp_num_sds];
+	unsigned char *cmp_buf;
+	unsigned char *wr_buf;
+	struct pollfd *pfd;
 
 	CHECK_FOR_DATALOSS;
 	CHECK_FOR_SBC;
 	MPATH_SKIP_IF_UNAVAILABLE(mp_sds, mp_num_sds);
 	MPATH_SKIP_UNLESS_ISCSI(mp_sds, mp_num_sds);
 
+	cmp_buf = malloc(block_size * mp_num_sds);
+	CU_ASSERT(cmp_buf != NULL);
+	if (!cmp_buf)
+		return;
+	wr_buf = malloc(block_size * mp_num_sds);
+	CU_ASSERT(wr_buf != NULL);
+	if (!wr_buf)
+		goto free_cmp_buf;
+	pfd = malloc(mp_num_sds * sizeof(struct pollfd));
+	CU_ASSERT(pfd != NULL);
+	if (!pfd)
+		goto free_wr_buf;
+
 	/* synchronously initialise zeros for first CAW */
 	memset(wr_buf, 0, block_size);
 	WRITESAME10(mp_sds[0], 0, block_size, 1, 0, 0, 0, 0, wr_buf,
@@ -157,8 +171,6 @@ test_mpio_async_caw(void)
 	}
 
 	while (state.completed < state.dispatched) {
-		struct pollfd pfd[mp_num_sds];
-
 		for (sd_i = 0; sd_i < mp_num_sds; sd_i++) {
 			pfd[sd_i].fd = iscsi_get_fd(mp_sds[sd_i]->iscsi_ctx);
 			pfd[sd_i].events
@@ -180,4 +192,9 @@ test_mpio_async_caw(void)
 
 	logging(LOG_VERBOSE, "[OK] All %d COMPARE_AND_WRITE IOs complete, with "
 		"%d mismatch(es)", state.completed, state.mismatches);
+
+free_wr_buf:
+	free(wr_buf);
+free_cmp_buf:
+	free(cmp_buf);
 }

test-tool/test_writesame10_check.c

@@ -32,7 +32,7 @@ test_writesame10_check(void)
 {
 	int i;
 	int ws_max_blocks = 256;
-	unsigned char read_buf[ws_max_blocks * block_size];
+	unsigned char *read_buf;
 
 	CHECK_FOR_DATALOSS;
 	CHECK_FOR_SBC;
@@ -40,6 +40,11 @@ test_writesame10_check(void)
 	logging(LOG_VERBOSE, LOG_BLANK_LINE);
 	logging(LOG_VERBOSE, "Test WRITESAME10 of 1-256 blocks at the start of the LUN");
 
+	read_buf = malloc(ws_max_blocks * block_size);
+	CU_ASSERT(read_buf != NULL);
+	if (!read_buf)
+		return;
+
 	for (i = 1; i <= ws_max_blocks; i++) {
 		/*
 		 * fill the full buffer so that memcmp is straightforward,
@@ -71,4 +76,6 @@ test_writesame10_check(void)
 
 		CU_ASSERT_EQUAL(0, memcmp(read_buf, scratch, i));
 	}
+
+	free(read_buf);
 }

test-tool/test_writesame16_check.c

@@ -32,7 +32,7 @@ test_writesame16_check(void)
 {
 	int i;
 	int ws_max_blocks = 256;
-	unsigned char read_buf[ws_max_blocks * block_size];
+	unsigned char *read_buf;
 
 	CHECK_FOR_DATALOSS;
 	CHECK_FOR_SBC;
@@ -40,6 +40,11 @@ test_writesame16_check(void)
 	logging(LOG_VERBOSE, LOG_BLANK_LINE);
 	logging(LOG_VERBOSE, "Test WRITESAME16 of 1-256 blocks at the start of the LUN");
 
+	read_buf = malloc(ws_max_blocks * block_size);
+	CU_ASSERT(read_buf != NULL);
+	if (!read_buf)
+		return;
+
 	for (i = 1; i <= ws_max_blocks; i++) {
 		/*
 		 * fill the full buffer so that memcmp is straightforward,
@@ -71,4 +76,6 @@ test_writesame16_check(void)
 
 		CU_ASSERT_EQUAL(0, memcmp(read_buf, scratch, i));
 	}
+
+	free(read_buf);
 }