From patchwork Fri Feb 17 15:26:51 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keith Busch X-Patchwork-Id: 9580113 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id C30E96042F for ; Fri, 17 Feb 2017 15:18:30 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B439C28068 for ; Fri, 17 Feb 2017 15:18:30 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A77D728571; Fri, 17 Feb 2017 15:18:30 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A3A5628068 for ; Fri, 17 Feb 2017 15:18:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934164AbdBQPS3 (ORCPT ); Fri, 17 Feb 2017 10:18:29 -0500 Received: from mga01.intel.com ([192.55.52.88]:49961 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934293AbdBQPS2 (ORCPT ); Fri, 17 Feb 2017 10:18:28 -0500 Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Feb 2017 07:18:27 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,172,1484035200"; d="scan'208";a="1130327022" Received: from unknown (HELO localhost.localdomain) ([10.232.112.96]) by fmsmga002.fm.intel.com with ESMTP; 17 Feb 2017 07:18:27 -0800 Date: Fri, 17 Feb 2017 10:26:51 -0500 From: Keith Busch To: Christoph Hellwig Cc: scott.bauer@intel.com, jonathan.derrick@intel.com, axboe@fb.com, linux-block@vger.kernel.org, linux-nvme@lists.infradead.org Subject: Re: [PATCH 4/4] nvme: re-check security protocol support after reset Message-ID: <20170217152651.GA18275@localhost.localdomain> References: <20170217125941.14319-1-hch@lst.de> <20170217125941.14319-5-hch@lst.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170217125941.14319-5-hch@lst.de> User-Agent: Mutt/1.7.0 (2016-08-17) Sender: linux-block-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Feb 17, 2017 at 01:59:41PM +0100, Christoph Hellwig wrote: > @@ -1789,7 +1789,8 @@ static void nvme_reset_work(struct work_struct *work) > if (result) > goto out; > > - if ((dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) && !dev->ctrl.opal_dev) { > + kfree(dev->ctrl.opal_dev); > + if (dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) { > dev->ctrl.opal_dev = > init_opal_dev(&dev->ctrl, &nvme_sec_submit); > } A couple things. This has a use-after-free in opal_unlock_from_suspend if the nvme device had an opal_dev before, but no longer support the capability after resume. So you'd want to set ctrl.opal_dev to NULL after the free. But we don't want to unconditionally free it anyway during resume since opal_unlock_from_suspend requires the exisiting opal_dev state information saved in the 'unlk_list'. Something like this instead: --- -- diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index ddc51ad..8fa6be9 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -1789,13 +1789,17 @@ static void nvme_reset_work(struct work_struct *work) if (result) goto out; - if ((dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) && !dev->ctrl.opal_dev) { - dev->ctrl.opal_dev = - init_opal_dev(&dev->ctrl, &nvme_sec_submit); + if (dev->ctrl.oacs & NVME_CTRL_OACS_SEC_SUPP) + if (was_suspend && dev->ctrl.opal_dev) + opal_unlock_from_suspend(dev->ctrl.opal_dev); + else if (!dev->ctrl.opal_dev) + dev->ctrl.opal_dev = + init_opal_dev(&dev->ctrl, &nvme_sec_submit); + } else { + kfree(dev->ctrl.opal_dev); + dev->ctrl.opal_dev = NULL; } - if (was_suspend) - opal_unlock_from_suspend(dev->ctrl.opal_dev); result = nvme_setup_io_queues(dev); if (result)