diff mbox series

[v2,18/19] libnvdimm: Introduce CONFIG_NVDIMM_SECURITY_TEST flag

Message ID 166377439534.430546.10690686781480251163.stgit@djiang5-desk3.ch.intel.com (mailing list archive)
State Superseded
Headers show
Series Introduce security commands for CXL pmem device | expand

Commit Message

Dave Jiang Sept. 21, 2022, 3:33 p.m. UTC
nfit_test overrode the security_show() sysfs attribute function in nvdimm
dimm_devs in order to allow testing of security unlock. With the
introduction of CXL security commands, the trick to override
security_show() becomes significantly more complicated. By introdcing a
security flag CONFIG_NVDIMM_SECURITY_TEST, libnvdimm can just toggle the
check via a compile option. In addition the original override can can be
removed from tools/testing/nvdimm/.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/nvdimm/Kconfig           |    9 +++++++++
 drivers/nvdimm/dimm_devs.c       |    9 ++++++++-
 drivers/nvdimm/security.c        |    4 ++++
 tools/testing/nvdimm/Kbuild      |    1 -
 tools/testing/nvdimm/dimm_devs.c |   30 ------------------------------
 5 files changed, 21 insertions(+), 32 deletions(-)
 delete mode 100644 tools/testing/nvdimm/dimm_devs.c

Comments

Jonathan Cameron Nov. 7, 2022, 4:01 p.m. UTC | #1
On Wed, 21 Sep 2022 08:33:15 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> nfit_test overrode the security_show() sysfs attribute function in nvdimm
> dimm_devs in order to allow testing of security unlock. With the
> introduction of CXL security commands, the trick to override
> security_show() becomes significantly more complicated. By introdcing a
> security flag CONFIG_NVDIMM_SECURITY_TEST, libnvdimm can just toggle the
> check via a compile option. In addition the original override can can be
> removed from tools/testing/nvdimm/.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/nvdimm/Kconfig           |    9 +++++++++
>  drivers/nvdimm/dimm_devs.c       |    9 ++++++++-
>  drivers/nvdimm/security.c        |    4 ++++
>  tools/testing/nvdimm/Kbuild      |    1 -
>  tools/testing/nvdimm/dimm_devs.c |   30 ------------------------------
>  5 files changed, 21 insertions(+), 32 deletions(-)
>  delete mode 100644 tools/testing/nvdimm/dimm_devs.c
> 
> diff --git a/drivers/nvdimm/Kconfig b/drivers/nvdimm/Kconfig
> index 5a29046e3319..fd336d138eda 100644
> --- a/drivers/nvdimm/Kconfig
> +++ b/drivers/nvdimm/Kconfig
> @@ -114,4 +114,13 @@ config NVDIMM_TEST_BUILD
>  	  core devm_memremap_pages() implementation and other
>  	  infrastructure.
>  
> +config NVDIMM_SECURITY_TEST
> +	bool "Nvdimm security test code toggle"
> +	depends on NVDIMM_KEYS
> +	help
> +	  Debug flag for security testing when using nfit_test or cxl_test
> +	  modules in tools/testing/.
> +
> +	  Select Y if using nfit_test or cxl_test for security testing.

Probably want to say it if has any unfortunate side effects when not doing
such testing.  Distros will want guidance on whether to enable.

Jonathan

> +
Dave Jiang Nov. 7, 2022, 11:46 p.m. UTC | #2
On 11/7/2022 8:01 AM, Jonathan Cameron wrote:
> On Wed, 21 Sep 2022 08:33:15 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
>> nfit_test overrode the security_show() sysfs attribute function in nvdimm
>> dimm_devs in order to allow testing of security unlock. With the
>> introduction of CXL security commands, the trick to override
>> security_show() becomes significantly more complicated. By introdcing a
>> security flag CONFIG_NVDIMM_SECURITY_TEST, libnvdimm can just toggle the
>> check via a compile option. In addition the original override can can be
>> removed from tools/testing/nvdimm/.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>   drivers/nvdimm/Kconfig           |    9 +++++++++
>>   drivers/nvdimm/dimm_devs.c       |    9 ++++++++-
>>   drivers/nvdimm/security.c        |    4 ++++
>>   tools/testing/nvdimm/Kbuild      |    1 -
>>   tools/testing/nvdimm/dimm_devs.c |   30 ------------------------------
>>   5 files changed, 21 insertions(+), 32 deletions(-)
>>   delete mode 100644 tools/testing/nvdimm/dimm_devs.c
>>
>> diff --git a/drivers/nvdimm/Kconfig b/drivers/nvdimm/Kconfig
>> index 5a29046e3319..fd336d138eda 100644
>> --- a/drivers/nvdimm/Kconfig
>> +++ b/drivers/nvdimm/Kconfig
>> @@ -114,4 +114,13 @@ config NVDIMM_TEST_BUILD
>>   	  core devm_memremap_pages() implementation and other
>>   	  infrastructure.
>>   
>> +config NVDIMM_SECURITY_TEST
>> +	bool "Nvdimm security test code toggle"
>> +	depends on NVDIMM_KEYS
>> +	help
>> +	  Debug flag for security testing when using nfit_test or cxl_test
>> +	  modules in tools/testing/.
>> +
>> +	  Select Y if using nfit_test or cxl_test for security testing.
> 
> Probably want to say it if has any unfortunate side effects when not doing
> such testing.  Distros will want guidance on whether to enable.

