[Intel-wired-lan] [PATCH RFT net-next 0/2] ethernet: intel: fix freeing uninitialized pointers with __free

[Intel-wired-lan] [PATCH RFT net-next 0/2] ethernet: intel: fix freeing uninitialized pointers with __free

[Ally Heev]

Uninitialized pointers with `__free` attribute can cause undefined
behavior as the memory assigned randomly to the pointer is freed
automatically when the pointer goes out of scope.

We could just fix it by initializing the pointer to NULL, but, as usage of
cleanup attributes is discouraged in net [1], trying to achieve cleanup
using goto

[1] https://docs.kernel.org/process/maintainer-netdev.html#using-device-managed-and-cleanup-h-constructs

Signed-off-by: Ally Heev <allyheev@gmail.com>
---
Ally Heev (2):
      ice: remove __free usage in ice_flow
      idpf: remove __free usage in idpf_virtchnl

 drivers/net/ethernet/intel/ice/ice_flow.c       |  6 ++++--
 drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 28 +++++++++++++++++--------
 2 files changed, 23 insertions(+), 11 deletions(-)
---
base-commit: 24598358a1b4ca1d596b8e7b34a7bc76f54e630f
change-id: 20251113-aheev-fix-free-uninitialized-ptrs-ethernet-intel-abc0cc9278d8

Best regards,
-- 
Ally Heev <allyheev@gmail.com>

[Intel-wired-lan] [PATCH RFT net-next 1/2] ice: remove __free usage in ice_flow

[Ally Heev]

usage of cleanup attributes is discouraged in net [1], achieve cleanup
using goto

Suggested-by: Simon Horman <horms@kernel.org>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/aPiG_F5EBQUjZqsl@stanley.mountain/
Signed-off-by: Ally Heev <allyheev@gmail.com>

[1] https://docs.kernel.org/process/maintainer-netdev.html#using-device-managed-and-cleanup-h-constructs

Signed-off-by: Ally Heev <allyheev@gmail.com>
---
 drivers/net/ethernet/intel/ice/ice_flow.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_flow.c b/drivers/net/ethernet/intel/ice/ice_flow.c
index 6d5c939dc8a515c252cd2b77d155b69fa264ee92..dd62f5f14d60401d6a24cb9f86664425db1532d0 100644
--- a/drivers/net/ethernet/intel/ice/ice_flow.c
+++ b/drivers/net/ethernet/intel/ice/ice_flow.c
@@ -1573,7 +1573,7 @@ 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);
+	struct ice_flow_prof_params *params = NULL;
 	u8 fv_words = hw->blk[blk].es.fvw;
 	int status;
 	int i, idx;
@@ -1621,12 +1621,14 @@ ice_flow_set_parser_prof(struct ice_hw *hw, u16 dest_vsi, u16 fdir_vsi,
 			      params->attr, params->attr_cnt,
 			      params->es, params->mask, false, false);
 	if (status)
-		return status;
+		goto out;
 
 	status = ice_flow_assoc_fdir_prof(hw, blk, dest_vsi, fdir_vsi, id);
 	if (status)
 		ice_rem_prof(hw, blk, id);
 
+out:
+	kfree(params);
 	return status;
 }
 

-- 
2.47.3

[Intel-wired-lan] [PATCH RFT net-next 2/2] idpf: remove __free usage in idpf_virtchnl

[Ally Heev]

usage of cleanup attributes is discouraged in net [1], achieve cleanup
using goto. In this patch though, only uninitialized pointers with __free
attribute are cleaned as they can cause undefined behavior when they
go out of scope

Suggested-by: Simon Horman <horms@kernel.org>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/aPiG_F5EBQUjZqsl@stanley.mountain/
Signed-off-by: Ally Heev <allyheev@gmail.com>

[1] https://docs.kernel.org/process/maintainer-netdev.html#using-device-managed-and-cleanup-h-constructs

