Skip to contentSkip to conversation

Btrfs: fix an integer overflow check

PATCH In review Dan Carpenter 2 participants 2 messages Mar 17, 2017 – Mar 27, 2017

Subsystems (2)
Kernel janitors Btrfs file system
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 Btrfs: fix an integer overflow check

This isn't super serious because you need CAP_ADMIN to run this code.

I added this integer overflow check last year but apparently I am rubbish at writing integer overflow checks... There are two issues. First, access_ok() works on unsigned long type and not u64 so on 32 bit systems the access_ok() could be checking a truncated size. The other issue is that we should be using a stricter limit so we don't overflow the kzalloc() setting ctx->clone_roots later in the function after the access_ok():

	alloc_size = sizeof(struct clone_root) * (arg->clone_sources_count + 1);
	sctx->clone_roots = kzalloc(alloc_size, GFP_KERNEL | __GFP_NOWARN);
Fixes f5ecec3ce21f ("btrfs: send: silence an integer overflow warning")
Signed-off-by Dan Carpenter <[email protected]>
fs/btrfs/send.c +1 -1
index 030d592ed1fe..ad9508e67384 100644
@@ -6306,7 +6306,7 @@ long btrfs_ioctl_send(struct file *mnt_file, void __user *arg_)
 	}
 
 	if (arg->clone_sources_count >
-	    ULLONG_MAX / sizeof(*arg->clone_sources)) {
+	    ULONG_MAX / sizeof(struct clone_root) - 1) {
 		ret = -EINVAL;
 		goto out;
 	}
Reviewed-by on v1 1/1
#1 lore raw
Quoted 10 lines · Dan Carpenter

This isn't super serious because you need CAP_ADMIN to run this code.

I added this integer overflow check last year but apparently I am rubbish at writing integer overflow checks... There are two issues. First, access_ok() works on unsigned long type and not u64 so on 32 bit systems the access_ok() could be checking a truncated size. The other issue is that we should be using a stricter limit so we don't overflow the kzalloc() setting ctx->clone_roots later in the function after the access_ok():

Quoted 5 lines
	alloc_size = sizeof(struct clone_root) * (arg->clone_sources_count + 1);
	sctx->clone_roots = kzalloc(alloc_size, GFP_KERNEL | __GFP_NOWARN);
Fixes f5ecec3ce21f ("btrfs: send: silence an integer overflow warning")
Signed-off-by Dan Carpenter <[email protected]>
Reviewed-by David Sterba <[email protected]>

I'll copy parts of the changelog and add as comments, it's not obvious what the check does.