[PATCH v4 1/2] create SMAF module
Thanks for your review I will add a lock in smaf_handle structure.
One of the goal of smaf is to create a standard kernel API to allocate
and secure buffers to avoid forking
while implementing buffer securing feature.
One concern about ION is that the selection of the heap is done by userland
so hardware constraints need to be known by the userland, which is
problematic from my point of view.
Compare to ION I have try to introduce features like securing API,
flexible allocator selection on kernel
and the possibility to add custom allocator and securing modules.
Benjamin
2015-10-06 3:58 GMT+02:00 Laura Abbott :
> On 10/05/2015 03:11 AM, Benjamin Gaignard wrote:
>>
>> diff --git a/drivers/smaf/smaf-core.c b/drivers/smaf/smaf-core.c
>> new file mode 100644
>> index 000..37914e7
>> --- /dev/null
>> +++ b/drivers/smaf/smaf-core.c
>> @@ -0,0 +1,736 @@
>> +/*
>> + * smaf.c
>> + *
>> + * Copyright (C) Linaro SA 2015
>> + * Author: Benjamin Gaignard for Linaro.
>> + * License terms: GNU General Public License (GPL), version 2
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +struct smaf_handle {
>> + struct dma_buf *dmabuf;
>> + struct smaf_allocator *allocator;
>> + struct dma_buf *db_alloc;
>> + size_t length;
>> + unsigned int flags;
>> + int fd;
>> + bool is_secure;
>> + void *secure_ctx;
>> +};
>> +
>> +/**
>> + * struct smaf_device - smaf device node private data
>> + * @misc_dev: the misc device
>> + * @head: list of allocator
>> + * @lock: list and secure pointer mutex
>> + * @secure:pointer to secure functions helpers
>> + */
>> +struct smaf_device {
>> + struct miscdevice misc_dev;
>> + struct list_head head;
>> + /* list and secure pointer lock*/
>> + struct mutex lock;
>> + struct smaf_secure *secure;
>> +};
>> +
>> +static struct smaf_device smaf_dev;
>> +
>> +/**
>> + * smaf_allow_cpu_access return true if CPU can access to memory
>> + * if their is no secure module associated to SMAF assume that CPU can
>> get
>> + * access to the memory.
>> + */
>> +static bool smaf_allow_cpu_access(struct smaf_handle *handle,
>> + unsigned long flags)
>> +{
>> + if (!handle->is_secure)
>> + return true;
>> +
>> + if (!smaf_dev.secure)
>> + return true;
>> +
>> + if (!smaf_dev.secure->allow_cpu_access)
>> + return true;
>> +
>> + return smaf_dev.secure->allow_cpu_access(handle->secure_ctx,
>> flags);
>> +}
>> +
>> +static int smaf_grant_access(struct smaf_handle *handle, struct device
>> *dev,
>> +dma_addr_t addr, size_t size,
>> +enum dma_data_direction dir)
>> +{
>> + if (!handle->is_secure)
>> + return 0;
>> +
>> + if (!smaf_dev.secure)
>> + return -EINVAL;
>> +
>> + if (!smaf_dev.secure->grant_access)
>> + return -EINVAL;
>> +
>> + return smaf_dev.secure->grant_access(handle->secure_ctx,
>> +dev, addr, size, dir);
>> +}
>> +
>> +static void smaf_revoke_access(struct smaf_handle *handle, struct device
>> *dev,
>> + dma_addr_t addr, size_t size,
>> + enum dma_data_direction dir)
>> +{
>> + if (!handle->is_secure)
>> + return;
>> +
>> + if (!smaf_dev.secure)
>> + return;
>> +
>> + if (!smaf_dev.secure->revoke_access)
>> + return;
>> +
>> + smaf_dev.secure->revoke_access(handle->secure_ctx,
>> + dev, addr, size, dir);
>> +}
>> +
>> +static int smaf_secure_handle(struct smaf_handle *handle)
>> +{
>> + if (handle->is_secure)
>> + return 0;
>> +
>> + if (!smaf_dev.secure)
>> + return -EINVAL;
>> +
>> + if (!smaf_dev.secure->create_context)
>> + return -EINVAL;
>> +
>> + handle->secure_ctx = smaf_dev.secure->create_context();
>> +
>> + if (!handle->secure_ctx)
>> + return -EINVAL;
>> +
>> + handle->is_secure = true;
>> + return 0;
>> +}
>> +
>> +static int smaf_unsecure_handle(struct smaf_handle *handle)
>> +{
>> + if (!handle->is_secure)
>> + return 0;
>> +
>> + if (!smaf_dev.secure)
>> + return -EINVAL;
>> +
>> + if (!smaf_dev.secure->destroy_context)
>> + return -EINVAL;
>> +
>> + if (smaf_dev.secure->destroy_context(handle->secure_ctx))
>> + return -EINVAL;
>> +
>> + handle->secure_ctx = NULL;
>> + handle->is_secure = false;
>> + return 0;
>> +}
>
>
> All these functions need to be protected by a lock, otherwise the
> secure state could change. For
[PATCH v4 1/2] create SMAF module
Hi Benjamin,
[auto build test ERROR on v4.3-rc4 -- if it's inappropriate base, please ignore]
config: s390-allmodconfig (attached as .config)
reproduce:
wget
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
-O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=s390
All error/warnings (new ones prefixed by >>):
In file included from arch/s390/include/asm/elf.h:130:0,
from include/linux/elf.h:4,
from include/linux/module.h:15,
from drivers/smaf/smaf-core.c:16:
drivers/smaf/smaf-core.c: In function 'smaf_unsecure_handle':
>> arch/s390/include/asm/mmu_context.h:33:41: error: expected identifier before
>> 'do'
#define destroy_context(mm) do { } while (0)
^
>> drivers/smaf/smaf-core.c:136:23: note: in expansion of macro
>> 'destroy_context'
if (smaf_dev.secure->destroy_context(handle->secure_ctx))
^
vim +/destroy_context +136 drivers/smaf/smaf-core.c
10 #include
11 #include
12 #include
13 #include
14 #include
15 #include
> 16 #include
17 #include
18 #include
19 #include
20 #include
21 #include
22
23 struct smaf_handle {
24 struct dma_buf *dmabuf;
25 struct smaf_allocator *allocator;
26 struct dma_buf *db_alloc;
27 size_t length;
28 unsigned int flags;
29 int fd;
30 bool is_secure;
31 void *secure_ctx;
32 };
33
34 /**
35 * struct smaf_device - smaf device node private data
36 * @misc_dev: the misc device
37 * @head: list of allocator
38 * @lock: list and secure pointer mutex
39 * @secure: pointer to secure functions helpers
40 */
41 struct smaf_device {
42 struct miscdevice misc_dev;
43 struct list_head head;
44 /* list and secure pointer lock*/
45 struct mutex lock;
46 struct smaf_secure *secure;
47 };
48
49 static struct smaf_device smaf_dev;
50
51 /**
52 * smaf_allow_cpu_access return true if CPU can access to memory
53 * if their is no secure module associated to SMAF assume that CPU can
get
54 * access to the memory.
55 */
56 static bool smaf_allow_cpu_access(struct smaf_handle *handle,
57unsigned long flags)
58 {
59 if (!handle->is_secure)
60 return true;
61
62 if (!smaf_dev.secure)
63 return true;
64
65 if (!smaf_dev.secure->allow_cpu_access)
66 return true;
67
68 return smaf_dev.secure->allow_cpu_access(handle->secure_ctx,
flags);
69 }
70
71 static int smaf_grant_access(struct smaf_handle *handle, struct device
*dev,
72 dma_addr_t addr, size_t size,
73 enum dma_data_direction dir)
74 {
75 if (!handle->is_secure)
76 return 0;
77
78 if (!smaf_dev.secure)
79 return -EINVAL;
80
81 if (!smaf_dev.secure->grant_access)
82 return -EINVAL;
83
84 return smaf_dev.secure->grant_access(handle->secure_ctx,
85 dev, addr, size, dir);
86 }
87
88 static void smaf_revoke_access(struct smaf_handle *handle, struct
device *dev,
89 dma_addr_t addr, size_t size,
90 enum dma_data_direction dir)
91 {
92 if (!handle->is_secure)
93 return;
94
95 if (!smaf_dev.secure)
96 return;
97
98 if (!smaf_dev.secure->revoke_access)
99 return;
100
101 smaf_dev.secure->revoke_access(handle->secure_ctx,
102 dev, addr, size, dir);
103 }
104
105 static int smaf_secure_handle(struct smaf_handle *handle)
106 {
107 if (handle->is_secure)
108 return 0;
109
110 if (!smaf_dev.secure)
111 return -EINVAL;
112
113 if (!smaf_dev.secure->create_context)
114 return -EINVAL;
115
116 handle->secure_ctx = smaf_dev.secure->create_context();
117
118 if (!handle->secure_ctx)
119 return -EINVAL;
120
121 handle->is_secure = true;
122 return 0;
123 }
124
125 static int
[PATCH v4 1/2] create SMAF module
On 10/05/2015 03:11 AM, Benjamin Gaignard wrote:
> diff --git a/drivers/smaf/smaf-core.c b/drivers/smaf/smaf-core.c
> new file mode 100644
> index 000..37914e7
> --- /dev/null
> +++ b/drivers/smaf/smaf-core.c
> @@ -0,0 +1,736 @@
> +/*
> + * smaf.c
> + *
> + * Copyright (C) Linaro SA 2015
> + * Author: Benjamin Gaignard for Linaro.
> + * License terms: GNU General Public License (GPL), version 2
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +struct smaf_handle {
> + struct dma_buf *dmabuf;
> + struct smaf_allocator *allocator;
> + struct dma_buf *db_alloc;
> + size_t length;
> + unsigned int flags;
> + int fd;
> + bool is_secure;
> + void *secure_ctx;
> +};
> +
> +/**
> + * struct smaf_device - smaf device node private data
> + * @misc_dev:the misc device
> + * @head:list of allocator
> + * @lock:list and secure pointer mutex
> + * @secure: pointer to secure functions helpers
> + */
> +struct smaf_device {
> + struct miscdevice misc_dev;
> + struct list_head head;
> + /* list and secure pointer lock*/
> + struct mutex lock;
> + struct smaf_secure *secure;
> +};
> +
> +static struct smaf_device smaf_dev;
> +
> +/**
> + * smaf_allow_cpu_access return true if CPU can access to memory
> + * if their is no secure module associated to SMAF assume that CPU can get
> + * access to the memory.
> + */
> +static bool smaf_allow_cpu_access(struct smaf_handle *handle,
> + unsigned long flags)
> +{
> + if (!handle->is_secure)
> + return true;
> +
> + if (!smaf_dev.secure)
> + return true;
> +
> + if (!smaf_dev.secure->allow_cpu_access)
> + return true;
> +
> + return smaf_dev.secure->allow_cpu_access(handle->secure_ctx, flags);
> +}
> +
> +static int smaf_grant_access(struct smaf_handle *handle, struct device *dev,
> + dma_addr_t addr, size_t size,
> + enum dma_data_direction dir)
> +{
> + if (!handle->is_secure)
> + return 0;
> +
> + if (!smaf_dev.secure)
> + return -EINVAL;
> +
> + if (!smaf_dev.secure->grant_access)
> + return -EINVAL;
> +
> + return smaf_dev.secure->grant_access(handle->secure_ctx,
> + dev, addr, size, dir);
> +}
> +
> +static void smaf_revoke_access(struct smaf_handle *handle, struct device
> *dev,
> +dma_addr_t addr, size_t size,
> +enum dma_data_direction dir)
> +{
> + if (!handle->is_secure)
> + return;
> +
> + if (!smaf_dev.secure)
> + return;
> +
> + if (!smaf_dev.secure->revoke_access)
> + return;
> +
> + smaf_dev.secure->revoke_access(handle->secure_ctx,
> +dev, addr, size, dir);
> +}
> +
> +static int smaf_secure_handle(struct smaf_handle *handle)
> +{
> + if (handle->is_secure)
> + return 0;
> +
> + if (!smaf_dev.secure)
> + return -EINVAL;
> +
> + if (!smaf_dev.secure->create_context)
> + return -EINVAL;
> +
> + handle->secure_ctx = smaf_dev.secure->create_context();
> +
> + if (!handle->secure_ctx)
> + return -EINVAL;
> +
> + handle->is_secure = true;
> + return 0;
> +}
> +
> +static int smaf_unsecure_handle(struct smaf_handle *handle)
> +{
> + if (!handle->is_secure)
> + return 0;
> +
> + if (!smaf_dev.secure)
> + return -EINVAL;
> +
> + if (!smaf_dev.secure->destroy_context)
> + return -EINVAL;
> +
> + if (smaf_dev.secure->destroy_context(handle->secure_ctx))
> + return -EINVAL;
> +
> + handle->secure_ctx = NULL;
> + handle->is_secure = false;
> + return 0;
> +}
All these functions need to be protected by a lock, otherwise the
secure state could change. For that matter, I think the smaf_handle
needs a lock to protect its state as well for places like map_dma_buf
>
> +static long smaf_ioctl(struct file *file, unsigned int cmd, unsigned long
> arg)
> +{
> + switch (cmd) {
> + case SMAF_IOC_CREATE:
> + {
> + struct smaf_create_data data;
> + struct smaf_handle *handle;
> +
> + if (copy_from_user(, (void __user *)arg, _IOC_SIZE(cmd)))
> + return -EFAULT;
> +
> + handle = smaf_create_handle(data.length, data.flags);
> + if (!handle)
> + return -EINVAL;
> +
> + if (data.name[0]) {
> + /* user force allocator selection */
> + if (smaf_select_allocator_by_name(handle->dmabuf,
> + data.name)) {
> + dma_buf_put(handle->dmabuf);
[PATCH v4 1/2] create SMAF module
Secure Memory Allocation Framework goal is to be able
to allocate memory that can be securing.
There is so much ways to allocate and securing memory that SMAF
doesn't do it by itself but need help of additional modules.
To be sure to use the correct allocation method SMAF implement
deferred allocation (i.e. allocate memory when only really needed)
Allocation modules (smaf-alloctor.h):
SMAF could manage with multiple allocation modules at same time.
To select the good one SMAF call match() to be sure that a module
can allocate memory for a given list of devices. It is to the module
to check if the devices are compatible or not with it allocation
method.
Securing module (smaf-secure.h):
The way of how securing memory it is done is platform specific.
Secure module is responsible of grant/revoke memory access.
Signed-off-by: Benjamin Gaignard
---
drivers/Kconfig| 2 +
drivers/Makefile | 1 +
drivers/smaf/Kconfig | 5 +
drivers/smaf/Makefile | 1 +
drivers/smaf/smaf-core.c | 736 +
include/linux/smaf-allocator.h | 54 +++
include/linux/smaf-secure.h| 72
include/uapi/linux/smaf.h | 52 +++
8 files changed, 923 insertions(+)
create mode 100644 drivers/smaf/Kconfig
create mode 100644 drivers/smaf/Makefile
create mode 100644 drivers/smaf/smaf-core.c
create mode 100644 include/linux/smaf-allocator.h
create mode 100644 include/linux/smaf-secure.h
create mode 100644 include/uapi/linux/smaf.h
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 46b4a8e..a488c20 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -188,4 +188,6 @@ source "drivers/nvdimm/Kconfig"
source "drivers/nvmem/Kconfig"
+source "drivers/smaf/Kconfig"
+
endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index b250b36..693390b 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -167,3 +167,4 @@ obj-$(CONFIG_THUNDERBOLT) += thunderbolt/
obj-$(CONFIG_CORESIGHT)+= hwtracing/coresight/
obj-$(CONFIG_ANDROID) += android/
obj-$(CONFIG_NVMEM)+= nvmem/
+obj-$(CONFIG_SMAF) += smaf/
diff --git a/drivers/smaf/Kconfig b/drivers/smaf/Kconfig
new file mode 100644
index 000..d36651a
--- /dev/null
+++ b/drivers/smaf/Kconfig
@@ -0,0 +1,5 @@
+config SMAF
+ tristate "Secure Memory Allocation Framework"
+ depends on DMA_SHARED_BUFFER
+ help
+ Choose this option to enable Secure Memory Allocation Framework
diff --git a/drivers/smaf/Makefile b/drivers/smaf/Makefile
new file mode 100644
index 000..40cd882
--- /dev/null
+++ b/drivers/smaf/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SMAF) += smaf-core.o
diff --git a/drivers/smaf/smaf-core.c b/drivers/smaf/smaf-core.c
new file mode 100644
index 000..37914e7
--- /dev/null
+++ b/drivers/smaf/smaf-core.c
@@ -0,0 +1,736 @@
+/*
+ * smaf.c
+ *
+ * Copyright (C) Linaro SA 2015
+ * Author: Benjamin Gaignard for Linaro.
+ * License terms: GNU General Public License (GPL), version 2
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct smaf_handle {
+ struct dma_buf *dmabuf;
+ struct smaf_allocator *allocator;
+ struct dma_buf *db_alloc;
+ size_t length;
+ unsigned int flags;
+ int fd;
+ bool is_secure;
+ void *secure_ctx;
+};
+
+/**
+ * struct smaf_device - smaf device node private data
+ * @misc_dev: the misc device
+ * @head: list of allocator
+ * @lock: list and secure pointer mutex
+ * @secure:pointer to secure functions helpers
+ */
+struct smaf_device {
+ struct miscdevice misc_dev;
+ struct list_head head;
+ /* list and secure pointer lock*/
+ struct mutex lock;
+ struct smaf_secure *secure;
+};
+
+static struct smaf_device smaf_dev;
+
+/**
+ * smaf_allow_cpu_access return true if CPU can access to memory
+ * if their is no secure module associated to SMAF assume that CPU can get
+ * access to the memory.
+ */
+static bool smaf_allow_cpu_access(struct smaf_handle *handle,
+ unsigned long flags)
+{
+ if (!handle->is_secure)
+ return true;
+
+ if (!smaf_dev.secure)
+ return true;
+
+ if (!smaf_dev.secure->allow_cpu_access)
+ return true;
+
+ return smaf_dev.secure->allow_cpu_access(handle->secure_ctx, flags);
+}
+
+static int smaf_grant_access(struct smaf_handle *handle, struct device *dev,
+dma_addr_t addr, size_t size,
+enum dma_data_direction dir)
+{
+ if (!handle->is_secure)
+ return 0;
+
+ if (!smaf_dev.secure)
+ return -EINVAL;
+
+ if (!smaf_dev.secure->grant_access)
+ return -EINVAL;
+
+ return smaf_dev.secure->grant_access(handle->secure_ctx,
+
