All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	magnus.karlsson@intel.com, kuba@kernel.org, pabeni@redhat.com,
	przemyslaw.kitszel@intel.com, jacob.e.keller@intel.com
Subject: Re: [PATCH v5 net 2/7] i40e: avoid null ptr dereference in i40e_ptp_stop()
Date: Wed, 8 Jul 2026 19:31:40 +0100	[thread overview]
Message-ID: <20260708183140.GK1364329@horms.kernel.org> (raw)
In-Reply-To: <ak6HOhJh/pxdloEs@boxer>

On Wed, Jul 08, 2026 at 07:22:02PM +0200, Maciej Fijalkowski wrote:
> On Wed, Jul 08, 2026 at 04:04:56PM +0100, Simon Horman wrote:
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
> > Full review at: https://sashiko.dev
> > ---
> > i40e: avoid null ptr dereference in i40e_ptp_stop()
> >     
> > Check if main_vsi is not null before calling dev_info() to avoid a null
> > pointer dereference when i40e_vsi_clear() frees the main VSI during rebuild.
> > 
> > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > > index ff62b5f2c8150..ca93df4d67859 100644
> > > --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > > +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > > @@ -1556,8 +1556,9 @@ void i40e_ptp_stop(struct i40e_pf *pf)
> > >  	if (pf->ptp_clock) {
> > >  		ptp_clock_unregister(pf->ptp_clock);
> > >  		pf->ptp_clock = NULL;
> > > -		dev_info(&pf->pdev->dev, "%s: removed PHC on %s\n", __func__,
> > > -			 main_vsi->netdev->name);
> > > +		if (main_vsi)
> > > +			dev_info(&pf->pdev->dev, "%s: removed PHC on %s\n", __func__,
> > > +				 main_vsi->netdev->name);
> > 
> > [Severity: High]
> > This isn't a bug introduced by this patch, but this lockless check appears
> > to introduce a time-of-check to time-of-use race condition.
> 
> Thanks Simon I saw these, however since all of the issues are classified
> as pre-existing issues IMHO these changes are okay to be taken and then
> we could go with another round of fixes.

Yes, agreed. Sorry about the noise.

WARNING: multiple messages have this Message-ID (diff)
From: Simon Horman <horms@kernel.org>
To: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
	magnus.karlsson@intel.com, kuba@kernel.org, pabeni@redhat.com,
	przemyslaw.kitszel@intel.com, jacob.e.keller@intel.com
Subject: Re: [Intel-wired-lan] [PATCH v5 net 2/7] i40e: avoid null ptr dereference in i40e_ptp_stop()
Date: Wed, 8 Jul 2026 19:31:40 +0100	[thread overview]
Message-ID: <20260708183140.GK1364329@horms.kernel.org> (raw)
In-Reply-To: <ak6HOhJh/pxdloEs@boxer>

On Wed, Jul 08, 2026 at 07:22:02PM +0200, Maciej Fijalkowski wrote:
> On Wed, Jul 08, 2026 at 04:04:56PM +0100, Simon Horman wrote:
> > This is an AI-generated review of your patch. The human sending this
> > email has considered the AI review valid, or at least plausible.
> > Full review at: https://sashiko.dev
> > ---
> > i40e: avoid null ptr dereference in i40e_ptp_stop()
> >     
> > Check if main_vsi is not null before calling dev_info() to avoid a null
> > pointer dereference when i40e_vsi_clear() frees the main VSI during rebuild.
> > 
> > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > > index ff62b5f2c8150..ca93df4d67859 100644
> > > --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > > +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
> > > @@ -1556,8 +1556,9 @@ void i40e_ptp_stop(struct i40e_pf *pf)
> > >  	if (pf->ptp_clock) {
> > >  		ptp_clock_unregister(pf->ptp_clock);
> > >  		pf->ptp_clock = NULL;
> > > -		dev_info(&pf->pdev->dev, "%s: removed PHC on %s\n", __func__,
> > > -			 main_vsi->netdev->name);
> > > +		if (main_vsi)
> > > +			dev_info(&pf->pdev->dev, "%s: removed PHC on %s\n", __func__,
> > > +				 main_vsi->netdev->name);
> > 
> > [Severity: High]
> > This isn't a bug introduced by this patch, but this lockless check appears
> > to introduce a time-of-check to time-of-use race condition.
> 
> Thanks Simon I saw these, however since all of the issues are classified
> as pre-existing issues IMHO these changes are okay to be taken and then
> we could go with another round of fixes.

Yes, agreed. Sorry about the noise.

  reply	other threads:[~2026-07-08 18:31 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-01 12:45 [Intel-wired-lan] [PATCH v5 net 0/7] i40e: re-init and UAF fixes Maciej Fijalkowski
2026-07-01 12:45 ` Maciej Fijalkowski
2026-07-01 12:45 ` [Intel-wired-lan] [PATCH v5 net 1/7] i40e: unregister netdev before clearing VSI on reinit failure Maciej Fijalkowski
2026-07-01 12:45   ` Maciej Fijalkowski
2026-07-07 13:35   ` [Intel-wired-lan] " Loktionov, Aleksandr
2026-07-07 13:35     ` Loktionov, Aleksandr
2026-07-01 12:45 ` [Intel-wired-lan] [PATCH v5 net 2/7] i40e: avoid null ptr dereference in i40e_ptp_stop() Maciej Fijalkowski
2026-07-01 12:45   ` Maciej Fijalkowski
2026-07-07 13:36   ` [Intel-wired-lan] " Loktionov, Aleksandr
2026-07-07 13:36     ` Loktionov, Aleksandr
2026-07-08 15:04   ` Simon Horman
2026-07-08 15:04     ` Simon Horman
2026-07-08 17:22     ` [Intel-wired-lan] " Maciej Fijalkowski
2026-07-08 17:22       ` Maciej Fijalkowski
2026-07-08 18:31       ` Simon Horman [this message]
2026-07-08 18:31         ` [Intel-wired-lan] " Simon Horman
2026-07-01 12:45 ` [Intel-wired-lan] [PATCH v5 net 3/7] i40e: make ring pointers unreachable before freeing via rcu Maciej Fijalkowski
2026-07-01 12:45   ` Maciej Fijalkowski
2026-07-07 13:37   ` [Intel-wired-lan] " Loktionov, Aleksandr
2026-07-07 13:37     ` Loktionov, Aleksandr
2026-07-01 12:45 ` [Intel-wired-lan] [PATCH v5 net 4/7] i40e: avoid deadlock when calling unregister_netdev() Maciej Fijalkowski
2026-07-01 12:45   ` Maciej Fijalkowski
2026-07-07 13:38   ` [Intel-wired-lan] " Loktionov, Aleksandr
2026-07-07 13:38     ` Loktionov, Aleksandr
2026-07-01 12:45 ` [Intel-wired-lan] [PATCH v5 net 5/7] i40e: fix potential UAF in i40e_vsi_setup()'s error path Maciej Fijalkowski
2026-07-01 12:45   ` Maciej Fijalkowski
2026-07-01 12:45 ` [Intel-wired-lan] [PATCH v5 net 6/7] i40e: do not expose netdev too early Maciej Fijalkowski
2026-07-01 12:45   ` Maciej Fijalkowski
2026-07-01 12:45 ` [Intel-wired-lan] [PATCH v5 net 7/7] i40e: keep q_vectors array in sync with channel count changes Maciej Fijalkowski
2026-07-01 12:45   ` Maciej Fijalkowski
2026-07-08  8:51 ` [PATCH v5 net 0/7] i40e: re-init and UAF fixes Paolo Abeni
2026-07-08  8:51   ` [Intel-wired-lan] " Paolo Abeni
2026-07-08 10:14   ` Maciej Fijalkowski
2026-07-08 10:14     ` [Intel-wired-lan] " Maciej Fijalkowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260708183140.GK1364329@horms.kernel.org \
    --to=horms@kernel.org \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jacob.e.keller@intel.com \
    --cc=kuba@kernel.org \
    --cc=maciej.fijalkowski@intel.com \
    --cc=magnus.karlsson@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=przemyslaw.kitszel@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.