Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 25 Nov 2022 14:46:05 +0100
From:   Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To:     Ziyang Xuan <william.xuanziyang@huawei.com>
CC:     <sgoutham@marvell.com>, <gakula@marvell.com>,
        <sbhatta@marvell.com>, <hkelam@marvell.com>, <davem@davemloft.net>,
        <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>,
        <netdev@vger.kernel.org>, <naveenm@marvell.com>,
        <rsaladi2@marvell.com>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net 1/2] octeontx2-pf: Fix possible memory leak in
 otx2_probe()
Message-ID: <Y4DHHUUbGl5wWGQ+@boxer>
References: <cover.1669253985.git.william.xuanziyang@huawei.com>
 <e024450cf08f469fb1e0153b78a04a54829dfddb.1669253985.git.william.xuanziyang@huawei.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <e024450cf08f469fb1e0153b78a04a54829dfddb.1669253985.git.william.xuanziyang@huawei.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?2022B6jzl5hXjbWRDb8a8EGDVkrkIb7rveDrRrJttTc9bqkXn2Dc5Zlbs3ft?=
 =?us-ascii?Q?T6LtX/5WaXtoDr6QGrn6D8oNoMtFf9Tg/Gk75yegqgOSVt7AD4rqkUYK0en0?=
 =?us-ascii?Q?pU+i1BWtPxFVfxl8ck+fCRB3glJ2OiQeXii1KgaQj6mKyFjDc9uW9gW4eAJX?=
 =?us-ascii?Q?2VCt4KCnRLzQ3zmY0Z27vzgqFIPBIGtWTZ/ezyQAOEOnfcRuYPapofKsGNNl?=
 =?us-ascii?Q?WVoDgzFXCVEb9udlsekyUbchPuooWP87vrteKT1GIpmjR0l6MqVum90x89W+?=
 =?us-ascii?Q?n6He9+o2hP6Pxp8rJbE4ytBgxE9uEmqzTjngf0TyPWH80z0F9Tiz1/zZZ7gd?=
 =?us-ascii?Q?g9rk9E8XHBAmI57VBKZx0gTl3yqkIQs+TxGS6psFUNo/dTD0tGXNwmW6JrEi?=
 =?us-ascii?Q?UYMv85CinuJfzsgJs0Xkn5IzI9TPRoEAGY1vGXqQPWKPm5ram9gfq/W+GiN/?=
 =?us-ascii?Q?5OEflBB6EYeSi7s+/wvcFD94dpMCaYCjYQY09qKxq6ghTz2Qz4p8gVKprcMW?=
 =?us-ascii?Q?mUVrACQCyV8mZ1gJ2MdlXkY3rralugDN2rUQkrCBRQU58U4D5Fz6MH8CyZY2?=
 =?us-ascii?Q?Bjyt8e639SZHS35wPeoyCfQr2c2v0pUw7v6bi8tQ9MHIhQFptQdV8SA8+b/L?=
 =?us-ascii?Q?4QMPT1XNFvkhkgiKGoYequzemrYSa8i/vvOiIO5wZRRTdwkgIRNztEo9uU3/?=
 =?us-ascii?Q?i7DPUEiCopzACw1KKssPn3CSBG/H5QZpMcuzUXPiO4w4saU6NMygB7Hrk09Y?=
 =?us-ascii?Q?5s+e0QvWaXsCwu9wZVfPz6bqkQ+gbQmJXO+8DBp2/HrkpeoYxOguOn3BqhnM?=
 =?us-ascii?Q?xZFjH6vPCseiR1vrWpqD2+BXJ4RmUWp8LAhVpvKcc1RjOYVD2Upk9l1+zJVE?=
 =?us-ascii?Q?SvMnmMiznDxLslwzpgLnqBrmdM9QFUlt+60TRkcAY73UZ35gxBuNrPtosHpl?=
 =?us-ascii?Q?sjBNjz8GaWja7xoUxR7hSe7pUwvaR7R3b45kGTCZjW4PBkWu3u3U+Tc6r3jW?=
 =?us-ascii?Q?wupJ26I1zwt1RsQrPP2MXzfI/Ryjizuw0hJiVnFDAweO6TuCf7PE3xDPxmH0?=
 =?us-ascii?Q?+UVY1PotmsNdMkk1GyMSOIQSUg89Ry6v+8fFiFZcm2oE+sBrMkEoOS6WEIkA?=
 =?us-ascii?Q?59DXi+qY1yb1sbS+FTDCFHjjf+/yIwVMSBwmE+pGOhHQv5gmyMrWO3e2TjkR?=
 =?us-ascii?Q?aMcBJms6/nJS4OT3pv9egdPuwRsE64cXcvrNHSA6nf+gaLbEUFJMhLsZwF6f?=
 =?us-ascii?Q?N754UkzwojzxSll63svb3wGcNNHakhTtUYUIHf05pJC1H9uX63xmRDJoZvd4?=
 =?us-ascii?Q?GqqNlVdv6BUnZMxyf816i2DoJYwdeyMNGSks7reHcDnaGWSZqHrTevnq95Df?=
 =?us-ascii?Q?xdSE8g/uo/9SAXwXhsjQFc2Fu0ZEaJpMscm01wASP4SrHyyeY1+ssQjnKEEu?=
 =?us-ascii?Q?udxo3qwZJLqN1z9/vN/uZnhzqRisj75upqL56ACuk7MKpzdvdbo07jRzJtuk?=
 =?us-ascii?Q?2FOstR1CHXtMfeVlIt5EtlQJX50gy1MJLULM3hSa1JdXASgxv+p4phX8QQin?=
 =?us-ascii?Q?zUAkHbqHIaVVojYTwNgjXmK9tqL6eDnqH9nfMgSKF/z2dN1fi5JpdKaAlQw6?=
 =?us-ascii?Q?Hw=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: cc0e8c3f-9e8c-4624-2b95-08daceeb6e32
