netdev - RE: [Intel-wired-lan] [PATCH iwl-net v1 3/4] idpf: Add init, reinit, and deinit control lock

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: <MW4PR11MB59113501B8B665F165CBF0D3BA182@MW4PR11MB5911.namprd11.prod.outlook.com>
Date: Tue, 14 Jan 2025 06:50:56 +0000
From: "Singh, Krishneil K" <krishneil.k.singh@...el.com>
To: Tarun K Singh <tarun.k.singh@...el.com>,
	"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [Intel-wired-lan] [PATCH iwl-net v1 3/4] idpf: Add init, reinit,
 and deinit control lock

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On Behalf Of
> Tarun K Singh
> Sent: Tuesday, November 5, 2024 10:49 AM
> To: intel-wired-lan@...ts.osuosl.org
> Cc: netdev@...r.kernel.org
> Subject: [Intel-wired-lan] [PATCH iwl-net v1 3/4] idpf: Add init, reinit, and
> deinit control lock
> 
> Add new 'vport_init_lock' to prevent locking issue.
> 
> The existing 'vport_cfg_lock' was controlling the vport initialization,
> re-initialization due to reset, and de-initialization of code flow.
> In addition to controlling the above behavior it was also controlling
> the parallel netdevice calls such as open/close from Linux OS, which
> can happen independent of re-init or de-init of the vport(s). If first
> one such as re-init or de-init is going on then the second operation
> to config the netdevice with OS should not take place. The first
> operation (init or de-init) takes the precedence if both are to happen
> simultaneously.
> 
> Use of single lock cause deadlock and inconsistent behavior of code
> flow. E.g. when driver undergoes reset via 'idpf_init_hard_reset', it
> acquires the 'vport_cfg_lock', and during this process it tries to
> unregister netdevice which may call 'idpf_stop' which tries to acquire
> same lock causing it to deadlock.
> 
> To address above, add new lock 'vport_init_lock' which control the
> initialization, re-initialization, and de-initialization flow.
> The 'vport_cfg_lock' now exclusively controls the vport config
> operations.
> 
> Add vport config lock around 'idpf_vport_stop()' and 'idpf_vport_open()'
> to protect close and open operation at the same time.
> 
> Add vport init lock around 'idpf_sriv_configure()' to protect it from
> load and removal path. To accomplish it, use existing function
> as wrapper and introduce another function 'idpf_sriov_config_vfs'
> which is used inside the lock.
> 
> In idpf_remove, change 'idpf_sriov_configure' to
> 'idpf_sriov_config_vfs', and move inside the init lock.
> 
> Use these two locks in the following precedence:
> 'vport_init_lock' first, then 'vport_cfg_lock'.
> 
> Fixes: 8077c727561a ("idpf: add controlq init and reset checks")
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
> Reviewed-by: Madhu Chittim <madhu.chittim@...el.com>
> Signed-off-by: Tarun K Singh <tarun.k.singh@...el.com>
> ---
>  drivers/net/ethernet/intel/idpf/idpf.h      | 25 +++++++++++++
>  drivers/net/ethernet/intel/idpf/idpf_lib.c  | 41 ++++++++++++++++++---
>  drivers/net/ethernet/intel/idpf/idpf_main.c |  7 +++-
>  3 files changed, 67 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf.h
> b/drivers/net/ethernet/intel/idpf/idpf.h
> index 8dea2dd784af..34dbdc7d6c88 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf.h


Tested-by: Krishneil Singh <krishneil.k.singh@...el.com>