netdev - Re: [PATCH iwl-net 1/3] idpf: Fix RSS LUT NULL pointer crash on early ethtool operations

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <32e83b3a-a803-4464-b57b-a39fa45a2220@intel.com>
Date: Fri, 21 Nov 2025 13:22:21 -0800
From: Tony Nguyen <anthony.l.nguyen@...el.com>
To: Sreedevi Joshi <sreedevi.joshi@...el.com>,
	<intel-wired-lan@...ts.osuosl.org>
CC: <netdev@...r.kernel.org>, Sridhar Samudrala <sridhar.samudrala@...el.com>,
	Emil Tantilov <emil.s.tantilov@...el.com>
Subject: Re: [PATCH iwl-net 1/3] idpf: Fix RSS LUT NULL pointer crash on early
 ethtool operations



On 11/17/2025 8:22 PM, Sreedevi Joshi wrote:
> The RSS LUT is not initialized until the interface comes up, causing
> the following NULL pointer crash when ethtool operations like rxhash on/off
> are performed before the interface is brought up for the first time.
> 
> Move RSS LUT initialization from ndo_open to vport creation to ensure LUT
> is always available. This enables RSS configuration via ethtool before
> bringing the interface up. Simplify LUT management by maintaining all
> changes in the driver's soft copy and programming zeros to the indirection
> table when rxhash is disabled. Defer HW programming until the interface
> comes up if it is down during rxhash and LUT configuration changes.
> 
> [89408.371875] BUG: kernel NULL pointer dereference, address: 0000000000000000
> [89408.371908] #PF: supervisor read access in kernel mode
> [89408.371924] #PF: error_code(0x0000) - not-present page
> [89408.371940] PGD 0 P4D 0
> [89408.371953] Oops: Oops: 0000 [#1] SMP NOPTI
> <snip>
> [89408.372052] RIP: 0010:memcpy_orig+0x16/0x130
> [89408.372310] Call Trace:
> [89408.372317]  <TASK>
> [89408.372326]  ? idpf_set_features+0xfc/0x180 [idpf]
> [89408.372363]  __netdev_update_features+0x295/0xde0
> [89408.372384]  ethnl_set_features+0x15e/0x460
> [89408.372406]  genl_family_rcv_msg_doit+0x11f/0x180
> [89408.372429]  genl_rcv_msg+0x1ad/0x2b0
> [89408.372446]  ? __pfx_ethnl_set_features+0x10/0x10
> [89408.372465]  ? __pfx_genl_rcv_msg+0x10/0x10
> [89408.372482]  netlink_rcv_skb+0x58/0x100
> [89408.372502]  genl_rcv+0x2c/0x50
> [89408.372516]  netlink_unicast+0x289/0x3e0
> [89408.372533]  netlink_sendmsg+0x215/0x440
> [89408.372551]  __sys_sendto+0x234/0x240
> [89408.372571]  __x64_sys_sendto+0x28/0x30
> [89408.372585]  x64_sys_call+0x1909/0x1da0
> [89408.372604]  do_syscall_64+0x7a/0xfa0
> [89408.373140]  ? clear_bhb_loop+0x60/0xb0
> [89408.373647]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [89408.378887]  </TASK>
> <snip>
> 
> Fixes: a251eee62133 ("idpf: add SRIOV support and other ndo_ops")
> Signed-off-by: Sreedevi Joshi <sreedevi.joshi@...el.com>
> Reviewed-by: Sridhar Samudrala <sridhar.samudrala@...el.com>
> Reviewed-by: Emil Tantilov <emil.s.tantilov@...el.com>
> ---
>   drivers/net/ethernet/intel/idpf/idpf.h        |  2 -
>   drivers/net/ethernet/intel/idpf/idpf_lib.c    | 89 +++++++++----------
>   drivers/net/ethernet/intel/idpf/idpf_txrx.c   | 36 +++-----
>   drivers/net/ethernet/intel/idpf/idpf_txrx.h   |  4 +-
>   .../net/ethernet/intel/idpf/idpf_virtchnl.c   |  9 +-
>   5 files changed, 64 insertions(+), 76 deletions(-)
> 

...


> @@ -1289,6 +1291,13 @@ static struct idpf_vport *idpf_vport_alloc(struct idpf_adapter *adapter,
>   	/* Initialize default rss key */
>   	netdev_rss_key_fill((void *)rss_data->rss_key, rss_data->rss_key_size);
>   
> +	/* Initialize default rss LUT */
> +	err = idpf_init_rss_lut(vport);
> +	if (err) {
> +		kfree(rss_data->rss_key);

Can you move this free into the goto path? In case anything new gets 
added in the future, it'll already be there.

> +		goto free_vport;

The previous error/goto goes to free_vector_idxs. Would this goto leak 
q_vector_idxs?

> +	}
> +
>   	/* fill vport slot in the adapter struct */
>   	adapter->vports[idx] = vport;
>   	adapter->vport_ids[idx] = idpf_get_vport_id(vport);

...

> @@ -1593,7 +1612,7 @@ static int idpf_vport_open(struct idpf_vport *vport, bool rtnl)
>   	return 0;
>   
>   deinit_rss:
> -	idpf_deinit_rss(vport);
> +	idpf_deinit_rss_lut(vport);

Since this patch moved init out of open, should this be moved out too?

>   disable_vport:
>   	idpf_send_disable_vport_msg(vport);
>   disable_queues:
>

...

> @@ -2839,7 +2845,8 @@ int idpf_send_get_set_rss_lut_msg(struct idpf_vport *vport, bool get)
>   	} else {
>   		rl->lut_entries = cpu_to_le16(rss_data->rss_lut_size);
>   		for (i = 0; i < rss_data->rss_lut_size; i++)
> -			rl->lut[i] = cpu_to_le32(rss_data->rss_lut[i]);
> +			rl->lut[i] = (rxhash_ena) ?

The parens don't look needed.

Thanks,
Tony

> +				cpu_to_le32(rss_data->rss_lut[i]) : 0;
>   
>   		xn_params.vc_op = VIRTCHNL2_OP_SET_RSS_LUT;
>   	}