netdev - Re: [PATCH net 2/4] ice: Don't process extts if PTP is disabled

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]

Date: Thu, 27 Jun 2024 10:42:42 +0000
From: "Kolacinski, Karol" <karol.kolacinski@...el.com>
To: "Nelson, Shannon" <shannon.nelson@....com>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.com>, "davem@...emloft.net" <davem@...emloft.net>,
	"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"edumazet@...gle.com" <edumazet@...gle.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>
CC: "Keller, Jacob E" <jacob.e.keller@...el.com>, "richardcochran@...il.com"
	<richardcochran@...il.com>, "Kitszel, Przemyslaw"
	<przemyslaw.kitszel@...el.com>, Simon Horman <horms@...nel.org>, "Pucha,
 HimasekharX Reddy" <himasekharx.reddy.pucha@...el.com>
Subject: Re: [PATCH net 2/4] ice: Don't process extts if PTP is disabled

 On 6/25/2024 7:57 PM, Nelson Shannon wrote:
> > From: Jacob Keller <jacob.e.keller@...el.com>
> >
> > The ice_ptp_extts_event() function can race with ice_ptp_release() and
> > result in a NULL pointer dereference which leads to a kernel panic.
> >
> > Panic occurs because the ice_ptp_extts_event() function calls
> > ptp_clock_event() with a NULL pointer. The ice driver has already
> > released the PTP clock by the time the interrupt for the next external
> > timestamp event occurs.
> >
> > To fix this, modify the ice_ptp_extts_event() function to check the
> > PTP state and bail early if PTP is not ready.
> >
> > Fixes: 172db5f91d5f ("ice: add support for auxiliary input/output pins")
> > Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>
> > Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
> > Signed-off-by: Karol Kolacinski <karol.kolacinski@...el.com>
> > Reviewed-by: Simon Horman <horms@...nel.org>
> > Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@...el.com> (A Contingent worker at Intel)
> > Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
> > ---
> >   drivers/net/ethernet/intel/ice/ice_ptp.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
> > index d8ff9f26010c..0500ced1adf8 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> > @@ -1559,6 +1559,10 @@ void ice_ptp_extts_event(struct ice_pf *pf)
> >          u8 chan, tmr_idx;
> >          u32 hi, lo;
> >
> > +       /* Don't process timestamp events if PTP is not ready */
> > +       if (pf->ptp.state != ICE_PTP_READY)
> > +               return;
> > +
> 
> If this is a potential race problem, is there some sort of locking that
> assures this stays true while running through your for-loop below here?
> 
> sln

Currently, we have no locking around PTP state.
The code above happens only in the top half of the interrupt and race
can happen when ice_ptp_release() is called and the driver starts to
release PTP structures, but hasn't stopped EXTTS yet.

Thanks,
Karol