linux-kernel - Re: [PATCH] bcachefs: initialize local variables in bch2_evacuate

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241118010040.GB3588455@thelio-3990X>
Date: Sun, 17 Nov 2024 18:00:40 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Piotr Zalewski <pZ010001011111@...ton.me>, kent.overstreet@...ux.dev
Cc: ndesaulniers@...gle.com, morbo@...gle.com, justinstitt@...gle.com,
	linux-bcachefs@...r.kernel.org, linux-kernel@...r.kernel.org,
	llvm@...ts.linux.dev, skhan@...uxfoundation.org
Subject: Re: [PATCH] bcachefs: initialize local variables in
 bch2_evacuate_bucket

On Sun, Nov 17, 2024 at 11:48:23PM +0000, Piotr Zalewski wrote:
> Compiling bcachefs sources with LLVM triggers uninitialized variables
> warnings.
> 
> Signed-off-by: Piotr Zalewski <pZ010001011111@...ton.me>
> ---
>  fs/bcachefs/move.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/bcachefs/move.c b/fs/bcachefs/move.c
> index 8c032ef3a567..94cac498d372 100644
> --- a/fs/bcachefs/move.c
> +++ b/fs/bcachefs/move.c
> @@ -674,8 +674,8 @@ int bch2_evacuate_bucket(struct moving_context *ctxt,
>  	struct bkey_buf sk;
>  	struct bkey_s_c k;
>  	struct data_update_opts data_opts;
> -	unsigned dirty_sectors, bucket_size;
> -	u64 fragmentation;
> +	unsigned dirty_sectors = 0, bucket_size = 0;
> +	u64 fragmentation = 0;

Shouldn't these just be removed altogether...? They are only used in a
trace event and now only zero.

$ rg 'dirty_sectors|bucket_size|fragmentation' fs/bcachefs/move.c
677:    unsigned dirty_sectors, bucket_size;
678:    u64 fragmentation;
814:    trace_evacuate_bucket(c, &bucket, dirty_sectors, bucket_size, fragmentation, ret);

Something like below seems to compile.

Cheers,
Nathan

diff --git a/fs/bcachefs/move.c b/fs/bcachefs/move.c
index 88ab9d7e1a1b..412ef9897361 100644
--- a/fs/bcachefs/move.c
+++ b/fs/bcachefs/move.c
@@ -674,8 +674,6 @@ int bch2_evacuate_bucket(struct moving_context *ctxt,
 	struct bkey_buf sk;
 	struct bkey_s_c k;
 	struct data_update_opts data_opts;
-	unsigned dirty_sectors, bucket_size;
-	u64 fragmentation;
 	int ret = 0;
 
 	struct bch_dev *ca = bch2_dev_tryget(c, bucket.inode);
@@ -811,7 +809,7 @@ int bch2_evacuate_bucket(struct moving_context *ctxt,
 		bch2_btree_iter_advance(&bp_iter);
 	}
 
-	trace_evacuate_bucket(c, &bucket, dirty_sectors, bucket_size, fragmentation, ret);
+	trace_evacuate_bucket(c, &bucket, ret);
 err:
 	bch2_trans_iter_exit(trans, &bp_iter);
 	bch2_dev_put(ca);
diff --git a/fs/bcachefs/trace.h b/fs/bcachefs/trace.h
index 5597b9d6297f..f67914367acf 100644
--- a/fs/bcachefs/trace.h
+++ b/fs/bcachefs/trace.h
@@ -846,18 +846,13 @@ TRACE_EVENT(move_data,
 );
 
 TRACE_EVENT(evacuate_bucket,
-	TP_PROTO(struct bch_fs *c, struct bpos *bucket,
-		 unsigned sectors, unsigned bucket_size,
-		 u64 fragmentation, int ret),
-	TP_ARGS(c, bucket, sectors, bucket_size, fragmentation, ret),
+	TP_PROTO(struct bch_fs *c, struct bpos *bucket, int ret),
+	TP_ARGS(c, bucket, ret),
 
 	TP_STRUCT__entry(
 		__field(dev_t,		dev		)
 		__field(u64,		member		)
 		__field(u64,		bucket		)
-		__field(u32,		sectors		)
-		__field(u32,		bucket_size	)
-		__field(u64,		fragmentation	)
 		__field(int,		ret		)
 	),
 
@@ -865,17 +860,12 @@ TRACE_EVENT(evacuate_bucket,
 		__entry->dev			= c->dev;
 		__entry->member			= bucket->inode;
 		__entry->bucket			= bucket->offset;
-		__entry->sectors		= sectors;
-		__entry->bucket_size		= bucket_size;
-		__entry->fragmentation		= fragmentation;
 		__entry->ret			= ret;
 	),
 
-	TP_printk("%d,%d %llu:%llu sectors %u/%u fragmentation %llu ret %i",
+	TP_printk("%d,%d %llu:%llu ret %i",
 		  MAJOR(__entry->dev), MINOR(__entry->dev),
-		  __entry->member, __entry->bucket,
-		  __entry->sectors, __entry->bucket_size,
-		  __entry->fragmentation, __entry->ret)
+		  __entry->member, __entry->bucket, __entry->ret)
 );
 
 TRACE_EVENT(copygc,