An overflow may occur if the function is called with the last
block and an offset greater than zero. It is necessary to add
a check to avoid this.

Overflow is also possible when we sum offset and
sizeof(struct spaceBitmapDesc) << 3. For this reason it
is necessary to check overflow of this too. The result is
stored in total_offset.

Found by Linux Verification Center (linuxtesting.org) with Svace.

Suggested-by: Jan Kara <jack@suse.com>
Signed-off-by: Roman Smirnov <r.smirnov@omp.ru>
---
 V1 -> V2: Overflow checks have been replaced by functions

 fs/udf/balloc.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/fs/udf/balloc.c b/fs/udf/balloc.c
index ab3ffc355949..4bf2720111f1 100644
--- a/fs/udf/balloc.c
+++ b/fs/udf/balloc.c
@@ -18,6 +18,7 @@
 #include "udfdecl.h"
 
 #include <linux/bitops.h>
+#include <linux/overflow.h>
 
 #include "udf_i.h"
 #include "udf_sb.h"
@@ -133,6 +134,7 @@ static void udf_bitmap_free_blocks(struct super_block *sb,
 	unsigned long block;
 	unsigned long block_group;
 	unsigned long bit;
+	unsigned int total_offset;
 	unsigned long i;
 	int bitmap_nr;
 	unsigned long overflow;
@@ -140,17 +142,20 @@ static void udf_bitmap_free_blocks(struct super_block *sb,
 	mutex_lock(&sbi->s_alloc_mutex);
 	partmap = &sbi->s_partmaps[bloc->partitionReferenceNum];
 	if (bloc->logicalBlockNum + count < count ||
-	    (bloc->logicalBlockNum + count) > partmap->s_partition_len) {
-		udf_debug("%u < %d || %u + %u > %u\n",
+	    (bloc->logicalBlockNum + count) > partmap->s_partition_len ||
+	    check_add_overflow(offset,
+			       sizeof(struct spaceBitmapDesc) << 3, &total_offset) ||
+	    check_add_overflow(bloc->logicalBlockNum, total_offset, &block)) {
+		udf_debug("%u < %d || %u + %u > %u || %u + %zu > %u || %u + %u + %zu > %u\n",
 			  bloc->logicalBlockNum, 0,
 			  bloc->logicalBlockNum, count,
-			  partmap->s_partition_len);
+			  partmap->s_partition_len,
+			  offset, sizeof(struct spaceBitmapDesc) << 3, UINT_MAX,
+			  bloc->logicalBlockNum, offset,
+			  sizeof(struct spaceBitmapDesc) << 3, UINT_MAX);
 		goto error_return;
 	}
 
-	block = bloc->logicalBlockNum + offset +
-		(sizeof(struct spaceBitmapDesc) << 3);
-
 	do {
 		overflow = 0;
 		block_group = block >> (sb->s_blocksize_bits + 3);
-- 
2.34.1

On Thu 20-06-24 10:24:13, Roman Smirnov wrote:
> An overflow may occur if the function is called with the last
> block and an offset greater than zero. It is necessary to add
> a check to avoid this.
> 
> Overflow is also possible when we sum offset and
> sizeof(struct spaceBitmapDesc) << 3. For this reason it
> is necessary to check overflow of this too. The result is
> stored in total_offset.
> 
> Found by Linux Verification Center (linuxtesting.org) with Svace.
> 
> Suggested-by: Jan Kara <jack@suse.com>
> Signed-off-by: Roman Smirnov <r.smirnov@omp.ru>

Thanks for the patch. In the end I've noticed that unalloc table block
freeing has the same overflow checks and I've decided to move bitmap offset
overflow verification into mount code (so that any bitmap offset for a
block within a partition cannot overflow u32). The resulting patches are
attached for reference and I've queued them in my tree.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR