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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230320115117.GK36557@unreal>
Date:   Mon, 20 Mar 2023 13:51:17 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
Cc:     intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
        piotr.raczynski@...el.com,
        Mateusz Palczewski <mateusz.palczewski@...el.com>
Subject: Re: [PATCH net v2] ice: clear number of qs when rings are free

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@...el.com>
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@...ux.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@...dia.com>