2021-04-13 15:23:56

by Khaled Romdhani

[permalink] [raw]
Subject: [PATCH-next] fs/btrfs: Fix uninitialized variable

The variable zone is not initialized. It
may causes a failed assertion.

Addresses-Coverity: ("Uninitialized variables")

Signed-off-by: Khaled ROMDHANI <[email protected]>
---
fs/btrfs/zoned.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index eeb3ebe11d7a..ee15ab8dccb5 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -136,7 +136,7 @@ static int sb_write_pointer(struct block_device *bdev, struct blk_zone *zones,
*/
static inline u32 sb_zone_number(int shift, int mirror)
{
- u64 zone;
+ u64 zone = 0;

ASSERT(mirror < BTRFS_SUPER_MIRROR_MAX);
switch (mirror) {
--
2.17.1


2021-04-13 22:08:20

by Boris Burkov

[permalink] [raw]
Subject: Re: [PATCH-next] fs/btrfs: Fix uninitialized variable

On Tue, Apr 13, 2021 at 02:06:04PM +0100, Khaled ROMDHANI wrote:
> The variable zone is not initialized. It
> may causes a failed assertion.
>
> Addresses-Coverity: ("Uninitialized variables")
>
> Signed-off-by: Khaled ROMDHANI <[email protected]>

Reviewed-by: Boris Burkov <[email protected]>

> ---
> fs/btrfs/zoned.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index eeb3ebe11d7a..ee15ab8dccb5 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -136,7 +136,7 @@ static int sb_write_pointer(struct block_device *bdev, struct blk_zone *zones,
> */
> static inline u32 sb_zone_number(int shift, int mirror)
> {
> - u64 zone;
> + u64 zone = 0;
>
> ASSERT(mirror < BTRFS_SUPER_MIRROR_MAX);

Thanks for the fix.

I assume this was dug up by coverity static analysis rather than hitting
it in a live system?

Since there is already an assert for the pre-condition 'mirror < max',
I feel like it would make sense to also add one for mirror > 0.

> switch (mirror) {
> --
> 2.17.1
>

2021-04-16 20:41:36

by David Sterba

[permalink] [raw]
Subject: Re: [PATCH-next] fs/btrfs: Fix uninitialized variable

On Tue, Apr 13, 2021 at 02:06:04PM +0100, Khaled ROMDHANI wrote:
> The variable zone is not initialized. It
> may causes a failed assertion.

Failed assertion means the 2nd one checking that the result still fits
to 32bit type. That would mean that none of the cases were hit, but all
callers pass valid values.

It would be better to add a default: case to catch that explicitly,
though hitting that is considered 'will not happen'.

2021-04-17 11:51:25

by Khaled Romdhani

[permalink] [raw]
Subject: Re: [PATCH-next] fs/btrfs: Fix uninitialized variable

On Fri, Apr 16, 2021 at 07:32:03PM +0200, David Sterba wrote:
> On Tue, Apr 13, 2021 at 02:06:04PM +0100, Khaled ROMDHANI wrote:
> > The variable zone is not initialized. It
> > may causes a failed assertion.
>
> Failed assertion means the 2nd one checking that the result still fits
> to 32bit type. That would mean that none of the cases were hit, but all
> callers pass valid values.
>
> It would be better to add a default: case to catch that explicitly,
> though hitting that is considered 'will not happen'.

Yes. I will send a V2.