From 42cb22f40c96f7df796436aba979721b88cf2105 Mon Sep 17 00:00:00 2001 From: Corey Hickey Date: Sun, 23 Jan 2022 18:53:13 -0800 Subject: [PATCH] badblocks: fix operation with large-ish block sizes and/or counts test_rw() and test_nd() need to allocate two or three times the product of the block size and the block counts. This can overflow the signed int type of block_size and result in allocate_buffer() being called with a value smaller than intended. Once that buffer is written to, badblocks segfaults. Since allocate_buffer() accepts a size_t, change the input validation to use SIZE_MAX and cast accordingly when calculating the argument. Fixing the segfault allows larger values to be passed to read() and write(); these need to be cast to size_t as well in order to avoid a signed integer overflow causing failure, in which case badblocks would fall back to testing a single block at once. Before: $ misc/badblocks -w -b 4096 -c 524288 -e 1 -s -v /tmp/testfile.bin Checking for bad blocks in read-write mode From block 0 to 524287 Segmentation fault $ misc/badblocks -n -b 4096 -c 524288 -e 1 -s -v /tmp/testfile.bin Checking for bad blocks in non-destructive read-write mode From block 0 to 524287 Checking for bad blocks (non-destructive read-write test) Segmentation fault After: $ misc/badblocks -w -b 4096 -c 524288 -e 1 -s -v /tmp/testfile.bin Checking for bad blocks in read-write mode From block 0 to 524287 Testing with pattern 0xaa: done Reading and comparing: done Testing with pattern 0x55: done Reading and comparing: done Testing with pattern 0xff: done Reading and comparing: done Testing with pattern 0x00: done Reading and comparing: done Pass completed, 0 bad blocks found. (0/0/0 errors) $ misc/badblocks -n -b 4096 -c 524288 -e 1 -s -v /tmp/testfile.bin Checking for bad blocks in non-destructive read-write mode From block 0 to 524287 Checking for bad blocks (non-destructive read-write test) Testing with random pattern: done Pass completed, 0 bad blocks found. (0/0/0 errors) Signed-off-by: Corey Hickey Signed-off-by: Theodore Ts'o --- misc/badblocks.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/misc/badblocks.c b/misc/badblocks.c index 0e74be8..2b5ff6d 100644 --- a/misc/badblocks.c +++ b/misc/badblocks.c @@ -389,7 +389,7 @@ static int do_read (int dev, unsigned char * buffer, int try, int block_size, /* Try the read */ if (d_flag) gettimeofday(&tv1, NULL); - got = read (dev, buffer, try * block_size); + got = read (dev, buffer, (size_t) try * block_size); if (d_flag) gettimeofday(&tv2, NULL); if (got < 0) @@ -460,7 +460,7 @@ static int do_write(int dev, unsigned char * buffer, int try, int block_size, com_err (program_name, errno, "%s", _("during seek")); /* Try the write */ - got = write (dev, buffer, try * block_size); + got = write (dev, buffer, (size_t) try * block_size); if (got < 0) got = 0; if (got & 511) @@ -510,9 +510,9 @@ static unsigned int test_ro (int dev, blk_t last_block, } while (next_bad && next_bad < first_block); if (t_flag) { - blkbuf = allocate_buffer((blocks_at_once + 1) * block_size); + blkbuf = allocate_buffer(((size_t) blocks_at_once + 1) * block_size); } else { - blkbuf = allocate_buffer(blocks_at_once * block_size); + blkbuf = allocate_buffer((size_t) blocks_at_once * block_size); } if (!blkbuf) { @@ -612,7 +612,7 @@ static unsigned int test_rw (int dev, blk_t last_block, /* set up abend handler */ capture_terminate(NULL); - buffer = allocate_buffer(2 * blocks_at_once * block_size); + buffer = allocate_buffer((size_t) 2 * blocks_at_once * block_size); read_buffer = buffer + blocks_at_once * block_size; if (!buffer) { @@ -771,7 +771,7 @@ static unsigned int test_nd (int dev, blk_t last_block, ext2fs_badblocks_list_iterate (bb_iter, &next_bad); } while (next_bad && next_bad < first_block); - blkbuf = allocate_buffer(3 * blocks_at_once * block_size); + blkbuf = allocate_buffer((size_t) 3 * blocks_at_once * block_size); test_record = malloc(blocks_at_once * sizeof(struct saved_blk_record)); if (!blkbuf || !test_record) { com_err(program_name, ENOMEM, "%s", @@ -1215,7 +1215,8 @@ int main (int argc, char ** argv) com_err(program_name, 0, _("Invalid number of blocks: %d\n"), blocks_at_once); exit(1); - } else if (((unsigned long long) block_size * blocks_at_once) > 0xFFFFFFFF) { + } else if (((size_t) block_size * blocks_at_once) > SIZE_MAX / 3) { + /* maximum usage is in test_nd() */ com_err(program_name, 0, _("For block size %d, number of blocks too large: %d\n"), block_size, blocks_at_once); exit(1); -- 1.8.3.1