Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
IronPort-SDR: UGDWHMxGWdhRT3JdA1QeW7lf46f0O7zb1jbYR9FDQ64+qszRmb79OkttOFmot3UBcMUJdGOKs5
 fOZyNYgiOwprXxYuByJWQmQirdy5/gbeDP70QYIoC4wnutlAMpozem5k7Vju2MWz3y8RtTqXSn
 EX5ic6u6tLO/0NM6kRHM3ZaPxdcU468+7M+cses3cWb0HRLBL/03p6T4GTWzUA3GW1roR7lU/V
 OEfCJOvMVYo2dwKWVKtQFF8vLIJWR6SsnUkck7i0gg+PF38kO/BXEslhpNdbEKZ6OSjHOqJGnI
 xPQ=
From:   Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
To:     Logan Gunthorpe <logang@deltatee.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>
CC:     Christoph Hellwig <hch@lst.de>, Keith Busch <kbusch@kernel.org>,
        Jens Axboe <axboe@fb.com>, Sagi Grimberg <sagi@grimberg.me>,
        Max Gurtovoy <maxg@mellanox.com>
Subject: Re: [PATCH v3 1/4] nvmet: Fix use-after-free bug when a port is
 removed
Thread-Topic: [PATCH v3 1/4] nvmet: Fix use-after-free bug when a port is
 removed
Thread-Index: AQHVR/i8tpJRykRFFUC6z3IaFYfMVA==
Date:   Wed, 31 Jul 2019 23:52:06 +0000
Message-ID: <BYAPR04MB5749D4309C95D1318BEC505B86DF0@BYAPR04MB5749.namprd04.prod.outlook.com>
References: <20190731233534.4841-1-logang@deltatee.com>
 <20190731233534.4841-2-logang@deltatee.com>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 8c1521fe-60df-4ed3-ba22-08d716121827
X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Jul 2019 23:52:06.4804
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Chaitanya.Kulkarni@wdc.com
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR04MB4455
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Looks good.=0A=
=0A=
Reviewed-by : Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>=0A=
=0A=
On 07/31/2019 04:36 PM, Logan Gunthorpe wrote:=0A=
> When a port is removed through configfs, any connected controllers=0A=
> are still active and can still send commands. This causes a=0A=
> use-after-free bug which is detected by KASAN for any admin command=0A=
> that dereferences req->port (like in nvmet_execute_identify_ctrl).=0A=
>=0A=
> To fix this, disconnect all active controllers when a subsystem is=0A=
> removed from a port. This ensures there are no active controllers=0A=
> when the port is eventually removed.=0A=
>=0A=
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>=0A=
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>=0A=
> Reviewed-by: Max Gurtovoy <maxg@mellanox.com>=0A=
> ---=0A=
>   drivers/nvme/target/configfs.c |  1 +=0A=
>   drivers/nvme/target/core.c     | 12 ++++++++++++=0A=
>   drivers/nvme/target/nvmet.h    |  3 +++=0A=
>   3 files changed, 16 insertions(+)=0A=
>=0A=
> diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configf=
s.c=0A=
> index cd52b9f15376..98613a45bd3b 100644=0A=
> --- a/drivers/nvme/target/configfs.c=0A=
> +++ b/drivers/nvme/target/configfs.c=0A=
> @@ -675,6 +675,7 @@ static void nvmet_port_subsys_drop_link(struct config=
_item *parent,=0A=
>=0A=
>   found:=0A=
>   	list_del(&p->entry);=0A=
> +	nvmet_port_del_ctrls(port, subsys);=0A=
>   	nvmet_port_disc_changed(port, subsys);=0A=
>=0A=
>   	if (list_empty(&port->subsystems))=0A=
> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c=0A=
> index dad0243c7c96..b86a23aa9020 100644=0A=
> --- a/drivers/nvme/target/core.c=0A=
> +++ b/drivers/nvme/target/core.c=0A=
> @@ -280,6 +280,18 @@ void nvmet_unregister_transport(const struct nvmet_f=
abrics_ops *ops)=0A=
>   }=0A=
>   EXPORT_SYMBOL_GPL(nvmet_unregister_transport);=0A=
>=0A=
> +void nvmet_port_del_ctrls(struct nvmet_port *port, struct nvmet_subsys *=
subsys)=0A=
> +{=0A=
> +	struct nvmet_ctrl *ctrl;=0A=
> +=0A=
> +	mutex_lock(&subsys->lock);=0A=
> +	list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry) {=0A=
> +		if (ctrl->port =3D=3D port)=0A=
> +			ctrl->ops->delete_ctrl(ctrl);=0A=
> +	}=0A=
> +	mutex_unlock(&subsys->lock);=0A=
> +}=0A=
> +=0A=
>   int nvmet_enable_port(struct nvmet_port *port)=0A=
>   {=0A=
>   	const struct nvmet_fabrics_ops *ops;=0A=
> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h=0A=
> index 6ee66c610739..c51f8dd01dc4 100644=0A=
> --- a/drivers/nvme/target/nvmet.h=0A=
> +++ b/drivers/nvme/target/nvmet.h=0A=
> @@ -418,6 +418,9 @@ void nvmet_port_send_ana_event(struct nvmet_port *por=
t);=0A=
>   int nvmet_register_transport(const struct nvmet_fabrics_ops *ops);=0A=
>   void nvmet_unregister_transport(const struct nvmet_fabrics_ops *ops);=
=0A=
>=0A=
> +void nvmet_port_del_ctrls(struct nvmet_port *port,=0A=
> +			  struct nvmet_subsys *subsys);=0A=
> +=0A=
>   int nvmet_enable_port(struct nvmet_port *port);=0A=
>   void nvmet_disable_port(struct nvmet_port *port);=0A=
>=0A=
>=0A=
=0A=