From patchwork Mon Apr 17 06:58:19 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Chan X-Patchwork-Id: 13213361 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7D948C77B72 for ; Mon, 17 Apr 2023 06:58:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230280AbjDQG6l (ORCPT ); Mon, 17 Apr 2023 02:58:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50808 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230327AbjDQG6i (ORCPT ); Mon, 17 Apr 2023 02:58:38 -0400 Received: from mail-pj1-x102b.google.com (mail-pj1-x102b.google.com [IPv6:2607:f8b0:4864:20::102b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 87A331BD2 for ; Sun, 16 Apr 2023 23:58:37 -0700 (PDT) Received: by mail-pj1-x102b.google.com with SMTP id w11so25057280pjh.5 for ; Sun, 16 Apr 2023 23:58:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; t=1681714717; x=1684306717; h=mime-version:references:in-reply-to:message-id:date:subject:cc:to :from:from:to:cc:subject:date:message-id:reply-to; bh=PwI/EnbF7rZ1k9MzyO9Qxm7E/+W61hGgN7ymasifzXw=; b=L5TLkIglOHQq2eZyrjkdS/BOPBTJxn2kbwp/0os0y/A/7e76WxuD+5bXfDeynDmb17 iH8T//FaeLzoj69JewszOYu5olgKNdoQ9yCqrKzPxWGq+C+DtWuVG1vLF2aqFM9hyekg hqn1bLw0DAkq/faWSC1DW5qPo90a3phhGwvdE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681714717; x=1684306717; h=mime-version:references:in-reply-to:message-id:date:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=PwI/EnbF7rZ1k9MzyO9Qxm7E/+W61hGgN7ymasifzXw=; b=EmgQx2EiU+i+ZHj7PhF6GyBPbT1nfnEfTq/XfrUiHi+3EprE85Je2LwU7/GyXduNJL L5L/aA0VmptIkhcSfmIF14UcrlJT5BG4Hen9yiOhZwX+vtgGY3qepiT5pkggyaFsE/zA k/+Aeyp4qtsvaZaNVouNywUY/VFFLpEWD66LwLuGMbL8xoHD8n+J2ZWHYYXL5kf1HmuY lhKd5xTe/+SI7BjCPH6vNbqc3LWEZcHMO4qWAnGTdKvHzbBz7Y4NbqwCTkITlaYBIMCB EBd3inBaNzxDd/HHAorR6AbtJQyhg0uZ8LALkXj0m6j8TVqW5Xm2KmQC+licnzTPI8kP O9Kg== X-Gm-Message-State: AAQBX9cFmbrqgJNGjwmD00QD9sI5eVpJVETwv5/IpJihMVsXwhX+vmaw LnWI5Gmb1wfPzOh6UDYwb4Agmg== X-Google-Smtp-Source: AKy350b+D3Wt52LiE99chOeIwzTRFqxqtAAlEqyCYNHgO1hi2Dgf5ycidICSPvwXC6sSCnmqMbkC3w== X-Received: by 2002:a17:90a:7d06:b0:247:8ce1:996e with SMTP id g6-20020a17090a7d0600b002478ce1996emr2968865pjl.29.1681714716478; Sun, 16 Apr 2023 23:58:36 -0700 (PDT) Received: from lvnvda1597.lvn.broadcom.net ([192.19.161.250]) by smtp.gmail.com with ESMTPSA id h15-20020a17090a050f00b00247abbb157fsm374132pjh.31.2023.04.16.23.58.35 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sun, 16 Apr 2023 23:58:35 -0700 (PDT) From: Michael Chan To: davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, kuba@kernel.org, pabeni@redhat.com, gospo@broadcom.com, Kalesh AP , Ajit Khaparde Subject: [PATCH net 2/2] bnxt_en: Fix a possible NULL pointer dereference in unload path Date: Sun, 16 Apr 2023 23:58:19 -0700 Message-Id: <20230417065819.122055-3-michael.chan@broadcom.com> X-Mailer: git-send-email 2.32.0 In-Reply-To: <20230417065819.122055-1-michael.chan@broadcom.com> References: <20230417065819.122055-1-michael.chan@broadcom.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org From: Kalesh AP In the driver unload path, the driver currently checks the valid BNXT_FLAG_ROCE_CAP flag in bnxt_rdma_aux_device_uninit() before proceeding. This is flawed because the flag may not be set initially during driver load. It may be set later after the NVRAM setting is changed followed by a firmware reset. Relying on the BNXT_FLAG_ROCE_CAP flag may crash in bnxt_rdma_aux_device_uninit() if the aux device was never initialized: BUG: unable to handle kernel NULL pointer dereference at 0000000000000000 PGD 8ae6aa067 P4D 0 Oops: 0000 [#1] SMP NOPTI CPU: 39 PID: 42558 Comm: rmmod Kdump: loaded Tainted: G OE --------- - - 4.18.0-348.el8.x86_64 #1 Hardware name: Dell Inc. PowerEdge R750/0WT8Y6, BIOS 1.5.4 12/17/2021 RIP: 0010:device_del+0x1b/0x410 Code: 89 a5 50 03 00 00 4c 89 a5 58 03 00 00 eb 89 0f 1f 44 00 00 41 56 41 55 41 54 4c 8d a7 80 00 00 00 55 53 48 89 fb 48 83 ec 18 <48> 8b 2f 4c 89 e7 65 48 8b 04 25 28 00 00 00 48 89 44 24 10 31 c0 RSP: 0018:ff7f82bf469a7dc8 EFLAGS: 00010292 RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000 RDX: 0000000000000000 RSI: 0000000000000206 RDI: 0000000000000000 RBP: ff31b7cd114b0ac0 R08: 0000000000000000 R09: ffffffff935c3400 R10: ff31b7cd45bc3440 R11: 0000000000000001 R12: 0000000000000080 R13: ffffffffc1069f40 R14: 0000000000000000 R15: 0000000000000000 FS: 00007fc9903ce740(0000) GS:ff31b7d4ffac0000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 0000000992fee004 CR4: 0000000000773ee0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 PKRU: 55555554 Call Trace: bnxt_rdma_aux_device_uninit+0x1f/0x30 [bnxt_en] bnxt_remove_one+0x2f/0x1f0 [bnxt_en] pci_device_remove+0x3b/0xc0 device_release_driver_internal+0x103/0x1f0 driver_detach+0x54/0x88 bus_remove_driver+0x77/0xc9 pci_unregister_driver+0x2d/0xb0 bnxt_exit+0x16/0x2c [bnxt_en] __x64_sys_delete_module+0x139/0x280 do_syscall_64+0x5b/0x1a0 entry_SYSCALL_64_after_hwframe+0x65/0xca RIP: 0033:0x7fc98f3af71b Fix this by modifying the check inside bnxt_rdma_aux_device_uninit() to check for bp->aux_priv instead. We also need to make some changes in bnxt_rdma_aux_device_init() to make sure that bp->aux_priv is set only when the aux device is fully initialized. Fixes: d80d88b0dfff ("bnxt_en: Add auxiliary driver support") Reviewed-by: Ajit Khaparde Signed-off-by: Kalesh AP Signed-off-by: Michael Chan --- drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c index e7b5e28ee29f..852eb449ccae 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ulp.c @@ -304,7 +304,7 @@ void bnxt_rdma_aux_device_uninit(struct bnxt *bp) struct auxiliary_device *adev; /* Skip if no auxiliary device init was done. */ - if (!(bp->flags & BNXT_FLAG_ROCE_CAP)) + if (!bp->aux_priv) return; aux_priv = bp->aux_priv; @@ -324,6 +324,7 @@ static void bnxt_aux_dev_release(struct device *dev) bp->edev = NULL; kfree(aux_priv->edev); kfree(aux_priv); + bp->aux_priv = NULL; } static void bnxt_set_edev_info(struct bnxt_en_dev *edev, struct bnxt *bp) @@ -359,19 +360,18 @@ void bnxt_rdma_aux_device_init(struct bnxt *bp) if (!(bp->flags & BNXT_FLAG_ROCE_CAP)) return; - bp->aux_priv = kzalloc(sizeof(*bp->aux_priv), GFP_KERNEL); - if (!bp->aux_priv) + aux_priv = kzalloc(sizeof(*bp->aux_priv), GFP_KERNEL); + if (!aux_priv) goto exit; - bp->aux_priv->id = ida_alloc(&bnxt_aux_dev_ids, GFP_KERNEL); - if (bp->aux_priv->id < 0) { + aux_priv->id = ida_alloc(&bnxt_aux_dev_ids, GFP_KERNEL); + if (aux_priv->id < 0) { netdev_warn(bp->dev, "ida alloc failed for ROCE auxiliary device\n"); - kfree(bp->aux_priv); + kfree(aux_priv); goto exit; } - aux_priv = bp->aux_priv; aux_dev = &aux_priv->aux_dev; aux_dev->id = aux_priv->id; aux_dev->name = "rdma"; @@ -380,10 +380,11 @@ void bnxt_rdma_aux_device_init(struct bnxt *bp) rc = auxiliary_device_init(aux_dev); if (rc) { - ida_free(&bnxt_aux_dev_ids, bp->aux_priv->id); - kfree(bp->aux_priv); + ida_free(&bnxt_aux_dev_ids, aux_priv->id); + kfree(aux_priv); goto exit; } + bp->aux_priv = aux_priv; /* From this point, all cleanup will happen via the .release callback & * any error unwinding will need to include a call to