diff mbox series

[net] ice: Don't dereference NULL in ice_gns_read error path

Message ID 20230525-null-ice-v1-1-30d10557b91e@kernel.org (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series [net] ice: Don't dereference NULL in ice_gns_read error path | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 20 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Simon Horman May 25, 2023, 10:52 a.m. UTC
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(-)

Comments

Tariq Toukan May 26, 2023, 9:48 a.m. UTC | #1
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>
Mekala, SunithaX D June 6, 2023, 5:20 p.m. UTC | #2
> -----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 mbox series

Patch

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);
 }