btrfs: initialize variable cancel
PATCH 1/1 btrfs: initialize variable cancel
From: Tom Rix <[email protected]>
Clang static analysis reports this problem ioctl.c:3333:8: warning: 3rd function call argument is an
uninitialized value
ret = exclop_start_or_cancel_reloc(fs_info,
cancel is only set in one branch of an if-check and is always used. So initialize to false.
--- fs/btrfs/ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/btrfs/ioctl.c +1 -1
@@ -3308,7 +3308,7 @@ static long btrfs_ioctl_rm_dev(struct file *file, void __user *arg)struct block_device *bdev =NULL ;fmode_t mode;int ret; -bool cancel; +bool cancel = false;if (!capable (CAP_SYS_ADMIN ))return -EPERM ;
Signature · 1 line
2.26.3
Quoted 7 lines · On Fri, Jan 21, 2022 at 05:45:22AM -0800, [email protected] wrote:
From: Tom Rix <[email protected]>
Clang static analysis reports this problem ioctl.c:3333:8: warning: 3rd function call argument is an
uninitialized value
ret = exclop_start_or_cancel_reloc(fs_info,
Quoted 5 lines
cancel is only set in one branch of an if-check and is always used. So initialize to false.
Fixes 1a15eb724aae ("btrfs: use btrfs_get_dev_args_from_path in dev removal ioctls")Signed-off-by Tom Rix <[email protected]>
Could use a more precise subject like for example:
"btrfs: fix use of uninitialized variable at rm device ioctl"
Anyway, it looks good.
Thanks.
Quoted 20 lines
--- fs/btrfs/ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)fs/btrfs/ioctl.c +1
-1@@ -3308,7 +3308,7 @@ static long btrfs_ioctl_rm_dev(struct file *file, void __user *arg)struct block_device *bdev =NULL ;fmode_t mode;int ret; -bool cancel; +bool cancel = false;if (!capable (CAP_SYS_ADMIN ))return -EPERM ;Signature · 1 line
2.26.3
Quoted 21 lines · On 21/01/2022 21:45, [email protected] wrote:
From: Tom Rix <[email protected]>
Clang static analysis reports this problem ioctl.c:3333:8: warning: 3rd function call argument is an
uninitialized value
ret = exclop_start_or_cancel_reloc(fs_info,cancel is only set in one branch of an if-check and is always used. So initialize to false.
Fixes 1a15eb724aae ("btrfs: use btrfs_get_dev_args_from_path in dev removal ioctls")Signed-off-by Tom Rix <[email protected]>--- fs/btrfs/ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)fs/btrfs/ioctl.c +0
-0
fs/btrfs/ioctl.c static long btrfs_ioctl_rm_dev(struct file *file, void __user *arg) · line 3311
Quoted 5 lines
fs/btrfs/ioctl.c +1
-1@@ -3308,7 +3308,7 @@ static long btrfs_ioctl_rm_dev(struct file *file, void __user *arg)struct block_device *bdev =NULL ;fmode_t mode;int ret; -bool cancel; +bool cancel = false;
Thanks,
Anand
Quoted 3 lines
fs/btrfs/ioctl.c +0
-0if (!capable (CAP_SYS_ADMIN ))return -EPERM ;
Quoted 7 lines · On Fri, Jan 21, 2022 at 05:45:22AM -0800, [email protected] wrote:
From: Tom Rix <[email protected]>
Clang static analysis reports this problem ioctl.c:3333:8: warning: 3rd function call argument is an
uninitialized value
ret = exclop_start_or_cancel_reloc(fs_info,
Quoted 5 lines
cancel is only set in one branch of an if-check and is always used. So initialize to false.
Fixes 1a15eb724aae ("btrfs: use btrfs_get_dev_args_from_path in dev removal ioctls")Signed-off-by Tom Rix <[email protected]>
Added to misc-next, with the updted subject line, thanks.
Hi,
Mabye we should enable '-Wmaybe-uninitialized' for btrfs 'make W=1'.
There is another warning reproted by '-Wmaybe-uninitialized' in btrfs misc-next.
fs/btrfs/zoned.c:273:28: warning: ‘zno’ may be used uninitialized in this function [-Wmaybe-uninitialized]
memcpy(zinfo->zone_cache + zno, zones,
^
fs/btrfs/Makefile +1 -0
@@ -2,6 +2,7 @@ # Subset of W=1 warnings subdir-ccflags-y += -Wextra -Wunused -Wno-unused-parameter +subdir-ccflags-y += -Wmaybe-uninitialized subdir-ccflags-y += -Wmissing-declarations subdir-ccflags-y += -Wmissing-format-attribute subdir-ccflags-y += -Wmissing-prototypes
Best Regards
Wang Yugui ([email protected])
2022/01/25
Quoted 31 lines
From: Tom Rix <[email protected]>
Clang static analysis reports this problem ioctl.c:3333:8: warning: 3rd function call argument is an
uninitialized value
ret = exclop_start_or_cancel_reloc(fs_info,cancel is only set in one branch of an if-check and is always used. So initialize to false.
Fixes 1a15eb724aae ("btrfs: use btrfs_get_dev_args_from_path in dev removal ioctls")Signed-off-by Tom Rix <[email protected]>--- fs/btrfs/ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)fs/btrfs/ioctl.c +1
-1@@ -3308,7 +3308,7 @@ static long btrfs_ioctl_rm_dev(struct file *file, void __user *arg)struct block_device *bdev =NULL ;fmode_t mode;int ret; -bool cancel; +bool cancel = false;if (!capable (CAP_SYS_ADMIN ))return -EPERM ;Signature · 1 line
2.26.3
Quoted 11 lines · Wang Yugui
Hi,
Mabye we should enable '-Wmaybe-uninitialized' for btrfs 'make W=1'.
There is another warning reproted by '-Wmaybe-uninitialized' in btrfs misc-next.
fs/btrfs/zoned.c:273:28: warning: ‘zno’ may be used uninitialized in this function [-Wmaybe-uninitialized] memcpy(zinfo->zone_cache + zno, zones,
^
Isn't that one a false positive?
u32 zno;
[...]
/* Check cache */
if (zinfo->zone_cache) {
unsigned int i;
ASSERT(IS_ALIGNED(pos, zinfo->zone_size));
zno = pos >> zinfo->zone_size_shift;
[...]
} [...]
/* Populate cache */
if (zinfo->zone_cache)
memcpy(zinfo->zone_cache + zno, zones,
sizeof(*zinfo->zone_cache) * *nr_zones); Quoted 28 lines · Johannes Thumshirn
Quoted 11 lines · Wang Yugui
Hi,
Mabye we should enable '-Wmaybe-uninitialized' for btrfs 'make W=1'.
There is another warning reproted by '-Wmaybe-uninitialized' in btrfs misc-next.
fs/btrfs/zoned.c:273:28: warning: ‘zno’ may be used uninitialized in this function [-Wmaybe-uninitialized] memcpy(zinfo->zone_cache + zno, zones,
^Isn't that one a false positive?
u32 zno;[...]
/* Check cache */ if (zinfo->zone_cache) { unsigned int i; ASSERT(IS_ALIGNED(pos, zinfo->zone_size)); zno = pos >> zinfo->zone_size_shift; [...] }
Quoted 7 lines
[...]
/* Populate cache */ if (zinfo->zone_cache) memcpy(zinfo->zone_cache + zno, zones, sizeof(*zinfo->zone_cache) * *nr_zones);
Yeah looks like a false positive, maybe it can't reason that zinfo->zone_cache won't change between the conditions from NULL to valid pointer.
And as there are other warnings in sb_zone_number we can't set -Wmaybe-uninitialized by default. I think this could be even more problematic with older compiler versions.