Thread (11 messages) 11 messages, 5 authors, 2025-11-19

Re: [Intel-wired-lan] [PATCH v3] net: ethernet: fix uninitialized pointers with free attribute

From: Dan Carpenter <hidden>
Date: 2025-11-17 13:11:39
Also in: intel-wired-lan, linux-hyperv, lkml

On Thu, Nov 06, 2025 at 03:07:26PM +0100, Alexander Lobakin wrote:
quoted
diff --git a/drivers/net/ethernet/intel/ice/ice_flow.c b/drivers/net/ethernet/intel/ice/ice_flow.c
index 6d5c939dc8a515c252cd2b77d155b69fa264ee92..3590dacf3ee57879b3809d715e40bb290e40c4aa 100644
--- a/drivers/net/ethernet/intel/ice/ice_flow.c
+++ b/drivers/net/ethernet/intel/ice/ice_flow.c
@@ -1573,12 +1573,13 @@ ice_flow_set_parser_prof(struct ice_hw *hw, u16 dest_vsi, u16 fdir_vsi,
 			 struct ice_parser_profile *prof, enum ice_block blk)
 {
 	u64 id = find_first_bit(prof->ptypes, ICE_FLOW_PTYPE_MAX);
-	struct ice_flow_prof_params *params __free(kfree);
 	u8 fv_words = hw->blk[blk].es.fvw;
 	int status;
 	int i, idx;
 
-	params = kzalloc(sizeof(*params), GFP_KERNEL);
+	struct ice_flow_prof_params *params __free(kfree) =
+		kzalloc(sizeof(*params), GFP_KERNEL);
Please don't do it that way. It's not C++ with RAII and
declare-where-you-use.
Just leave the variable declarations where they are, but initialize them
with `= NULL`.

Variable declarations must be in one block and sorted from the longest
to the shortest.
These days, with __free the trend is to say yes this is RAII and we
should declare it where you use it.  I personally don't have a strong
opinion on this either way, but other maintainers do and will NAK the
`= NULL` approach.

The documentation says you should do it that way and avoid the `= NULL`
as well.  The issue is with lock ordering.  It's a FILO ordering, so if
we require a specific unlock order then declaring variables at the top
could mess things up.

The counter argument is that if you declare a variable after a goto
then that's undefined behavior as well.  Clang will detect that bug so
it be detected before it hits actual users.

regards,
dan carpenter
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help