Signed-off-by: Ally Heev <allyheev@gmail.com>
---
 drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 28 +++++++++++++++++--------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index cbb5fa30f5a0ec778c1ee30470da3ca21cc1af24..5b2bf8c3205bc1ea0746f78afa2a24f3f8ad2a8c 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -1012,7 +1012,7 @@ static int idpf_send_get_caps_msg(struct idpf_adapter *adapter)
  */
 static int idpf_send_get_lan_memory_regions(struct idpf_adapter *adapter)
 {
-	struct virtchnl2_get_lan_memory_regions *rcvd_regions __free(kfree);
+	struct virtchnl2_get_lan_memory_regions *rcvd_regions = NULL;
 	struct idpf_vc_xn_params xn_params = {
 		.vc_op = VIRTCHNL2_OP_GET_LAN_MEMORY_REGIONS,
 		.recv_buf.iov_len = IDPF_CTLQ_MAX_BUF_LEN,
@@ -1029,21 +1029,29 @@ static int idpf_send_get_lan_memory_regions(struct idpf_adapter *adapter)
 
 	xn_params.recv_buf.iov_base = rcvd_regions;
 	reply_sz = idpf_vc_xn_exec(adapter, &xn_params);
-	if (reply_sz < 0)
-		return reply_sz;
+	if (reply_sz < 0) {
+		err = reply_sz;
+		goto out;
+	}
 
 	num_regions = le16_to_cpu(rcvd_regions->num_memory_regions);
 	size = struct_size(rcvd_regions, mem_reg, num_regions);
-	if (reply_sz < size)
-		return -EIO;
+	if (reply_sz < size) {
+		err = -EIO;
+		goto out;
+	}
 
-	if (size > IDPF_CTLQ_MAX_BUF_LEN)
-		return -EINVAL;
+	if (size > IDPF_CTLQ_MAX_BUF_LEN) {
+		err = -EINVAL;
+		goto out;
+	}
 
 	hw = &adapter->hw;
 	hw->lan_regs = kcalloc(num_regions, sizeof(*hw->lan_regs), GFP_KERNEL);
-	if (!hw->lan_regs)
-		return -ENOMEM;
+	if (!hw->lan_regs) {
+		err = -ENOMEM;
+		goto out;
+	}
 
 	for (int i = 0; i < num_regions; i++) {
 		hw->lan_regs[i].addr_len =
@@ -1053,6 +1061,8 @@ static int idpf_send_get_lan_memory_regions(struct idpf_adapter *adapter)
 	}
 	hw->num_lan_regs = num_regions;
 
+out:
+	kfree(rcvd_regions);
 	return err;
 }
 

-- 
2.47.3

Re: [Intel-wired-lan] [PATCH RFT net-next 2/2] idpf: remove __free usage in idpf_virtchnl

[Jeff Johnson via Intel-wired-lan]

On 11/16/2025 7:56 AM, Ally Heev wrote:
> usage of cleanup attributes is discouraged in net [1], achieve cleanup
> using goto. In this patch though, only uninitialized pointers with __free
> attribute are cleaned as they can cause undefined behavior when they
> go out of scope
> 
> Suggested-by: Simon Horman <horms@kernel.org>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/all/aPiG_F5EBQUjZqsl@stanley.mountain/
> Signed-off-by: Ally Heev <allyheev@gmail.com>
> 
> [1] https://docs.kernel.org/process/maintainer-netdev.html#using-device-managed-and-cleanup-h-constructs

<SMH>

If using __free() *correctly* gives more readable code, why wouldn't we do it?

The only problem with this function is that it doesn't follow the guidance
from cleanup.h:

 * the recommendation is to always define and assign variables in one
 * statement and not group variable definitions at the top of the
 * function when __free() is used.


> 
> Signed-off-by: Ally Heev <allyheev@gmail.com>
> ---
>  drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 28 +++++++++++++++++--------
>  1 file changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> index cbb5fa30f5a0ec778c1ee30470da3ca21cc1af24..5b2bf8c3205bc1ea0746f78afa2a24f3f8ad2a8c 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> @@ -1012,7 +1012,7 @@ static int idpf_send_get_caps_msg(struct idpf_adapter *adapter)
>   */
>  static int idpf_send_get_lan_memory_regions(struct idpf_adapter *adapter)
>  {
> -	struct virtchnl2_get_lan_memory_regions *rcvd_regions __free(kfree);
> +	struct virtchnl2_get_lan_memory_regions *rcvd_regions = NULL;
>  	struct idpf_vc_xn_params xn_params = {
>  		.vc_op = VIRTCHNL2_OP_GET_LAN_MEMORY_REGIONS,
>  		.recv_buf.iov_len = IDPF_CTLQ_MAX_BUF_LEN,
> @@ -1029,21 +1029,29 @@ static int idpf_send_get_lan_memory_regions(struct idpf_adapter *adapter)
>  
>  	xn_params.recv_buf.iov_base = rcvd_regions;
>  	reply_sz = idpf_vc_xn_exec(adapter, &xn_params);
> -	if (reply_sz < 0)
> -		return reply_sz;
> +	if (reply_sz < 0) {
> +		err = reply_sz;
> +		goto out;
> +	}
>  
>  	num_regions = le16_to_cpu(rcvd_regions->num_memory_regions);
>  	size = struct_size(rcvd_regions, mem_reg, num_regions);
> -	if (reply_sz < size)
> -		return -EIO;
> +	if (reply_sz < size) {
> +		err = -EIO;
> +		goto out;
> +	}
>  
> -	if (size > IDPF_CTLQ_MAX_BUF_LEN)
> -		return -EINVAL;
> +	if (size > IDPF_CTLQ_MAX_BUF_LEN) {
> +		err = -EINVAL;
> +		goto out;
> +	}
>  
>  	hw = &adapter->hw;
>  	hw->lan_regs = kcalloc(num_regions, sizeof(*hw->lan_regs), GFP_KERNEL);
> -	if (!hw->lan_regs)
> -		return -ENOMEM;
> +	if (!hw->lan_regs) {
> +		err = -ENOMEM;
> +		goto out;
> +	}
>  
>  	for (int i = 0; i < num_regions; i++) {
>  		hw->lan_regs[i].addr_len =
> @@ -1053,6 +1061,8 @@ static int idpf_send_get_lan_memory_regions(struct idpf_adapter *adapter)
>  	}
>  	hw->num_lan_regs = num_regions;
>  
> +out:
> +	kfree(rcvd_regions);
>  	return err;
>  }
>  
> 

Re: [Intel-wired-lan] [PATCH RFT net-next 2/2] idpf: remove __free usage in idpf_virtchnl

[Jeff Johnson via Intel-wired-lan]

On 8/4/2026 3:54 PM, Jeff Johnson wrote:
> On 11/16/2025 7:56 AM, Ally Heev wrote:
>> usage of cleanup attributes is discouraged in net [1], achieve cleanup
>> using goto. In this patch though, only uninitialized pointers with __free
>> attribute are cleaned as they can cause undefined behavior when they
>> go out of scope
>>
>> Suggested-by: Simon Horman <horms@kernel.org>
>> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
>> Closes: https://lore.kernel.org/all/aPiG_F5EBQUjZqsl@stanley.mountain/
>> Signed-off-by: Ally Heev <allyheev@gmail.com>
>>
>> [1] https://docs.kernel.org/process/maintainer-netdev.html#using-device-managed-and-cleanup-h-constructs
> 
> <SMH>
> 
> If using __free() *correctly* gives more readable code, why wouldn't we do it?
> 
> The only problem with this function is that it doesn't follow the guidance
> from cleanup.h:
> 
>  * the recommendation is to always define and assign variables in one
>  * statement and not group variable definitions at the top of the
>  * function when __free() is used.
OMG, not sure why Thunderbird showed this as a recent unread e-mail.
Please ignore this noise!

Re: [Intel-wired-lan] __free usage

[Przemek Kitszel]

On 8/5/26 00:56, Jeff Johnson via Intel-wired-lan wrote:
> On 8/4/2026 3:54 PM, Jeff Johnson wrote:
>> On 11/16/2025 7:56 AM, Ally Heev wrote:
>>> usage of cleanup attributes is discouraged in net [1], achieve cleanup
>>> using goto. In this patch though, only uninitialized pointers with __free
>>> attribute are cleaned as they can cause undefined behavior when they
>>> go out of scope
>>>
>>> Suggested-by: Simon Horman <horms@kernel.org>
>>> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
>>> Closes: https://lore.kernel.org/all/aPiG_F5EBQUjZqsl@stanley.mountain/
>>> Signed-off-by: Ally Heev <allyheev@gmail.com>
>>>
>>> [1] https://docs.kernel.org/process/maintainer-netdev.html#using-device-managed-and-cleanup-h-constructs
>>
>> <SMH>
>>
>> If using __free() *correctly* gives more readable code, why wouldn't we do it?
>>
>> The only problem with this function is that it doesn't follow the guidance
>> from cleanup.h:
>>
>>   * the recommendation is to always define and assign variables in one
>>   * statement and not group variable definitions at the top of the
>>   * function when __free() is used.
> OMG, not sure why Thunderbird showed this as a recent unread e-mail.
> Please ignore this noise!

always happy to see some __free() advocates :)

perhaps with our new meticulous friend Sashiko __free() would not be
misused anymore, especially if we let them know the following quote:

 > So just make the rule be that __free() without an assignment is 
simply > a bug.
 >               Linus

and the resulting "less code" is always better (if only to fit more
data into the AI context window)