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]

Message-ID: <CO1PR11MB50894BA120B90043BEA857F6D6DA2@CO1PR11MB5089.namprd11.prod.outlook.com>
Date: Mon, 8 Jul 2024 21:24:56 +0000
From: "Keller, Jacob E" <jacob.e.keller@...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: "richardcochran@...il.com" <richardcochran@...il.com>, "Kitszel,
 Przemyslaw" <przemyslaw.kitszel@...el.com>, "Kolacinski, Karol"
	<karol.kolacinski@...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



> -----Original Message-----
> From: Nelson, Shannon <shannon.nelson@....com>
> Sent: Tuesday, June 25, 2024 10:58 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@...el.com>; davem@...emloft.net;
> kuba@...nel.org; pabeni@...hat.com; edumazet@...gle.com;
> netdev@...r.kernel.org
> Cc: Keller, Jacob E <jacob.e.keller@...el.com>; richardcochran@...il.com; Kitszel,
> Przemyslaw <przemyslaw.kitszel@...el.com>; Kolacinski, Karol
> <karol.kolacinski@...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 10:02 AM, Tony Nguyen 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
> 

This function is called by the IRQ when it gets an extts event. During shutdown, we set the state to say its shutdown and then call synchronize_irq on the miscellaneous vector which will wait until the misc IRQ handler completes. From that point the state will always be not ready and the function will have completed execution.

> >          tmr_idx = hw->func_caps.ts_func_info.tmr_index_owned;
> >          /* Event time is captured by one of the two matched registers
> >           *      GLTSYN_EVNT_L: 32 LSB of sampled time event
> > --
> > 2.41.0
> >
> >