netdev - Re: [PATCH net] sfc: fix NULL dereferences in ef100_process_design

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z+zITLJ4wB2Mhk8h@mev-dev.igk.intel.com>
Date: Wed, 2 Apr 2025 07:17:00 +0200
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
To: edward.cree@....com
Cc: linux-net-drivers@....com, davem@...emloft.net, kuba@...nel.org,
	edumazet@...gle.com, pabeni@...hat.com, horms@...nel.org,
	andrew+netdev@...n.ch, Edward Cree <ecree.xilinx@...il.com>,
	netdev@...r.kernel.org, Kyungwook Boo <bookyungwook@...il.com>
Subject: Re: [PATCH net] sfc: fix NULL dereferences in
 ef100_process_design_param()

On Tue, Apr 01, 2025 at 11:54:39PM +0100, edward.cree@....com wrote:
> From: Edward Cree <ecree.xilinx@...il.com>
> 
> Since cited commit, ef100_probe_main() and hence also
>  ef100_check_design_params() run before efx->net_dev is created;
>  consequently, we cannot netif_set_tso_max_size() or _segs() at this
>  point.
> Move those netif calls to ef100_probe_netdev(), and also replace
>  netif_err within the design params code with pci_err.
> 
> Reported-by: Kyungwook Boo <bookyungwook@...il.com>
> Fixes: 98ff4c7c8ac7 ("sfc: Separate netdev probe/remove from PCI probe/remove")
> Signed-off-by: Edward Cree <ecree.xilinx@...il.com>
> ---
>  drivers/net/ethernet/sfc/ef100_netdev.c |  6 ++--
>  drivers/net/ethernet/sfc/ef100_nic.c    | 47 +++++++++++--------------
>  2 files changed, 24 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/ef100_netdev.c b/drivers/net/ethernet/sfc/ef100_netdev.c
> index d941f073f1eb..3a06e3b1bd6b 100644
> --- a/drivers/net/ethernet/sfc/ef100_netdev.c
> +++ b/drivers/net/ethernet/sfc/ef100_netdev.c
> @@ -450,8 +450,9 @@ int ef100_probe_netdev(struct efx_probe_data *probe_data)
>  	net_dev->hw_enc_features |= efx->type->offload_features;
>  	net_dev->vlan_features |= NETIF_F_HW_CSUM | NETIF_F_SG |
>  				  NETIF_F_HIGHDMA | NETIF_F_ALL_TSO;
> -	netif_set_tso_max_segs(net_dev,
> -			       ESE_EF100_DP_GZ_TSO_MAX_HDR_NUM_SEGS_DEFAULT);
> +	nic_data = efx->nic_data;
> +	netif_set_tso_max_size(efx->net_dev, nic_data->tso_max_payload_len);
> +	netif_set_tso_max_segs(efx->net_dev, nic_data->tso_max_payload_num_segs);

Is it fine to drop default value for max segs? Previously if somehow
this value wasn't read from HW it was set to default, now it will be 0.

At the beggining of ef100_probe_main() default values for nic_data are
set. Maybe it is worth to set also this default for max segs?

>  
>  	rc = efx_ef100_init_datapath_caps(efx);
>  	if (rc < 0)
> @@ -477,7 +478,6 @@ int ef100_probe_netdev(struct efx_probe_data *probe_data)
>  	/* Don't fail init if RSS setup doesn't work. */
>  	efx_mcdi_push_default_indir_table(efx, efx->n_rx_channels);
>  
> -	nic_data = efx->nic_data;
>  	rc = ef100_get_mac_address(efx, net_dev->perm_addr, CLIENT_HANDLE_SELF,
>  				   efx->type->is_vf);
>  	if (rc)

[...]