From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="BNPKoSNc" Received: from mail-lf1-x12c.google.com (mail-lf1-x12c.google.com [IPv6:2a00:1450:4864:20::12c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 381F610C1 for ; Fri, 17 Nov 2023 23:22:01 -0800 (PST) Received: by mail-lf1-x12c.google.com with SMTP id 2adb3069b0e04-50a6ff9881fso3984124e87.1 for ; Fri, 17 Nov 2023 23:22:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1700292119; x=1700896919; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=Ck4P2+ZKsmJo86GTwgisAYut3quxIggprqt+AHmxY3Q=; b=BNPKoSNcWHUbvE8qoFNll6YHOBwiJfNFjaZkYG2ogJICcuj8dL/62UZhA4TVgpJWQO 4VI48CcBPuzK89sodsPR2BZ0O0IQ+ZtRLyNwMkSud6AYCWDh+0o3copqzNeNXoKoR82T c5++YTDmHyWgp0faxJNH7jZUK7jC/h3VNhr2WRj9wdrv6FSPqGRRqTXBfF4YgfmlFAm4 LaEb87nzAeiCPiFzs9jXnzA6tOk63aaSOs/WEvVJGYi9kxPBrH3RfxvnropSz4RgY2hR cf5Xy08uzdmnQqyZPJxgCHmarwUfrbTdY6pdQ/YyHNLKkYvXeXhYpy7Rt986JT9OqiO7 SSig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700292119; x=1700896919; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Ck4P2+ZKsmJo86GTwgisAYut3quxIggprqt+AHmxY3Q=; b=FnvFkKwq4FCexSlVXgcFHe3rkeNLVinGdMvpFPgyNQp/CO1ivRDYQSBYRZsMGNahIb elPducDJvcYJKaf2dwf2rP4FYs/pMIBwCLC3lDV9cqsRzmKU3A6VLKjMHOGKlJk0YPMo y1ifXsod2fe6scVel8m/Ev44OPUj961bP8sGAKiCjOYpvSKooIDBia4pkh3rISNC+mAO FH7A+/rBMqc74+mA3CkYyZ0PhBUrN3iqTidU4AGEoCX3y74EEc7mOWvRh78/l/0A1zmH egrE0y7q9e/LXZCSQGDtZHUbMV/dbpf9eHDtdxTCSH2CDfBbQQKPK59GcfZsLmHkkEb8 unEA== X-Gm-Message-State: AOJu0YwIo4SbLKfpMqRetiQ245sPDBBCb1Ha8FT7ANGvRUMGZju91bZb vHZ4163DIRNMPe+2SoDBb1T/1OcuuM3XbTHWNN2z/A== X-Google-Smtp-Source: AGHT+IGmCsPW2DtD/iX2I0p/vAioavmL3nUdlFm9LTuiQ8hEFhSVfZynWt3wCr0Ylq+E12kfdVP2MQpjE5tjjXdAwZI= X-Received: by 2002:a05:6512:23aa:b0:507:b15b:8b92 with SMTP id c42-20020a05651223aa00b00507b15b8b92mr1551640lfv.59.1700292119299; Fri, 17 Nov 2023 23:21:59 -0800 (PST) Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20231117091655.872426-1-u.kleine-koenig@pengutronix.de> <20231117091655.872426-4-u.kleine-koenig@pengutronix.de> In-Reply-To: <20231117091655.872426-4-u.kleine-koenig@pengutronix.de> From: Ilias Apalodimas Date: Sat, 18 Nov 2023 09:21:23 +0200 Message-ID: Subject: Re: [PATCH 3/7] net: ethernet: ti: cpsw-new: Don't error out in .remove() To: =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= Cc: Siddharth Vadapalli , Ravi Gunasekaran , Roger Quadros , Alexei Starovoitov , Marek Majtyka , Gerhard Engleder , Rob Herring , Yunsheng Lin , linux-omap@vger.kernel.org, netdev@vger.kernel.org, kernel@pengutronix.de Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 17 Nov 2023 at 11:17, Uwe Kleine-K=C3=B6nig wrote: > > Returning early from .remove() with an error code still results in the > driver unbinding the device. So the driver core ignores the returned erro= r > code and the resources that were not freed are never catched up. In > combination with devm this also often results in use-after-free bugs. > > If runtime resume fails, it's still important to free all resources, so > don't return with an error code, but emit an error message and continue > freeing acquired stuff. > > This prepares changing cpsw_remove() to return void. > > Fixes: ed3525eda4c4 ("net: ethernet: ti: introduce cpsw switchdev based d= river part 1 - dual-emac") > Signed-off-by: Uwe Kleine-K=C3=B6nig > --- > drivers/net/ethernet/ti/cpsw_new.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/ti/cpsw_new.c b/drivers/net/ethernet/ti= /cpsw_new.c > index 0e4f526b1753..a6ce409f563c 100644 > --- a/drivers/net/ethernet/ti/cpsw_new.c > +++ b/drivers/net/ethernet/ti/cpsw_new.c > @@ -2042,16 +2042,24 @@ static int cpsw_remove(struct platform_device *pd= ev) > struct cpsw_common *cpsw =3D platform_get_drvdata(pdev); > int ret; > > - ret =3D pm_runtime_resume_and_get(&pdev->dev); > + ret =3D pm_runtime_get_sync(&pdev->dev); > if (ret < 0) > - return ret; > + /* There is no need to do something about that. The impor= tant > + * thing is to not exit early, but do all cleanup that do= esn't > + * requrie register access. > + */ > + dev_err(&pdev->dev, "runtime resume failed (%pe)\n", > + ERR_PTR(ret)); > > cpsw_unregister_notifiers(cpsw); > cpsw_unregister_devlink(cpsw); > cpsw_unregister_ports(cpsw); > > - cpts_release(cpsw->cpts); > - cpdma_ctlr_destroy(cpsw->dma); > + if (ret >=3D 0) { > + cpts_release(cpsw->cpts); > + cpdma_ctlr_destroy(cpsw->dma); > + } > + > cpsw_remove_dt(cpsw); > pm_runtime_put_sync(&pdev->dev); > pm_runtime_disable(&pdev->dev); > -- > 2.42.0 > Acked-by: Ilias Apalodimas