X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB6117.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Nov 2022 13:46:19.1982
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: CIvAen1WOxXh3q325WNN4IplNCuBJHlEnM1Ed4IRgCniD63Ox99bfPUm3RtXQRkf+0k1MQnyWfD7zaxwpl8WQCqZcSE5swyk9jO2vdtXD4M=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL3PR11MB6316
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Nov 24, 2022 at 09:56:43AM +0800, Ziyang Xuan wrote:
> In otx2_probe(), there are several possible memory leak bugs
> in exception paths as follows:
> 1. Do not release pf->otx2_wq when excute otx2_init_tc() failed.
> 2. Do not shutdown tc when excute otx2_register_dl() failed.
> 3. Do not unregister devlink when initialize SR-IOV failed.
> 
> Fixes: 1d4d9e42c240 ("octeontx2-pf: Add tc flower hardware offload on ingress traffic")
> Fixes: 2da489432747 ("octeontx2-pf: devlink params support to set mcam entry count")
> Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
> ---
>  drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> index 303930499a4c..8d7f2c3b0cfd 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> @@ -2900,7 +2900,7 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  
>  	err = otx2_register_dl(pf);
>  	if (err)
> -		goto err_mcam_flow_del;
> +		goto err_register_dl;
>  
>  	/* Initialize SR-IOV resources */
>  	err = otx2_sriov_vfcfg_init(pf);
> @@ -2919,8 +2919,11 @@ static int otx2_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	return 0;

If otx2_dcbnl_set_ops() fails at the end then shouldn't we also call
otx2_sriov_vfcfg_cleanup() ?

>  
>  err_pf_sriov_init:
> +	otx2_unregister_dl(pf);
> +err_register_dl:
>  	otx2_shutdown_tc(pf);
>  err_mcam_flow_del:
> +	destroy_workqueue(pf->otx2_wq);
>  	otx2_mcam_flow_del(pf);
>  err_unreg_netdev:
>  	unregister_netdev(netdev);
> -- 
> 2.25.1
>