From: Ally Heev <[email protected]> Date: Thu, 06 Nov 2025 17:25:48 +0530
> 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. > > It is better to initialize and assign pointers with `__free` > attribute in one statement to ensure proper scope-based cleanup. > > Reported-by: Dan Carpenter <[email protected]> > Closes: https://lore.kernel.org/all/[email protected]/ > Signed-off-by: Ally Heev <[email protected]> > --- > Changes in v3: > - fixed style issues > - Link to v2: > https://lore.kernel.org/r/20251106-aheev-uninitialized-free-attr-net-ethernet-v2-1-048da0c5d...@gmail.com > > Changes in v2: > - fixed non-pointer initialization to NULL > - NOTE: drop v1 > - Link to v1: > https://lore.kernel.org/r/20251105-aheev-uninitialized-free-attr-net-ethernet-v1-1-f6ea84bbd...@gmail.com > --- > drivers/net/ethernet/intel/ice/ice_flow.c | 5 +++-- > drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 5 +++-- > 2 files changed, 6 insertions(+), 4 deletions(-) > > 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. But most important, I'm not even sure how you could trigger an "undefined behaviour" here. Both here and below the variable tagged with `__free` is initialized right after the declaration block, before any return. So how to trigger an UB here? > + > if (!params) > return -ENOMEM; > > diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c > b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c > index > cbb5fa30f5a0ec778c1ee30470da3ca21cc1af24..368138715cd55cd1dadc686931cdda51c7a5130d > 100644 > --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c > +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c > @@ -1012,7 +1012,6 @@ 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 idpf_vc_xn_params xn_params = { > .vc_op = VIRTCHNL2_OP_GET_LAN_MEMORY_REGIONS, > .recv_buf.iov_len = IDPF_CTLQ_MAX_BUF_LEN, > @@ -1023,7 +1022,9 @@ static int idpf_send_get_lan_memory_regions(struct > idpf_adapter *adapter) > ssize_t reply_sz; > int err = 0; > > - rcvd_regions = kzalloc(IDPF_CTLQ_MAX_BUF_LEN, GFP_KERNEL); > + struct virtchnl2_get_lan_memory_regions *rcvd_regions __free(kfree) = > + kzalloc(IDPF_CTLQ_MAX_BUF_LEN, GFP_KERNEL); > + > if (!rcvd_regions) > return -ENOMEM; Same here, @rcvd_regions is initialized before the very first return, no idea how one can provoke an UB here. > > > --- > base-commit: c9cfc122f03711a5124b4aafab3211cf4d35a2ac > change-id: 20251105-aheev-uninitialized-free-attr-net-ethernet-7d106e4ab3f7 > > Best regards, Thanks, Olek
