Re: [Intel-wired-lan] [PATCH net v2] ice: clear number of qs when rings are free

From: Leon Romanovsky <leon@kernel.org>
To: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
Cc: netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org
Subject: Re: [Intel-wired-lan] [PATCH net v2] ice: clear number of qs when rings are free
Date: Mon, 20 Mar 2023 13:51:17 +0200	[thread overview]
Message-ID: <20230320115117.GK36557@unreal> (raw)
In-Reply-To: <20230320112347.117363-1-michal.swiatkowski@linux.intel.com>

On Mon, Mar 20, 2023 at 12:23:47PM +0100, Michal Swiatkowski wrote:
> In case rebuild fails not clearing this field can lead to call trace.
> 
> [  +0.009792] BUG: kernel NULL pointer dereference, address: 0000000000000000
> [  +0.000009] #PF: supervisor read access in kernel mode
> [  +0.000006] #PF: error_code(0x0000) - not-present page
> [  +0.000005] PGD 0 P4D 0
> [  +0.000009] Oops: 0000 [#1] PREEMPT SMP PTI
> [  +0.000009] CPU: 45 PID: 77867 Comm: ice-ptp-0000:60 Kdump: loaded Tainted: G S         OE      6.2.0-rc6+ #110
> [  +0.000010] Hardware name: Dell Inc. PowerEdge R740/0JMK61, BIOS 2.11.2 004/21/2021
> [  +0.000005] RIP: 0010:ice_ptp_update_cached_phctime+0xb0/0x130 [ice]
> [  +0.000145] Code: fa 7e 55 48 8b 93 48 01 00 00 48 8b 0c fa 48 85 c9 74 e1 8b 51 68 85 d2 75 da 66 83 b9 86 04 00 00 00 74 d0 31 d2 48 8b 71 20 <48> 8b 34 d6 48 85 f6 74 07 48 89 86 d8 00 00 00 0f b7 b1 86 04 00
> [  +0.000008] RSP: 0018:ffffa036cf7c7ea8 EFLAGS: 00010246
> [  +0.000008] RAX: 174ab1a8ab400f43 RBX: ffff937cda2c01a0 RCX: ffff937cdca9b028
> [  +0.000005] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> [  +0.000005] RBP: ffffa036cf7c7eb8 R08: 0000000000000000 R09: 0000000000000000
> [  +0.000005] R10: 0000000000000080 R11: 0000000000000001 R12: ffff937cdc971f40
> [  +0.000006] R13: ffff937cdc971f44 R14: 0000000000000001 R15: ffffffffc13f3210
> [  +0.000005] FS:  0000000000000000(0000) GS:ffff93826f980000(0000) knlGS:0000000000000000
> [  +0.000006] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  +0.000006] CR2: 0000000000000000 CR3: 00000004b7310002 CR4: 00000000007726e0
> [  +0.000006] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [  +0.000004] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [  +0.000005] PKRU: 55555554
> [  +0.000004] Call Trace:
> [  +0.000004]  <TASK>
> [  +0.000007]  ice_ptp_periodic_work+0x2a/0x60 [ice]
> [  +0.000126]  kthread_worker_fn+0xa6/0x250
> [  +0.000014]  ? __pfx_kthread_worker_fn+0x10/0x10
> [  +0.000010]  kthread+0xfc/0x130
> [  +0.000009]  ? __pfx_kthread+0x10/0x10
> [  +0.000010]  ret_from_fork+0x29/0x50
> 
> ice_ptp_update_cached_phctime() is calling ice_for_each_rxq macro, in
> case of rebuild fail the rx_ring is NULL and there is NULL pointer
> dereference.
> 
> Also for future safety it is better to clear the size values for tx and
> rx ring when they are cleared.
> 
> Fixes: 6624e780a577 ("ice: split ice_vsi_setup into smaller functions")
> Reported-by: Mateusz Palczewski <mateusz.palczewski@intel.com>
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> ---
> v1 --> v2:
>  * change subject to net and add fixes tag
> ---
>  drivers/net/ethernet/intel/ice/ice_lib.c | 2 ++
>  1 file changed, 2 insertions(+)
> 

It will be so great if all these ice_for_each_*(*, i) macros will go.
They do nothing except hide basic for-loop.

Thanks,
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan