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

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

Message-ID: <CO1PR11MB5089D31ACBC078C12A6D18E3D6DA2@CO1PR11MB5089.namprd11.prod.outlook.com>
Date: Mon, 8 Jul 2024 21:25:35 +0000
From: "Keller, Jacob E" <jacob.e.keller@...el.com>
To: "Kolacinski, Karol" <karol.kolacinski@...el.com>, "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>, 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: Kolacinski, Karol <karol.kolacinski@...el.com>
> Sent: Thursday, June 27, 2024 3:43 AM
> To: Nelson, Shannon <shannon.nelson@....com>; 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>; 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

My understanding is this is serialized via synchronize_irq on the miscellaneous IRQ vector.

Thanks,
Jake