Messages in this thread | | | From | Subbaraya Sundeep Bhatta <> | Subject | RE: [EXT] Re: [net PATCH 5/9] octeontx2-pf: mcs: Fix NULL pointer dereferences | Date | Mon, 24 Apr 2023 10:29:02 +0000 |
| |
Hi,
>-----Original Message----- >From: Leon Romanovsky <leon@kernel.org> >Sent: Sunday, April 23, 2023 10:22 PM >To: Geethasowjanya Akula <gakula@marvell.com> >Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; kuba@kernel.org; >davem@davemloft.net; edumazet@google.com; pabeni@redhat.com; >richardcochran@gmail.com; Sunil Kovvuri Goutham ><sgoutham@marvell.com>; Subbaraya Sundeep Bhatta ><sbhatta@marvell.com>; Hariprasad Kelam <hkelam@marvell.com> >Subject: [EXT] Re: [net PATCH 5/9] octeontx2-pf: mcs: Fix NULL pointer >dereferences > >On Sun, Apr 23, 2023 at 03:24:50PM +0530, Geetha sowjanya wrote: >> From: Subbaraya Sundeep <sbhatta@marvell.com> >> >> When system is rebooted after creating macsec interface below NULL >> pointer dereference crashes occurred. This patch fixes those crashes. >> >> [ 3324.406942] Unable to handle kernel NULL pointer dereference at >> virtual address 0000000000000000 [ 3324.415726] Mem abort info: >> [ 3324.418510] ESR = 0x96000006 >> [ 3324.421557] EC = 0x25: DABT (current EL), IL = 32 bits >> [ 3324.426865] SET = 0, FnV = 0 >> [ 3324.429913] EA = 0, S1PTW = 0 >> [ 3324.433047] Data abort info: >> [ 3324.435921] ISV = 0, ISS = 0x00000006 >> [ 3324.439748] CM = 0, WnR = 0 >> .... >> [ 3324.575915] Call trace: >> [ 3324.578353] cn10k_mdo_del_secy+0x24/0x180 [ 3324.582440] >> macsec_common_dellink+0xec/0x120 [ 3324.586788] >> macsec_notify+0x17c/0x1c0 [ 3324.590529] >> raw_notifier_call_chain+0x50/0x70 [ 3324.594965] >> call_netdevice_notifiers_info+0x34/0x7c >> [ 3324.599921] rollback_registered_many+0x354/0x5bc >> [ 3324.604616] unregister_netdevice_queue+0x88/0x10c >> [ 3324.609399] unregister_netdev+0x20/0x30 [ 3324.613313] >> otx2_remove+0x8c/0x310 [ 3324.616794] pci_device_shutdown+0x30/0x70 >[ >> 3324.620882] device_shutdown+0x11c/0x204 >> >> [ 966.664930] Unable to handle kernel NULL pointer dereference at >> virtual address 0000000000000000 [ 966.673712] Mem abort info: >> [ 966.676497] ESR = 0x96000006 >> [ 966.679543] EC = 0x25: DABT (current EL), IL = 32 bits >> [ 966.684848] SET = 0, FnV = 0 >> [ 966.687895] EA = 0, S1PTW = 0 >> [ 966.691028] Data abort info: >> [ 966.693900] ISV = 0, ISS = 0x00000006 >> [ 966.697729] CM = 0, WnR = 0 >> .... >> [ 966.833467] Call trace: >> [ 966.835904] cn10k_mdo_stop+0x20/0xa0 [ 966.839557] >> macsec_dev_stop+0xe8/0x11c [ 966.843384] >__dev_close_many+0xbc/0x140 >> [ 966.847298] dev_close_many+0x84/0x120 [ 966.851039] >> rollback_registered_many+0x114/0x5bc >> [ 966.855735] unregister_netdevice_many.part.0+0x14/0xa0 >> [ 966.860952] unregister_netdevice_many+0x18/0x24 >> [ 966.865560] macsec_notify+0x1ac/0x1c0 [ 966.869303] >> raw_notifier_call_chain+0x50/0x70 [ 966.873738] >> call_netdevice_notifiers_info+0x34/0x7c >> [ 966.878694] rollback_registered_many+0x354/0x5bc >> [ 966.883390] unregister_netdevice_queue+0x88/0x10c >> [ 966.888173] unregister_netdev+0x20/0x30 [ 966.892090] >> otx2_remove+0x8c/0x310 [ 966.895571] pci_device_shutdown+0x30/0x70 [ >> 966.899660] device_shutdown+0x11c/0x204 [ 966.903574] >> __do_sys_reboot+0x208/0x290 [ 966.907487] >> __arm64_sys_reboot+0x20/0x30 [ 966.911489] >> el0_svc_handler+0x80/0x1c0 [ 966.915316] el0_svc+0x8/0x180 [ >> 966.918362] Code: f9400000 f9400a64 91220014 f94b3403 (f9400060) [ >> 966.924448] ---[ end trace 341778e799c3d8d7 ]--- >> >> Fixes: c54ffc73601c ("octeontx2-pf: mcs: Introduce MACSEC hardware >> offloading") >> Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com> >> Signed-off-by: Sunil Goutham <sgoutham@marvell.com> >> Signed-off-by: Geetha sowjanya <gakula@marvell.com> >> --- >> drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c >> b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c >> index 9ec5f38d38a8..5f4402f7b03e 100644 >> --- a/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c >> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/cn10k_macsec.c >> @@ -1065,6 +1065,9 @@ static int cn10k_mdo_stop(struct macsec_context >*ctx) >> struct cn10k_mcs_txsc *txsc; >> int err; >> >> + if (!cfg) >> + return 0; >> + >> txsc = cn10k_mcs_get_txsc(cfg, ctx->secy); >> if (!txsc) >> return -ENOENT; >> @@ -1146,6 +1149,9 @@ static int cn10k_mdo_del_secy(struct >macsec_context *ctx) >> struct cn10k_mcs_cfg *cfg = pfvf->macsec_cfg; >> struct cn10k_mcs_txsc *txsc; >> >> + if (!cfg) >> + return 0; > >How did you get call to .mdo_del_secy if you didn't add any secy? > >Thanks > It is because of the order of teardown in otx2_remove: cn10k_mcs_free(pf); unregister_netdev(netdev);
cn10k_mcs_free free the resources and makes cfg as NULL. Later unregister_netdev calls mdo_del_secy and finds cfg as NULL. Thanks for the review I will change the order and submit next version.
Sundeep
>> + >> txsc = cn10k_mcs_get_txsc(cfg, ctx->secy); >> if (!txsc) >> return -ENOENT; >> -- >> 2.25.1 >>
| |