On Thu, 2025-11-06 at 15:07 +0100, Alexander Lobakin wrote:
[..]
> >
> > 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?
It doesn't occur here. But, many maintainers/developers consider it a
bad practice because if the function returns before initialization or
use of `goto` can cause such behaviors.
Here though, the definitions are still at the top right? Maybe I could
just sort them
>
> > +
> > 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
Regards,
Ally
