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 <[email protected]>
---
drivers/net/ethernet/intel/ice/ice_gnss.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
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);
}
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 <[email protected]>
> ---
LGTM.
Reviewed-by: Tariq Toukan <[email protected]>
> -----Original Message-----
> From: Intel-wired-lan <[email protected]> On Behalf Of Tariq Toukan
> Sent: Friday, May 26, 2023 2:48 AM
> To: Simon Horman <[email protected]>; Brandeburg, Jesse <[email protected]>; Nguyen, Anthony L <[email protected]>
> Cc: Mishra, Sudhansu Sekhar <[email protected]>; Kolacinski, Karol <[email protected]>; [email protected]; Eric Dumazet <[email protected]>; [email protected]; [email protected]; Jakub Kicinski <[email protected]>; Paolo Abeni <[email protected]>; David S. Miller <[email protected]>; Dan Carpenter <[email protected]>
> 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 <[email protected]>
>> ---
Tested-by: Sunitha Mekala <[email protected]> (A Contingent worker at Intel)