netdev - Re: [PATCH net 4/6] pds_core: Remove unnecessary check in pds_client_adminq

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: <20250409170745.GO395307@horms.kernel.org>
Date: Wed, 9 Apr 2025 18:07:45 +0100
From: Simon Horman <horms@...nel.org>
To: Shannon Nelson <shannon.nelson@....com>
Cc: andrew+netdev@...n.ch, brett.creeley@....com, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	michal.swiatkowski@...ux.intel.com, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org
Subject: Re: [PATCH net 4/6] pds_core: Remove unnecessary check in
 pds_client_adminq_cmd()

On Mon, Apr 07, 2025 at 03:51:11PM -0700, Shannon Nelson wrote:
> From: Brett Creeley <brett.creeley@....com>
> 
> When the pds_core driver was first created there were some race
> conditions around using the adminq, especially for client drivers.
> To reduce the possibility of a race condition there's a check
> against pf->state in pds_client_adminq_cmd(). This is problematic
> for a couple of reasons:
> 
> 1. The PDSC_S_INITING_DRIVER bit is set during probe, but not
>    cleared until after everything in probe is complete, which
>    includes creating the auxiliary devices. For pds_fwctl this
>    means it can't make any adminq commands until after pds_core's
>    probe is complete even though the adminq is fully up by the
>    time pds_fwctl's auxiliary device is created.
> 
> 2. The race conditions around using the adminq have been fixed
>    and this path is already protected against client drivers
>    calling pds_client_adminq_cmd() if the adminq isn't ready,
>    i.e. see pdsc_adminq_post() -> pdsc_adminq_inc_if_up().
> 
> Fix this by removing the pf->state check in pds_client_adminq_cmd()
> because invalid accesses to pds_core's adminq is already handled by
> pdsc_adminq_post()->pdsc_adminq_inc_if_up().
> 
> Fixes: 10659034c622 ("pds_core: add the aux client API")

I'm assuming that backporting this patch that far only
makes sense if other fixes have been backported too.
And that their fixes tags should enable that happening.

If so, this seems fine to me.

> Signed-off-by: Brett Creeley <brett.creeley@....com>
> Signed-off-by: Shannon Nelson <shannon.nelson@....com>

Reviewed-by: Simon Horman <horms@...nel.org>