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

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 for Android: free password hash cracker in your pocket

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

Message-ID: <00748f83a8ae688b7063f36844e38073d29b5e19.camel@gmail.com>
Date: Thu, 06 Nov 2025 21:35:21 +0530
From: ally heev <allyheev@...il.com>
To: Alexander Lobakin <aleksander.lobakin@...el.com>
Cc: Tony Nguyen <anthony.l.nguyen@...el.com>, Przemek Kitszel	
 <przemyslaw.kitszel@...el.com>, Andrew Lunn <andrew+netdev@...n.ch>, "David
 S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub
 Kicinski <kuba@...nel.org>,  Paolo Abeni <pabeni@...hat.com>, "K. Y.
 Srinivasan" <kys@...rosoft.com>, Haiyang Zhang	 <haiyangz@...rosoft.com>,
 Wei Liu <wei.liu@...nel.org>, Dexuan Cui	 <decui@...rosoft.com>, Aleksandr
 Loktionov <aleksandr.loktionov@...el.com>, 
	intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-hyperv@...r.kernel.org, Dan Carpenter	
 <dan.carpenter@...aro.org>
Subject: Re: [Intel-wired-lan] [PATCH v3] net: ethernet: fix uninitialized
 pointers with free attribute

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