Ok will add.
> 
> Jonathan
> 
>> +
> 
>
diff mbox series

Patch

diff --git a/drivers/nvdimm/Kconfig b/drivers/nvdimm/Kconfig
index 5a29046e3319..fd336d138eda 100644
--- a/drivers/nvdimm/Kconfig
+++ b/drivers/nvdimm/Kconfig
@@ -114,4 +114,13 @@  config NVDIMM_TEST_BUILD
 	  core devm_memremap_pages() implementation and other
 	  infrastructure.
 
+config NVDIMM_SECURITY_TEST
+	bool "Nvdimm security test code toggle"
+	depends on NVDIMM_KEYS
+	help
+	  Debug flag for security testing when using nfit_test or cxl_test
+	  modules in tools/testing/.
+
+	  Select Y if using nfit_test or cxl_test for security testing.
+
 endif
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index c7c980577491..1fc081dcf631 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -349,11 +349,18 @@  static ssize_t available_slots_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(available_slots);
 
-__weak ssize_t security_show(struct device *dev,
+ssize_t security_show(struct device *dev,
 		struct device_attribute *attr, char *buf)
 {
 	struct nvdimm *nvdimm = to_nvdimm(dev);
 
+	/*
+	 * For the test version we need to poll the "hardware" in order
+	 * to get the updated status for unlock testing.
+	 */
+	if (IS_ENABLED(CONFIG_NVDIMM_SECURITY_TEST))
+		nvdimm->sec.flags = nvdimm_security_flags(nvdimm, NVDIMM_USER);
+
 	if (test_bit(NVDIMM_SECURITY_OVERWRITE, &nvdimm->sec.flags))
 		return sprintf(buf, "overwrite\n");
 	if (test_bit(NVDIMM_SECURITY_DISABLED, &nvdimm->sec.flags))
diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
index c1c9d0feae9d..6ae924d8f2d7 100644
--- a/drivers/nvdimm/security.c
+++ b/drivers/nvdimm/security.c
@@ -177,6 +177,10 @@  static int __nvdimm_security_unlock(struct nvdimm *nvdimm)
 			|| !nvdimm->sec.flags)
 		return -EIO;
 
+	/* While nfit_test does not need this, cxl_test does */
+	if (IS_ENABLED(CONFIG_NVDIMM_SECURITY_TEST))
+		nvdimm->sec.flags = nvdimm_security_flags(nvdimm, NVDIMM_USER);
+
 	/* No need to go further if security is disabled */
 	if (test_bit(NVDIMM_SECURITY_DISABLED, &nvdimm->sec.flags))
 		return 0;
diff --git a/tools/testing/nvdimm/Kbuild b/tools/testing/nvdimm/Kbuild
index 5eb5c23b062f..8153251ea389 100644
--- a/tools/testing/nvdimm/Kbuild
+++ b/tools/testing/nvdimm/Kbuild
@@ -79,7 +79,6 @@  libnvdimm-$(CONFIG_BTT) += $(NVDIMM_SRC)/btt_devs.o
 libnvdimm-$(CONFIG_NVDIMM_PFN) += $(NVDIMM_SRC)/pfn_devs.o
 libnvdimm-$(CONFIG_NVDIMM_DAX) += $(NVDIMM_SRC)/dax_devs.o
 libnvdimm-$(CONFIG_NVDIMM_KEYS) += $(NVDIMM_SRC)/security.o
-libnvdimm-y += dimm_devs.o
 libnvdimm-y += libnvdimm_test.o
 libnvdimm-y += config_check.o
 
diff --git a/tools/testing/nvdimm/dimm_devs.c b/tools/testing/nvdimm/dimm_devs.c
deleted file mode 100644
index 57bd27dedf1f..000000000000
--- a/tools/testing/nvdimm/dimm_devs.c
+++ /dev/null
@@ -1,30 +0,0 @@ 
-// SPDX-License-Identifier: GPL-2.0
-/* Copyright Intel Corp. 2018 */
-#include <linux/init.h>
-#include <linux/module.h>
-#include <linux/moduleparam.h>
-#include <linux/nd.h>
-#include "pmem.h"
-#include "pfn.h"
-#include "nd.h"
-#include "nd-core.h"
-
-ssize_t security_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
-{
-	struct nvdimm *nvdimm = to_nvdimm(dev);
-
-	/*
-	 * For the test version we need to poll the "hardware" in order
-	 * to get the updated status for unlock testing.
-	 */
-	nvdimm->sec.flags = nvdimm_security_flags(nvdimm, NVDIMM_USER);
-
-	if (test_bit(NVDIMM_SECURITY_DISABLED, &nvdimm->sec.flags))
-		return sprintf(buf, "disabled\n");
-	if (test_bit(NVDIMM_SECURITY_UNLOCKED, &nvdimm->sec.flags))
-		return sprintf(buf, "unlocked\n");
-	if (test_bit(NVDIMM_SECURITY_LOCKED, &nvdimm->sec.flags))
-		return sprintf(buf, "locked\n");
-	return -ENOTTY;
-}