Re: [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs

2019-08-26 Thread Greg Kroah-Hartman
On Mon, Aug 26, 2019 at 11:46:11AM -0400, Nayna wrote:
> 
> 
> On 08/26/2019 10:56 AM, Greg Kroah-Hartman wrote:
> > On Mon, Aug 26, 2019 at 09:23:36AM -0400, Nayna Jain wrote:
> > > +static struct kobj_attribute size_attr = __ATTR_RO(size);
> > Wait, why not just normal ATTR_RO()?
> 
> Oh!! Sorry. I am not seeing this macro in sysfs.h. am I missing something ?

Ugh, no, you are right, I thought it was there as the BIN_ATTR_RO() one
was there :)

> > > +static struct bin_attribute data_attr = __BIN_ATTR_RO(data, 
> > > VARIABLE_MAX_SIZE);
> > And BIN_ATTR_RO() here?
> 
> This would have worked. I think I just thought to use the same way as
> __ATTR_RO().

Yes, that's fine to use, sorry for the noise.

greg k-h


Re: [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs

2019-08-26 Thread Nayna




On 08/26/2019 10:56 AM, Greg Kroah-Hartman wrote:

On Mon, Aug 26, 2019 at 09:23:36AM -0400, Nayna Jain wrote:

+static struct kobj_attribute size_attr = __ATTR_RO(size);

Wait, why not just normal ATTR_RO()?


Oh!! Sorry. I am not seeing this macro in sysfs.h. am I missing something ?




+static struct bin_attribute data_attr = __BIN_ATTR_RO(data, VARIABLE_MAX_SIZE);

And BIN_ATTR_RO() here?


This would have worked. I think I just thought to use the same way as 
__ATTR_RO().

I will change this to __BIN_ATTR_RO and use __BIN_ATTR_WO from your patch.

Thanks for the patch.

Thanks & Regards,
    - Nayna


Re: [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs

2019-08-26 Thread Greg Kroah-Hartman
On Mon, Aug 26, 2019 at 09:23:36AM -0400, Nayna Jain wrote:
> +static struct kobj_attribute size_attr = __ATTR_RO(size);

Wait, why not just normal ATTR_RO()?

> +static struct bin_attribute data_attr = __BIN_ATTR_RO(data, 
> VARIABLE_MAX_SIZE);

And BIN_ATTR_RO() here?

Do those not work for you somehow?

thanks,

greg k-h


Re: [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs

2019-08-26 Thread Nayna




On 08/26/2019 10:01 AM, Greg Kroah-Hartman wrote:

On Mon, Aug 26, 2019 at 09:23:36AM -0400, Nayna Jain wrote:

+static struct bin_attribute update_attr = {
+   .attr = {.name = "update", .mode = 0200},
+   .size = VARIABLE_MAX_SIZE,
+   .write = update_write,
+};

Ah, do we need a __BIN_ATTR_WO() macro for you?  That would make this
more obvious, right?


Thanks Greg.  Yes, I agree it will be good to have that.

Thanks & Regards,
 - Nayna



Re: [PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs

2019-08-26 Thread Greg Kroah-Hartman
On Mon, Aug 26, 2019 at 09:23:36AM -0400, Nayna Jain wrote:
> +static struct bin_attribute update_attr = {
> + .attr = {.name = "update", .mode = 0200},
> + .size = VARIABLE_MAX_SIZE,
> + .write = update_write,
> +};

Ah, do we need a __BIN_ATTR_WO() macro for you?  That would make this
more obvious, right?

Other than that minor thing (not a complaint at all) looks much better
to me, nice work:

Reviewed-by: Greg Kroah-Hartman 


[PATCH v3 2/4] powerpc: expose secure variables to userspace via sysfs

2019-08-26 Thread Nayna Jain
PowerNV secure variables, which store the keys used for OS kernel
verification, are managed by the firmware. These secure variables need to
be accessed by the userspace for addition/deletion of the certificates.

This patch adds the sysfs interface to expose secure variables for PowerNV
secureboot. The users shall use this interface for manipulating
the keys stored in the secure variables.

Signed-off-by: Nayna Jain 
---
 Documentation/ABI/testing/sysfs-secvar |  37 +
 arch/powerpc/Kconfig   |  10 ++
 arch/powerpc/kernel/Makefile   |   1 +
 arch/powerpc/kernel/secvar-sysfs.c | 200 +
 4 files changed, 248 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-secvar
 create mode 100644 arch/powerpc/kernel/secvar-sysfs.c

diff --git a/Documentation/ABI/testing/sysfs-secvar 
b/Documentation/ABI/testing/sysfs-secvar
new file mode 100644
index ..815bd8ec4d5e
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-secvar
@@ -0,0 +1,37 @@
+What:  /sys/firmware/secvar
+Date:  August 2019
+Contact:   Nayna Jain 
+Description:   This directory is created if the POWER firmware supports OS
+   secureboot, thereby secure variables. It exposes interface
+   for reading/writing the secure variables
+
+What:  /sys/firmware/secvar/vars
+Date:  August 2019
+Contact:   Nayna Jain 
+Description:   This directory lists all the secure variables that are supported
+   by the firmware.
+
+What:  /sys/firmware/secvar/vars/
+Date:  August 2019
+Contact:   Nayna Jain 
+Description:   Each secure variable is represented as a directory named as
+   . The variable name is unique and is in ASCII
+   representation. The data and size can be determined by reading
+   their respective attribute files.
+
+What:  /sys/firmware/secvar/vars//size
+Date:  August 2019
+Contact:   Nayna Jain 
+Description:   An integer representation of the size of the content of the
+   variable. In other words, it represents the size of the data.
+
+What:  /sys/firmware/secvar/vars//data
+Date:  August 2019
+Contact:   Nayna Jain 
+Description:   A read-only file containing the value of the variable
+
+What:  /sys/firmware/secvar/vars//update
+Date:  August 2019
+Contact:   Nayna Jain 
+Description:   A write-only file that is used to submit the new value for the
+   variable.
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 42109682b727..11f553e68e1f 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -925,6 +925,16 @@ config PPC_SECURE_BOOT
  allows user to enable OS Secure Boot on PowerPC systems that
  have firmware secure boot support.
 
+config SECVAR_SYSFS
+   tristate "Enable sysfs interface for POWER secure variables"
+   depends on PPC_SECURE_BOOT
+   depends on SYSFS
+   help
+ POWER secure variables are managed and controlled by firmware.
+ These variables are exposed to userspace via sysfs to enable
+ read/write operations on these variables. Say Y if you have
+ secure boot enabled and want to expose variables to userspace.
+
 endmenu
 
 config ISA_DMA_API
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 9041563f1c74..cbdac2e6b6f8 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -158,6 +158,7 @@ obj-$(CONFIG_EPAPR_PARAVIRT)+= epapr_paravirt.o 
epapr_hcalls.o
 obj-$(CONFIG_KVM_GUEST)+= kvm.o kvm_emul.o
 
 obj-$(CONFIG_PPC_SECURE_BOOT)  += secboot.o ima_arch.o secvar-ops.o
+obj-$(CONFIG_SECVAR_SYSFS) += secvar-sysfs.o
 
 # Disable GCOV, KCOV & sanitizers in odd or sensitive code
 GCOV_PROFILE_prom_init.o := n
diff --git a/arch/powerpc/kernel/secvar-sysfs.c 
b/arch/powerpc/kernel/secvar-sysfs.c
new file mode 100644
index ..020529a5f6fb
--- /dev/null
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -0,0 +1,200 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 IBM Corporation 
+ *
+ * This code exposes secure variables to user via sysfs
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Approximating it for now. It will be read from device tree */
+#define VARIABLE_MAX_SIZE  32000
+/* Approximate value */
+#define NAME_MAX_SIZE 1024
+
+static struct kobject *secvar_kobj;
+static struct kset *secvar_kset;
+
+static ssize_t size_show(struct kobject *kobj, struct kobj_attribute *attr,
+char *buf)
+{
+   uint64_t dsize;
+   int rc;
+
+   rc = secvar_ops->get(kobj->name, strlen(kobj->name) + 1, NULL, );
+   if (rc) {
+   pr_err("Error retrieving variable size %d\n", rc);
+   return rc;
+   }
+
+   rc = sprintf(buf, "%llu\n", dsize);
+
+   return rc;
+}
+
+static ssize_t