From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BC74DC4360F for ; Tue, 2 Apr 2019 08:33:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9331E2084C for ; Tue, 2 Apr 2019 08:33:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729442AbfDBIdF (ORCPT ); Tue, 2 Apr 2019 04:33:05 -0400 Received: from smtp.nue.novell.com ([195.135.221.5]:44659 "EHLO smtp.nue.novell.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726496AbfDBIdE (ORCPT ); Tue, 2 Apr 2019 04:33:04 -0400 Received: from [10.67.17.168] ([45.122.156.254]) by smtp.nue.novell.com with ESMTP (TLS encrypted); Tue, 02 Apr 2019 10:33:00 +0200 From: Firo Subject: Re: [PATCH 1/1] be2net: Detach interface for avoiding a system crash To: Saeed Mahameed , "sathya.perla@broadcom.com" , "sriharsha.basavapatna@broadcom.com" , "somnath.kotur@broadcom.com" , "ajit.khaparde@broadcom.com" , "davem@davemloft.net" Cc: "firogm@gmail.com" , "netdev@vger.kernel.org" References: <20190401122421.30116-1-fyang@suse.com> <786ef1d29facea3163b91ce160dd146f5759d160.camel@mellanox.com> Message-ID: <5fe2ceb9-da2d-baac-1e05-11ba047a0a19@suse.com> Date: Tue, 2 Apr 2019 16:32:55 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: <786ef1d29facea3163b91ce160dd146f5759d160.camel@mellanox.com> Content-Type: multipart/mixed; boundary="------------033642C4186A3495983DCCD6" Content-Language: en-US Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org This is a multi-part message in MIME format. --------------033642C4186A3495983DCCD6 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 4/2/19 12:25 AM, Saeed Mahameed wrote: > On Mon, 2019-04-01 at 20:24 +0800, Firo Yang wrote: >> This crash is triggered by a user-after-free since lake of >> the synchronization of a race condition between =20 >> be_update_queues() modifying multi-purpose channels of >> network device and be_tx_timeout(). >> >> BUG: unable to handle kernel NULL pointer dereference at (null) >> Call Trace: >> be_tx_timeout+0xa5/0x360 [be2net] >> dev_watchdog+0x1d8/0x210 >> call_timer_fn+0x32/0x140 >> >> To fix it, detach the interface before modifying >> multi-purpose channels of network device. >> >> Signed-off-by: Firo Yang >> --- >> drivers/net/ethernet/emulex/benet/be_main.c | 12 ++++++++---- >> 1 file changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/ethernet/emulex/benet/be_main.c >> b/drivers/net/ethernet/emulex/benet/be_main.c >> index d5026909dec5..25d0128bf684 100644 >> --- a/drivers/net/ethernet/emulex/benet/be_main.c >> +++ b/drivers/net/ethernet/emulex/benet/be_main.c >> @@ -4705,6 +4705,8 @@ int be_update_queues(struct be_adapter >> *adapter) >> struct net_device *netdev =3D adapter->netdev; >> int status; >> =20 >> + netif_device_detach(netdev); >> + > =20 > This will reduce the probability, but will not do the trick. > since this will not guarantee that the dev_watchdog is disabled. Great catch! > =20 > what you need is proper locking mechanism and/or scheduling the > tx_timeout handling out of atomic context if a mutex will be required. Thank you; I will modify this patch. Best, Firo > =20 > netif_device_detach is a too heavy hammer for such synchronizations > tasks. > =20 >> if (netif_running(netdev)) >> be_close(netdev); >> =20 >> @@ -4719,21 +4721,21 @@ int be_update_queues(struct be_adapter >> *adapter) >> be_clear_queues(adapter); >> status =3D be_cmd_if_destroy(adapter, adapter->if_handle, 0); >> if (status) >> - return status; >> + goto out; >> =20 >> if (!msix_enabled(adapter)) { >> status =3D be_msix_enable(adapter); >> if (status) >> - return status; >> + goto out; >> } >> =20 >> status =3D be_if_create(adapter); >> if (status) >> - return status; >> + goto out; >> =20 >> status =3D be_setup_queues(adapter); >> if (status) >> - return status; >> + goto out; >> =20 >> be_schedule_worker(adapter); >> =20 >> @@ -4748,6 +4750,8 @@ int be_update_queues(struct be_adapter >> *adapter) >> if (netif_running(netdev)) >> status =3D be_open(netdev); >> =20 >> +out: >> + netif_device_attach(netdev); >> return status; >> } >> =20 --------------033642C4186A3495983DCCD6 Content-Type: application/pgp-keys; name="pEpkey.asc" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="pEpkey.asc" -----BEGIN PGP PUBLIC KEY BLOCK----- mQENBFwUfvkBCAC/xZ1XM9oIHHmMP1Bc0V+yW/KTvI6QnMt16nPz5TBcEbD3LD/o bJfSxEcnEJaQB32Vq1kra2y0fDJlPQwz/xc+83AtHbzmjq63w3RUhxEHZdKS2l4v Iq+4/8GstrBs4CwPBojLoHwSB71F9c9uXLPDNwt0RYaw9r3JErKgtdz+P8b1MfTH xpvVdOkVL/j/00bdM9zetPdH9OBiMze5tjhTwNM4j4lK+u+2SaSDTPKuDa79rRqa 8eLBK4UrmNR9mHpz5j5HhJ5sNYmRzncHRCrfb/dYAwUQ9MB24C3PkxffeQ7DQJkq uqVaTZjHYrgrSTwHTtSeIffAe7fIexDY5jclABEBAAG0FUZpcm8gPGZ5YW5nQHN1 c2UuY29tPokBVAQTAQgAPhYhBNTMhEhJlcBZQQTDn25dQMjSg/nfBQJcFH75AhsD BQkB4TOABQsJCAcCBhUKCQgLAgQWAgMBAh4BAheAAAoJEG5dQMjSg/nf16gH/28h QA0n7jGiyHDNZlErWo1TuGads6LU7WKIgmhMG/l8Q3hHoCqweICAOiiyXeMcFn6a kPLk7chAwr7Zv+rkdqpvZpkVHcA+iPaJhNDwyO/P+JV+m2NbB5ZQRPLscyV1WxEg 4/64i0u9wGSXjpqJXrCngcFRksalK9x+0dqxg5TiB+ABbP5CoipLcK7ujzRc1Mkk xpuqiyley5NAiT0WM/ryqreUJpOTVptxlmK26S6PUBXn2uwJjBI8fc1ZrcPFJE1h QFxClfy6gSHhQtPyVaRfGa80UqY8hb1aobVjPfOgkDYqWXER9AGQRKEi+PxxA7sF Dc1r7Pv4gM7w3pZMOWu5AQ0EXBR++QEIAKgQNPrOCmY0Qq0J+NcZGIOR+lp4TMI4 EEeeV69WfqOe6fkPu4CmwBLOgCd1ISgOALRKDXNhH/ghbUHLRW7pj3K5ROiGK/7Q pKnTx80OB2SiWsCK80x5f8qEbAnpt0i4RscpQEC9vb6Z9b101VHlZayaeCY8bltR ixp0mgZqwwcTYV9KaLgegAMgm78xIfYX/nYz2kqIOAFDGLxEURUqyoiGt94xmX4G 3FGr+oWY5DBMDJMLSuq9EB2iihrOhbBOoytnXYXDwiAawdV3roW01J34baZ6VP3e 2jKJc/p6ldYE+4hWPVHD/JLIWg3sdLbeeo6NBNLIGWdH5WpYskx8oYcAEQEAAYkB PAQYAQgAJhYhBNTMhEhJlcBZQQTDn25dQMjSg/nfBQJcFH75AhsMBQkB4TOAAAoJ EG5dQMjSg/nfrzwIAIrxkv90BX9+rJPkM0NTsucbQs/ybKJglNW+s/IaRb7l1nip hgDAkAP6eCYrxCo0BMu0DdeDE49t2Z212fE6Fin2Nv1kGd2tsmQfeWM8kPD+QJ0D 2LTj1GNj3ZEVo5acFSwzwLZ7ttZzAkoC30QG6m3H5pdEdvWZS/0hGT2oD+C9008e Ma87f6+SsRMuzx/lzJAhwVe1qq96+0nfq2/gdSeWZTreDBXqYO3dNrFfm3XcVROn 6RSsa2rhlw0fyHGN1sTP/TEBidH2tHiFsGcGt1QEWTRtcd1IHbzZEpv3Gmz0qMk2 hdyzI5Y5MGUYqir9zQA0qj1sptqgld01kBZUTRo=3D =3DMjHg -----END PGP PUBLIC KEY BLOCK----- --------------033642C4186A3495983DCCD6--