| Message ID | 20230525-null-ice-v1-1-30d10557b91e@kernel.org |
|---|---|
| State | Accepted |
| Delegated to: | Anthony Nguyen |
| Headers | show |
| Series | [net] ice: Don't dereference NULL in ice_gns_read error path | expand |
On 25/05/2023 13:52, Simon Horman wrote: > If pf is NULL in ice_gns_read() then it will be dereferenced > in the error path by a call to dev_dbg(ice_pf_to_dev(pf), ...). > > Avoid this by simply returning in this case. > If logging is desired an alternate approach might be to > use pr_err() before returning. > > Flagged by Smatch as: > > .../ice_gnss.c:196 ice_gnss_read() error: we previously assumed 'pf' could be null (see line 131) > > Fixes: 43113ff73453 ("ice: add TTY for GNSS module for E810T device") > Signed-off-by: Simon Horman <horms@kernel.org> > --- LGTM. Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Tariq Toukan > Sent: Friday, May 26, 2023 2:48 AM > To: Simon Horman <horms@kernel.org>; Brandeburg, Jesse <jesse.brandeburg@intel.com>; Nguyen, Anthony L <anthony.l.nguyen@intel.com> > Cc: Mishra, Sudhansu Sekhar <sudhansu.mishra@intel.com>; Kolacinski, Karol <karol.kolacinski@intel.com>; linux-kernel@vger.kernel.org; Eric Dumazet <edumazet@google.com>; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; David S. Miller <davem@davemloft.net>; Dan Carpenter <dan.carpenter@linaro.org> > Subject: Re: [Intel-wired-lan] [PATCH net] ice: Don't dereference NULL in ice_gns_read error path > > > > On 25/05/2023 13:52, Simon Horman wrote: >> If pf is NULL in ice_gns_read() then it will be dereferenced in the >> error path by a call to dev_dbg(ice_pf_to_dev(pf), ...). >> >> Avoid this by simply returning in this case. >> If logging is desired an alternate approach might be to use pr_err() >> before returning. >> >> Flagged by Smatch as: >> >> .../ice_gnss.c:196 ice_gnss_read() error: we previously assumed >> 'pf' could be null (see line 131) > > >> Fixes: 43113ff73453 ("ice: add TTY for GNSS module for E810T device") >> Signed-off-by: Simon Horman <horms@kernel.org> >> --- Tested-by: Sunitha Mekala <sunithax.d.mekala@intel.com> (A Contingent worker at Intel)
diff --git a/drivers/net/ethernet/intel/ice/ice_gnss.c b/drivers/net/ethernet/intel/ice/ice_gnss.c index 2ea8a2b11bcd..3d0663840aa1 100644 --- a/drivers/net/ethernet/intel/ice/ice_gnss.c +++ b/drivers/net/ethernet/intel/ice/ice_gnss.c @@ -128,12 +128,7 @@ static void ice_gnss_read(struct kthread_work *work) int err = 0; pf = gnss->back; - if (!pf) { - err = -EFAULT; - goto exit; - } - - if (!test_bit(ICE_FLAG_GNSS, pf->flags)) + if (!pf || !test_bit(ICE_FLAG_GNSS, pf->flags)) return; hw = &pf->hw; @@ -191,7 +186,6 @@ static void ice_gnss_read(struct kthread_work *work) free_page((unsigned long)buf); requeue: kthread_queue_delayed_work(gnss->kworker, &gnss->read_work, delay); -exit: if (err) dev_dbg(ice_pf_to_dev(pf), "GNSS failed to read err=%d\n", err); }
If pf is NULL in ice_gns_read() then it will be dereferenced in the error path by a call to dev_dbg(ice_pf_to_dev(pf), ...). Avoid this by simply returning in this case. If logging is desired an alternate approach might be to use pr_err() before returning. Flagged by Smatch as: .../ice_gnss.c:196 ice_gnss_read() error: we previously assumed 'pf' could be null (see line 131) Fixes: 43113ff73453 ("ice: add TTY for GNSS module for E810T device") Signed-off-by: Simon Horman <horms@kernel.org> --- drivers/net/ethernet/intel/ice/ice_gnss.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)