[PATCH 0/3] Security: add lsm hooks for checking permissions on eBPF objects

[PATCH 0/3] Security: add lsm hooks for checking permissions on eBPF objects

[Chenbo Feng]

From: Chenbo Feng <fengc@google.com>

Much like files and sockets, eBPF objects are accessed, controlled, and
shared via a file descriptor (FD). Unlike files and sockets, the existing
mechanism for eBPF object access control is very limited. Currently there
are two options for granting accessing to eBPF operations: grant access to
all processes, or only CAP_SYS_ADMIN processes. The CAP_SYS_ADMIN-only
mode is not ideal because most users do not have this capability and
granting a user CAP_SYS_ADMIN grants too many other security-sensitive
permissions. It also unnecessarily allows all CAP_SYS_ADMIN processes
access to eBPF functionality. Allowing all processes to access to eBPF
objects is also undesirable since it has potential to allow unprivileged
processes to consume kernel memory, and opens up attack surface to the
kernel.

Adding LSM hooks maintains the status quo for systems which do not use an
LSM, preserving compatibility with userspace, while allowing security
modules to choose how best to handle permissions on eBPF objects. Here is
a possible use case for the lsm hooks with selinux module:

The network-control daemon (netd) creates and loads an eBPF object for
network packet filtering and analysis. It passes the object FD to an
unprivileged network monitor app (netmonitor), which is not allowed to
create, modify or load eBPF objects, but is allowed to read the traffic
stats from the object.

Selinux could use these hooks to grant the following permissions:
allow netd self:bpf { create modify read…};
allow netmonitor netd:bpf read;

In this patch series, 5 security hooks is added to the eBPF syscall
implementations to do permissions checks. The LSM hooks introduced to eBPF
maps and programs can be summarized as follows:

	Bpf_map_create: check for the ability of creating eBPF maps.
        Bpf_map_modify: check the ability of update and delete eBPF map
			entries.
        Bpf_map_read: 	check the ability of lookup map element as well as
		      	get map keys.
        Bpf_post_create: initialize the security struct inside struct
			 bpf_map
        Bpf_prog_load: check the ability for loading the eBPF program.

In order to store the ownership and security information about eBPF maps,
a security field pointer is added to the struct bpf_map. And a simple
implementation of selinux check on these hooks is added in selinux
subsystem.

Chenbo Feng (3):
  security: bpf: Add eBPF LSM hooks to security module
  security: bpf: Add eBPF LSM hooks and security field to eBPF map
  selinux: bpf: Implement the selinux checks for eBPF object

 include/linux/bpf.h                 |  3 +++
 include/linux/lsm_hooks.h           | 41 ++++++++++++++++++++++++++++
 include/linux/security.h            | 36 +++++++++++++++++++++++++
 kernel/bpf/syscall.c                | 28 +++++++++++++++++++
 security/security.c                 | 28 +++++++++++++++++++
 security/selinux/hooks.c            | 54 +++++++++++++++++++++++++++++++++++++
 security/selinux/include/classmap.h |  2 ++
 security/selinux/include/objsec.h   |  4 +++
 8 files changed, 196 insertions(+)

--
2.14.1.581.gf28d330327-goog

[PATCH 0/3] Security: add lsm hooks for checking permissions on eBPF objects

[Chenbo Feng]

From: Chenbo Feng <fengc@google.com>

Much like files and sockets, eBPF objects are accessed, controlled, and
shared via a file descriptor (FD). Unlike files and sockets, the existing
mechanism for eBPF object access control is very limited. Currently there
are two options for granting accessing to eBPF operations: grant access to
all processes, or only CAP_SYS_ADMIN processes. The CAP_SYS_ADMIN-only
mode is not ideal because most users do not have this capability and
granting a user CAP_SYS_ADMIN grants too many other security-sensitive
permissions. It also unnecessarily allows all CAP_SYS_ADMIN processes
access to eBPF functionality. Allowing all processes to access to eBPF
objects is also undesirable since it has potential to allow unprivileged
processes to consume kernel memory, and opens up attack surface to the
kernel.

Adding LSM hooks maintains the status quo for systems which do not use an
LSM, preserving compatibility with userspace, while allowing security
modules to choose how best to handle permissions on eBPF objects. Here is
a possible use case for the lsm hooks with selinux module:

The network-control daemon (netd) creates and loads an eBPF object for
network packet filtering and analysis. It passes the object FD to an
unprivileged network monitor app (netmonitor), which is not allowed to
create, modify or load eBPF objects, but is allowed to read the traffic
stats from the object.

Selinux could use these hooks to grant the following permissions:
allow netd self:bpf { create modify read?};
allow netmonitor netd:bpf read;

In this patch series, 5 security hooks is added to the eBPF syscall
implementations to do permissions checks. The LSM hooks introduced to eBPF
maps and programs can be summarized as follows:

	Bpf_map_create: check for the ability of creating eBPF maps.
        Bpf_map_modify: check the ability of update and delete eBPF map
			entries.
        Bpf_map_read: 	check the ability of lookup map element as well as
		      	get map keys.
        Bpf_post_create: initialize the security struct inside struct
			 bpf_map
        Bpf_prog_load: check the ability for loading the eBPF program.

In order to store the ownership and security information about eBPF maps,
a security field pointer is added to the struct bpf_map. And a simple
implementation of selinux check on these hooks is added in selinux
subsystem.

Chenbo Feng (3):
  security: bpf: Add eBPF LSM hooks to security module
  security: bpf: Add eBPF LSM hooks and security field to eBPF map
  selinux: bpf: Implement the selinux checks for eBPF object

 include/linux/bpf.h                 |  3 +++
 include/linux/lsm_hooks.h           | 41 ++++++++++++++++++++++++++++
 include/linux/security.h            | 36 +++++++++++++++++++++++++
 kernel/bpf/syscall.c                | 28 +++++++++++++++++++
 security/security.c                 | 28 +++++++++++++++++++
 security/selinux/hooks.c            | 54 +++++++++++++++++++++++++++++++++++++
 security/selinux/include/classmap.h |  2 ++
 security/selinux/include/objsec.h   |  4 +++
 8 files changed, 196 insertions(+)

--
2.14.1.581.gf28d330327-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] security: bpf: Add eBPF LSM hooks to security module

[Chenbo Feng]

From: Chenbo Feng <fengc@google.com>

Introduce 5 LSM hooks to provide finer granularity controls on eBPF
related operations including create eBPF maps, modify and read eBPF maps
content and load eBPF programs to the kernel. Hooks use the new security
pointer inside the eBPF map struct to store the owner's security
information and the different security modules can perform different
checks based on the information stored inside the security field.

Signed-off-by: Chenbo Feng <fengc@google.com>
---
 include/linux/lsm_hooks.h | 41 +++++++++++++++++++++++++++++++++++++++++
 include/linux/security.h  | 36 ++++++++++++++++++++++++++++++++++++
 security/security.c       | 28 ++++++++++++++++++++++++++++
 3 files changed, 105 insertions(+)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index ce02f76a6188..3aaf9a08a983 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1353,6 +1353,32 @@
  *	@inode we wish to get the security context of.
  *	@ctx is a pointer in which to place the allocated security context.
  *	@ctxlen points to the place to put the length of @ctx.
+ *
+ * Security hooks for using the eBPF maps and programs functionalities through
+ * eBPF syscalls.
+ *
+ * @bpf_map_create:
+ *	Check permissions prior to creating a new bpf map.
+ *	Return 0 if the permission is granted.
+ *
+ * @bpf_map_modify:
+ *	Check permission prior to insert, update and delete map content.
+ *	@map pointer to the struct bpf_map that contains map information.
+ *	Return 0 if the permission is granted.
+ *
+ * @bpf_map_read:
+ *	Check permission prior to read a bpf map content.
+ *	@map pointer to the struct bpf_map that contains map information.
+ *	Return 0 if the permission is granted.
+ *
+ * @bpf_prog_load:
+ *	Check permission prior to load eBPF program.
+ *	Return 0 if the permission is granted.
+ *
+ * @bpf_post_create:
+ *	Initialize the bpf object security field inside struct bpf_maps and
+ *	it is used for future security checks.
+ *
  */
 union security_list_options {
 	int (*binder_set_context_mgr)(struct task_struct *mgr);
@@ -1685,6 +1711,14 @@ union security_list_options {
 				struct audit_context *actx);
 	void (*audit_rule_free)(void *lsmrule);
 #endif /* CONFIG_AUDIT */
+
+#ifdef CONFIG_BPF_SYSCALL
+	int (*bpf_map_create)(void);
+	int (*bpf_map_read)(struct bpf_map *map);
+	int (*bpf_map_modify)(struct bpf_map *map);
+	int (*bpf_prog_load)(void);
+	int (*bpf_post_create)(struct bpf_map *map);
+#endif /* CONFIG_BPF_SYSCALL */
 };
 
 struct security_hook_heads {
@@ -1905,6 +1939,13 @@ struct security_hook_heads {
 	struct list_head audit_rule_match;
 	struct list_head audit_rule_free;
 #endif /* CONFIG_AUDIT */
+#ifdef CONFIG_BPF_SYSCALL
+	struct list_head bpf_map_create;
+	struct list_head bpf_map_read;
+	struct list_head bpf_map_modify;
+	struct list_head bpf_prog_load;
+	struct list_head bpf_post_create;
+#endif /* CONFIG_BPF_SYSCALL */
 } __randomize_layout;
 
 /*
diff --git a/include/linux/security.h b/include/linux/security.h
index 458e24bea2d4..0656a4f74d14 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -31,6 +31,7 @@
 #include <linux/string.h>
 #include <linux/mm.h>
 #include <linux/fs.h>
+#include <linux/bpf.h>
 
 struct linux_binprm;
 struct cred;
@@ -1735,6 +1736,41 @@ static inline void securityfs_remove(struct dentry *dentry)
 
 #endif
 
+#ifdef CONFIG_BPF_SYSCALL
+#ifdef CONFIG_SECURITY
+int security_map_create(void);
+int security_map_modify(struct bpf_map *map);
+int security_map_read(struct bpf_map *map);
+int security_prog_load(void);
+int security_post_create(struct bpf_map *map);
+#else
+static inline int security_map_create(void)
+{
+	return 0;
+}
+
+static inline int security_map_read(struct bpf_map *map)
+{
+	return 0;
+}
+
+static inline int security_map_modify(struct bpf_map *map)
+{
+	return 0;
+}
+
+static inline int security_prog_load(void)
+{
+	return 0;
+}
+
+static inline int security_post_create(struct bpf_map *map)
+{
+	return 0;
+}
+#endif /* CONFIG_SECURITY */
+#endif /* CONFIG_BPF_SYSCALL */
+
 #ifdef CONFIG_SECURITY
 
 static inline char *alloc_secdata(void)
diff --git a/security/security.c b/security/security.c
index 55b5997e4b72..02272f93a89e 100644
--- a/security/security.c
+++ b/security/security.c
@@ -12,6 +12,7 @@
  *	(at your option) any later version.
  */
 
+#include <linux/bpf.h>
 #include <linux/capability.h>
 #include <linux/dcache.h>
 #include <linux/module.h>
@@ -1708,3 +1709,30 @@ int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule,
 				actx);
 }
 #endif /* CONFIG_AUDIT */
+
+#ifdef CONFIG_BPF_SYSCALL
+int security_map_create(void)
+{
+	return call_int_hook(bpf_map_create, 0);
+}
+
+int security_map_modify(struct bpf_map *map)
+{
+	return call_int_hook(bpf_map_modify, 0, map);
+}
+
+int security_map_read(struct bpf_map *map)
+{
+	return call_int_hook(bpf_map_read, 0, map);
+}
+
+int security_prog_load(void)
+{
+	return call_int_hook(bpf_prog_load, 0);
+}
+
+int security_post_create(struct bpf_map *map)
+{
+	return call_int_hook(bpf_post_create, 0, map);
+}
+#endif /* CONFIG_BPF_SYSCALL */
-- 
2.14.1.581.gf28d330327-goog

[PATCH 1/3] security: bpf: Add eBPF LSM hooks to security module

[Chenbo Feng]

From: Chenbo Feng <fengc@google.com>

Introduce 5 LSM hooks to provide finer granularity controls on eBPF
related operations including create eBPF maps, modify and read eBPF maps
content and load eBPF programs to the kernel. Hooks use the new security
pointer inside the eBPF map struct to store the owner's security
information and the different security modules can perform different
checks based on the information stored inside the security field.

Signed-off-by: Chenbo Feng <fengc@google.com>
---
 include/linux/lsm_hooks.h | 41 +++++++++++++++++++++++++++++++++++++++++
 include/linux/security.h  | 36 ++++++++++++++++++++++++++++++++++++
 security/security.c       | 28 ++++++++++++++++++++++++++++
 3 files changed, 105 insertions(+)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index ce02f76a6188..3aaf9a08a983 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1353,6 +1353,32 @@
  *	@inode we wish to get the security context of.
  *	@ctx is a pointer in which to place the allocated security context.
  *	@ctxlen points to the place to put the length of @ctx.
+ *
+ * Security hooks for using the eBPF maps and programs functionalities through
+ * eBPF syscalls.
+ *
+ * @bpf_map_create:
+ *	Check permissions prior to creating a new bpf map.
+ *	Return 0 if the permission is granted.
+ *
+ * @bpf_map_modify:
+ *	Check permission prior to insert, update and delete map content.
+ *	@map pointer to the struct bpf_map that contains map information.
+ *	Return 0 if the permission is granted.
+ *
+ * @bpf_map_read:
+ *	Check permission prior to read a bpf map content.
+ *	@map pointer to the struct bpf_map that contains map information.
+ *	Return 0 if the permission is granted.
+ *
+ * @bpf_prog_load:
+ *	Check permission prior to load eBPF program.
+ *	Return 0 if the permission is granted.
+ *
+ * @bpf_post_create:
+ *	Initialize the bpf object security field inside struct bpf_maps and
+ *	it is used for future security checks.
+ *
  */
 union security_list_options {
 	int (*binder_set_context_mgr)(struct task_struct *mgr);
@@ -1685,6 +1711,14 @@ union security_list_options {
 				struct audit_context *actx);
 	void (*audit_rule_free)(void *lsmrule);
 #endif /* CONFIG_AUDIT */
+
+#ifdef CONFIG_BPF_SYSCALL
+	int (*bpf_map_create)(void);
+	int (*bpf_map_read)(struct bpf_map *map);
+	int (*bpf_map_modify)(struct bpf_map *map);
+	int (*bpf_prog_load)(void);
+	int (*bpf_post_create)(struct bpf_map *map);
+#endif /* CONFIG_BPF_SYSCALL */
 };
 
 struct security_hook_heads {
@@ -1905,6 +1939,13 @@ struct security_hook_heads {
 	struct list_head audit_rule_match;
 	struct list_head audit_rule_free;
 #endif /* CONFIG_AUDIT */
+#ifdef CONFIG_BPF_SYSCALL
+	struct list_head bpf_map_create;
+	struct list_head bpf_map_read;
+	struct list_head bpf_map_modify;
+	struct list_head bpf_prog_load;
+	struct list_head bpf_post_create;
+#endif /* CONFIG_BPF_SYSCALL */
 } __randomize_layout;
 
 /*
diff --git a/include/linux/security.h b/include/linux/security.h
index 458e24bea2d4..0656a4f74d14 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -31,6 +31,7 @@
 #include <linux/string.h>
 #include <linux/mm.h>
 #include <linux/fs.h>
+#include <linux/bpf.h>
 
 struct linux_binprm;
 struct cred;
@@ -1735,6 +1736,41 @@ static inline void securityfs_remove(struct dentry *dentry)
 
 #endif
 
+#ifdef CONFIG_BPF_SYSCALL
+#ifdef CONFIG_SECURITY
+int security_map_create(void);
+int security_map_modify(struct bpf_map *map);
+int security_map_read(struct bpf_map *map);
+int security_prog_load(void);
+int security_post_create(struct bpf_map *map);
+#else
+static inline int security_map_create(void)
+{
+	return 0;
+}
+
+static inline int security_map_read(struct bpf_map *map)
+{
+	return 0;
+}
+
+static inline int security_map_modify(struct bpf_map *map)
+{
+	return 0;
+}
+
+static inline int security_prog_load(void)
+{
+	return 0;
+}
+
+static inline int security_post_create(struct bpf_map *map)
+{
+	return 0;
+}
+#endif /* CONFIG_SECURITY */
+#endif /* CONFIG_BPF_SYSCALL */
+
 #ifdef CONFIG_SECURITY
 
 static inline char *alloc_secdata(void)
diff --git a/security/security.c b/security/security.c
index 55b5997e4b72..02272f93a89e 100644
--- a/security/security.c
+++ b/security/security.c
@@ -12,6 +12,7 @@
  *	(at your option) any later version.
  */
 
+#include <linux/bpf.h>
 #include <linux/capability.h>
 #include <linux/dcache.h>
 #include <linux/module.h>
@@ -1708,3 +1709,30 @@ int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule,
 				actx);
 }
 #endif /* CONFIG_AUDIT */
+
+#ifdef CONFIG_BPF_SYSCALL
+int security_map_create(void)
+{
+	return call_int_hook(bpf_map_create, 0);
+}
+
+int security_map_modify(struct bpf_map *map)
+{
+	return call_int_hook(bpf_map_modify, 0, map);
+}
+
+int security_map_read(struct bpf_map *map)
+{
+	return call_int_hook(bpf_map_read, 0, map);
+}
+
+int security_prog_load(void)
+{
+	return call_int_hook(bpf_prog_load, 0);
+}
+
+int security_post_create(struct bpf_map *map)
+{
+	return call_int_hook(bpf_post_create, 0, map);
+}
+#endif /* CONFIG_BPF_SYSCALL */
-- 
2.14.1.581.gf28d330327-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Chenbo Feng]

From: Chenbo Feng <fengc@google.com>

Introduce a pointer into struct bpf_map to hold the security information
about the map. The actual security struct varies based on the security
models implemented. Place the LSM hooks before each of the unrestricted
eBPF operations, the map_update_elem and map_delete_elem operations are
checked by security_map_modify. The map_lookup_elem and map_get_next_key
operations are checked by securtiy_map_read.

Signed-off-by: Chenbo Feng <fengc@google.com>
---
 include/linux/bpf.h  |  3 +++
 kernel/bpf/syscall.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index b69e7a5869ff..ca3e6ff7091d 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -53,6 +53,9 @@ struct bpf_map {
 	struct work_struct work;
 	atomic_t usercnt;
 	struct bpf_map *inner_map_meta;
+#ifdef CONFIG_SECURITY
+	void *security;
+#endif
 };
 
 /* function argument constraints */
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 045646da97cc..b15580bcf3b1 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -279,6 +279,10 @@ static int map_create(union bpf_attr *attr)
 	if (err)
 		return -EINVAL;
 
+	err = security_map_create();
+	if (err)
+		return -EACCES;
+
 	/* find map type and init map: hashtable vs rbtree vs bloom vs ... */
 	map = find_and_alloc_map(attr);
 	if (IS_ERR(map))
@@ -291,6 +295,10 @@ static int map_create(union bpf_attr *attr)
 	if (err)
 		goto free_map_nouncharge;
 
+	err = security_post_create(map);
+	if (err < 0)
+		goto free_map;
+
 	err = bpf_map_alloc_id(map);
 	if (err)
 		goto free_map;
@@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
 	if (IS_ERR(map))
 		return PTR_ERR(map);
 
+	err = security_map_read(map);
+	if (err)
+		return -EACCES;
+
 	key = memdup_user(ukey, map->key_size);
 	if (IS_ERR(key)) {
 		err = PTR_ERR(key);
@@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
 	if (IS_ERR(map))
 		return PTR_ERR(map);
 
+	err = security_map_modify(map);
+	if (err)
+		return -EACCES;
+
 	key = memdup_user(ukey, map->key_size);
 	if (IS_ERR(key)) {
 		err = PTR_ERR(key);
@@ -573,6 +589,10 @@ static int map_delete_elem(union bpf_attr *attr)
 	if (IS_ERR(map))
 		return PTR_ERR(map);
 
+	err = security_map_modify(map);
+	if (err)
+		return -EACCES;
+
 	key = memdup_user(ukey, map->key_size);
 	if (IS_ERR(key)) {
 		err = PTR_ERR(key);
@@ -616,6 +636,10 @@ static int map_get_next_key(union bpf_attr *attr)
 	if (IS_ERR(map))
 		return PTR_ERR(map);
 
+	err = security_map_read(map);
+	if (err)
+		return -EACCES;
+
 	if (ukey) {
 		key = memdup_user(ukey, map->key_size);
 		if (IS_ERR(key)) {
@@ -935,6 +959,10 @@ static int bpf_prog_load(union bpf_attr *attr)
 	if (CHECK_ATTR(BPF_PROG_LOAD))
 		return -EINVAL;
 
+	err = security_prog_load();
+	if (err)
+		return -EACCES;
+
 	if (attr->prog_flags & ~BPF_F_STRICT_ALIGNMENT)
 		return -EINVAL;
 
-- 
2.14.1.581.gf28d330327-goog

[PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Chenbo Feng]

From: Chenbo Feng <fengc@google.com>

Introduce a pointer into struct bpf_map to hold the security information
about the map. The actual security struct varies based on the security
models implemented. Place the LSM hooks before each of the unrestricted
eBPF operations, the map_update_elem and map_delete_elem operations are
checked by security_map_modify. The map_lookup_elem and map_get_next_key
operations are checked by securtiy_map_read.

Signed-off-by: Chenbo Feng <fengc@google.com>
---
 include/linux/bpf.h  |  3 +++
 kernel/bpf/syscall.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index b69e7a5869ff..ca3e6ff7091d 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -53,6 +53,9 @@ struct bpf_map {
 	struct work_struct work;
 	atomic_t usercnt;
 	struct bpf_map *inner_map_meta;
+#ifdef CONFIG_SECURITY
+	void *security;
+#endif
 };
 
 /* function argument constraints */
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 045646da97cc..b15580bcf3b1 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -279,6 +279,10 @@ static int map_create(union bpf_attr *attr)
 	if (err)
 		return -EINVAL;
 
+	err = security_map_create();
+	if (err)
+		return -EACCES;
+
 	/* find map type and init map: hashtable vs rbtree vs bloom vs ... */
 	map = find_and_alloc_map(attr);
 	if (IS_ERR(map))
@@ -291,6 +295,10 @@ static int map_create(union bpf_attr *attr)
 	if (err)
 		goto free_map_nouncharge;
 
+	err = security_post_create(map);
+	if (err < 0)
+		goto free_map;
+
 	err = bpf_map_alloc_id(map);
 	if (err)
 		goto free_map;
@@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
 	if (IS_ERR(map))
 		return PTR_ERR(map);
 
+	err = security_map_read(map);
+	if (err)
+		return -EACCES;
+
 	key = memdup_user(ukey, map->key_size);
 	if (IS_ERR(key)) {
 		err = PTR_ERR(key);
@@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
 	if (IS_ERR(map))
 		return PTR_ERR(map);
 
+	err = security_map_modify(map);
+	if (err)
+		return -EACCES;
+
 	key = memdup_user(ukey, map->key_size);
 	if (IS_ERR(key)) {
 		err = PTR_ERR(key);
@@ -573,6 +589,10 @@ static int map_delete_elem(union bpf_attr *attr)
 	if (IS_ERR(map))
 		return PTR_ERR(map);
 
+	err = security_map_modify(map);
+	if (err)
+		return -EACCES;
+
 	key = memdup_user(ukey, map->key_size);
 	if (IS_ERR(key)) {
 		err = PTR_ERR(key);
@@ -616,6 +636,10 @@ static int map_get_next_key(union bpf_attr *attr)
 	if (IS_ERR(map))
 		return PTR_ERR(map);
 
+	err = security_map_read(map);
+	if (err)
+		return -EACCES;
+
 	if (ukey) {
 		key = memdup_user(ukey, map->key_size);
 		if (IS_ERR(key)) {
@@ -935,6 +959,10 @@ static int bpf_prog_load(union bpf_attr *attr)
 	if (CHECK_ATTR(BPF_PROG_LOAD))
 		return -EINVAL;
 
+	err = security_prog_load();
+	if (err)
+		return -EACCES;
+
 	if (attr->prog_flags & ~BPF_F_STRICT_ALIGNMENT)
 		return -EINVAL;
 
-- 
2.14.1.581.gf28d330327-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] selinux: bpf: Implement the selinux checks for eBPF object

[Chenbo Feng]

From: Chenbo Feng <fengc@google.com>

Introduce 5 new selinux checks for eBPF object related operations. The
check is based on the ownership information of eBPF maps and the
capability of creating eBPF object.

Signed-off-by: Chenbo Feng <fengc@google.com>
---
 security/selinux/hooks.c            | 54 +++++++++++++++++++++++++++++++++++++
 security/selinux/include/classmap.h |  2 ++
 security/selinux/include/objsec.h   |  4 +++
 3 files changed, 60 insertions(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 33fd061305c4..39ad7d9f335d 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -85,6 +85,7 @@
 #include <linux/export.h>
 #include <linux/msg.h>
 #include <linux/shm.h>
+#include <linux/bpf.h>
 
 #include "avc.h"
 #include "objsec.h"
@@ -6245,6 +6246,52 @@ static void selinux_ib_free_security(void *ib_sec)
 }
 #endif
 
+#ifdef CONFIG_BPF_SYSCALL
+static int selinux_bpf_map_create(void)
+{
+	u32 sid = current_sid();
+
+	return avc_has_perm(sid, sid, SECCLASS_BPF, BPF__MAP_CREATE, NULL);
+}
+
+static int selinux_bpf_map_modify(struct bpf_map *map)
+{
+	struct bpf_security_struct *bpfsec = map->security;
+
+	return avc_has_perm(current_sid(), bpfsec->sid, SECCLASS_BPF,
+			    BPF__MAP_MODIFY, NULL);
+}
+
+static int selinux_bpf_map_read(struct bpf_map *map)
+{
+	struct bpf_security_struct *bpfsec = map->security;
+
+	return avc_has_perm(current_sid(), bpfsec->sid, SECCLASS_BPF,
+			    BPF__MAP_READ, NULL);
+}
+
+static int selinux_bpf_prog_load(void)
+{
+	u32 sid = current_sid();
+
+	return avc_has_perm(sid, sid, SECCLASS_BPF, BPF__PROG_LOAD, NULL);
+}
+
+static int selinux_bpf_post_create(struct bpf_map *map)
+{
+	struct bpf_security_struct *bpfsec;
+
+	bpfsec = kzalloc(sizeof(*bpfsec), GFP_KERNEL);
+	if (!bpfsec)
+		return -ENOMEM;
+
+	bpfsec->sid = current_sid();
+	map->security = bpfsec;
+
+	return 0;
+}
+#endif
+
 static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(binder_set_context_mgr, selinux_binder_set_context_mgr),
 	LSM_HOOK_INIT(binder_transaction, selinux_binder_transaction),
@@ -6465,6 +6512,13 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(audit_rule_match, selinux_audit_rule_match),
 	LSM_HOOK_INIT(audit_rule_free, selinux_audit_rule_free),
 #endif
+#ifdef CONFIG_BPF_SYSCALL
+	LSM_HOOK_INIT(bpf_map_create, selinux_bpf_map_create),
+	LSM_HOOK_INIT(bpf_map_modify, selinux_bpf_map_modify),
+	LSM_HOOK_INIT(bpf_map_read, selinux_bpf_map_read),
+	LSM_HOOK_INIT(bpf_prog_load, selinux_bpf_prog_load),
+	LSM_HOOK_INIT(bpf_post_create, selinux_bpf_post_create),
+#endif
 };
 
 static __init int selinux_init(void)
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index b9fe3434b036..83c880fb17b4 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -235,6 +235,8 @@ struct security_class_mapping secclass_map[] = {
 	  { "access", NULL } },
 	{ "infiniband_endport",
 	  { "manage_subnet", NULL } },
+	{ "bpf",
+	  {"map_create", "map_modify", "map_read", "prog_load" } },
 	{ NULL }
   };
 
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 6ebc61e370ff..ba564f662b0d 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -150,6 +150,10 @@ struct pkey_security_struct {
 	u32	sid;	/* SID of pkey */
 };
 
+struct bpf_security_struct {
+	u32 sid;	/*SID of bpf obj creater*/
+};
+
 extern unsigned int selinux_checkreqprot;
 
 #endif /* _SELINUX_OBJSEC_H_ */
-- 
2.14.1.581.gf28d330327-goog

[PATCH 3/3] selinux: bpf: Implement the selinux checks for eBPF object

[Chenbo Feng]

From: Chenbo Feng <fengc@google.com>

Introduce 5 new selinux checks for eBPF object related operations. The
check is based on the ownership information of eBPF maps and the
capability of creating eBPF object.

Signed-off-by: Chenbo Feng <fengc@google.com>
---
 security/selinux/hooks.c            | 54 +++++++++++++++++++++++++++++++++++++
 security/selinux/include/classmap.h |  2 ++
 security/selinux/include/objsec.h   |  4 +++
 3 files changed, 60 insertions(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 33fd061305c4..39ad7d9f335d 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -85,6 +85,7 @@
 #include <linux/export.h>
 #include <linux/msg.h>
 #include <linux/shm.h>
+#include <linux/bpf.h>
 
 #include "avc.h"
 #include "objsec.h"
@@ -6245,6 +6246,52 @@ static void selinux_ib_free_security(void *ib_sec)
 }
 #endif
 
+#ifdef CONFIG_BPF_SYSCALL
+static int selinux_bpf_map_create(void)
+{
+	u32 sid = current_sid();
+
+	return avc_has_perm(sid, sid, SECCLASS_BPF, BPF__MAP_CREATE, NULL);
+}
+
+static int selinux_bpf_map_modify(struct bpf_map *map)
+{
+	struct bpf_security_struct *bpfsec = map->security;
+
+	return avc_has_perm(current_sid(), bpfsec->sid, SECCLASS_BPF,
+			    BPF__MAP_MODIFY, NULL);
+}
+
+static int selinux_bpf_map_read(struct bpf_map *map)
+{
+	struct bpf_security_struct *bpfsec = map->security;
+
+	return avc_has_perm(current_sid(), bpfsec->sid, SECCLASS_BPF,
+			    BPF__MAP_READ, NULL);
+}
+
+static int selinux_bpf_prog_load(void)
+{
+	u32 sid = current_sid();
+
+	return avc_has_perm(sid, sid, SECCLASS_BPF, BPF__PROG_LOAD, NULL);
+}
+
+static int selinux_bpf_post_create(struct bpf_map *map)
+{
+	struct bpf_security_struct *bpfsec;
+
+	bpfsec = kzalloc(sizeof(*bpfsec), GFP_KERNEL);
+	if (!bpfsec)
+		return -ENOMEM;
+
+	bpfsec->sid = current_sid();
+	map->security = bpfsec;
+
+	return 0;
+}
+#endif
+
 static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(binder_set_context_mgr, selinux_binder_set_context_mgr),
 	LSM_HOOK_INIT(binder_transaction, selinux_binder_transaction),
@@ -6465,6 +6512,13 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(audit_rule_match, selinux_audit_rule_match),
 	LSM_HOOK_INIT(audit_rule_free, selinux_audit_rule_free),
 #endif
+#ifdef CONFIG_BPF_SYSCALL
+	LSM_HOOK_INIT(bpf_map_create, selinux_bpf_map_create),
+	LSM_HOOK_INIT(bpf_map_modify, selinux_bpf_map_modify),
+	LSM_HOOK_INIT(bpf_map_read, selinux_bpf_map_read),
+	LSM_HOOK_INIT(bpf_prog_load, selinux_bpf_prog_load),
+	LSM_HOOK_INIT(bpf_post_create, selinux_bpf_post_create),
+#endif
 };
 
 static __init int selinux_init(void)
diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index b9fe3434b036..83c880fb17b4 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -235,6 +235,8 @@ struct security_class_mapping secclass_map[] = {
 	  { "access", NULL } },
 	{ "infiniband_endport",
 	  { "manage_subnet", NULL } },
+	{ "bpf",
+	  {"map_create", "map_modify", "map_read", "prog_load" } },
 	{ NULL }
   };
 
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 6ebc61e370ff..ba564f662b0d 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -150,6 +150,10 @@ struct pkey_security_struct {
 	u32	sid;	/* SID of pkey */
 };
 
+struct bpf_security_struct {
+	u32 sid;	/*SID of bpf obj creater*/
+};
+
 extern unsigned int selinux_checkreqprot;
 
 #endif /* _SELINUX_OBJSEC_H_ */
-- 
2.14.1.581.gf28d330327-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Mimi Zohar]

On Thu, 2017-08-31 at 13:56 -0700, Chenbo Feng wrote:
> From: Chenbo Feng <fengc@google.com>
> 
> Introduce a pointer into struct bpf_map to hold the security information
> about the map. The actual security struct varies based on the security
> models implemented. Place the LSM hooks before each of the unrestricted
> eBPF operations, the map_update_elem and map_delete_elem operations are
> checked by security_map_modify. The map_lookup_elem and map_get_next_key
> operations are checked by securtiy_map_read.
> 
> Signed-off-by: Chenbo Feng <fengc@google.com>
> ---
>  include/linux/bpf.h  |  3 +++
>  kernel/bpf/syscall.c | 28 ++++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index b69e7a5869ff..ca3e6ff7091d 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -53,6 +53,9 @@ struct bpf_map {
>  	struct work_struct work;
>  	atomic_t usercnt;
>  	struct bpf_map *inner_map_meta;
> +#ifdef CONFIG_SECURITY
> +	void *security;
> +#endif
>  };
>  
>  /* function argument constraints */
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 045646da97cc..b15580bcf3b1 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -279,6 +279,10 @@ static int map_create(union bpf_attr *attr)
>  	if (err)
>  		return -EINVAL;
>  
> +	err = security_map_create();
> +	if (err)
> +		return -EACCES;

Any reason not to just return err?

Mimi

> +
>  	/* find map type and init map: hashtable vs rbtree vs bloom vs ... */
>  	map = find_and_alloc_map(attr);
>  	if (IS_ERR(map))
> @@ -291,6 +295,10 @@ static int map_create(union bpf_attr *attr)
>  	if (err)
>  		goto free_map_nouncharge;
>  
> +	err = security_post_create(map);
> +	if (err < 0)
> +		goto free_map;
> +
>  	err = bpf_map_alloc_id(map);
>  	if (err)
>  		goto free_map;
> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
>  	if (IS_ERR(map))
>  		return PTR_ERR(map);
>  
> +	err = security_map_read(map);
> +	if (err)
> +		return -EACCES;
> +
>  	key = memdup_user(ukey, map->key_size);
>  	if (IS_ERR(key)) {
>  		err = PTR_ERR(key);
> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
>  	if (IS_ERR(map))
>  		return PTR_ERR(map);
>  
> +	err = security_map_modify(map);
> +	if (err)
> +		return -EACCES;
> +
>  	key = memdup_user(ukey, map->key_size);
>  	if (IS_ERR(key)) {
>  		err = PTR_ERR(key);
> @@ -573,6 +589,10 @@ static int map_delete_elem(union bpf_attr *attr)
>  	if (IS_ERR(map))
>  		return PTR_ERR(map);
>  
> +	err = security_map_modify(map);
> +	if (err)
> +		return -EACCES;
> +
>  	key = memdup_user(ukey, map->key_size);
>  	if (IS_ERR(key)) {
>  		err = PTR_ERR(key);
> @@ -616,6 +636,10 @@ static int map_get_next_key(union bpf_attr *attr)
>  	if (IS_ERR(map))
>  		return PTR_ERR(map);
>  
> +	err = security_map_read(map);
> +	if (err)
> +		return -EACCES;
> +
>  	if (ukey) {
>  		key = memdup_user(ukey, map->key_size);
>  		if (IS_ERR(key)) {
> @@ -935,6 +959,10 @@ static int bpf_prog_load(union bpf_attr *attr)
>  	if (CHECK_ATTR(BPF_PROG_LOAD))
>  		return -EINVAL;
>  
> +	err = security_prog_load();
> +	if (err)
> +		return -EACCES;
> +
>  	if (attr->prog_flags & ~BPF_F_STRICT_ALIGNMENT)
>  		return -EINVAL;
>  

[PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Mimi Zohar]

On Thu, 2017-08-31 at 13:56 -0700, Chenbo Feng wrote:
> From: Chenbo Feng <fengc@google.com>
> 
> Introduce a pointer into struct bpf_map to hold the security information
> about the map. The actual security struct varies based on the security
> models implemented. Place the LSM hooks before each of the unrestricted
> eBPF operations, the map_update_elem and map_delete_elem operations are
> checked by security_map_modify. The map_lookup_elem and map_get_next_key
> operations are checked by securtiy_map_read.
> 
> Signed-off-by: Chenbo Feng <fengc@google.com>
> ---
>  include/linux/bpf.h  |  3 +++
>  kernel/bpf/syscall.c | 28 ++++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index b69e7a5869ff..ca3e6ff7091d 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -53,6 +53,9 @@ struct bpf_map {
>  	struct work_struct work;
>  	atomic_t usercnt;
>  	struct bpf_map *inner_map_meta;
> +#ifdef CONFIG_SECURITY
> +	void *security;
> +#endif
>  };
>  
>  /* function argument constraints */
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 045646da97cc..b15580bcf3b1 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -279,6 +279,10 @@ static int map_create(union bpf_attr *attr)
>  	if (err)
>  		return -EINVAL;
>  
> +	err = security_map_create();
> +	if (err)
> +		return -EACCES;

Any reason not to just return err?

Mimi

> +
>  	/* find map type and init map: hashtable vs rbtree vs bloom vs ... */
>  	map = find_and_alloc_map(attr);
>  	if (IS_ERR(map))
> @@ -291,6 +295,10 @@ static int map_create(union bpf_attr *attr)
>  	if (err)
>  		goto free_map_nouncharge;
>  
> +	err = security_post_create(map);
> +	if (err < 0)
> +		goto free_map;
> +
>  	err = bpf_map_alloc_id(map);
>  	if (err)
>  		goto free_map;
> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
>  	if (IS_ERR(map))
>  		return PTR_ERR(map);
>  
> +	err = security_map_read(map);
> +	if (err)
> +		return -EACCES;
> +
>  	key = memdup_user(ukey, map->key_size);
>  	if (IS_ERR(key)) {
>  		err = PTR_ERR(key);
> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
>  	if (IS_ERR(map))
>  		return PTR_ERR(map);
>  
> +	err = security_map_modify(map);
> +	if (err)
> +		return -EACCES;
> +
>  	key = memdup_user(ukey, map->key_size);
>  	if (IS_ERR(key)) {
>  		err = PTR_ERR(key);
> @@ -573,6 +589,10 @@ static int map_delete_elem(union bpf_attr *attr)
>  	if (IS_ERR(map))
>  		return PTR_ERR(map);
>  
> +	err = security_map_modify(map);
> +	if (err)
> +		return -EACCES;
> +
>  	key = memdup_user(ukey, map->key_size);
>  	if (IS_ERR(key)) {
>  		err = PTR_ERR(key);
> @@ -616,6 +636,10 @@ static int map_get_next_key(union bpf_attr *attr)
>  	if (IS_ERR(map))
>  		return PTR_ERR(map);
>  
> +	err = security_map_read(map);
> +	if (err)
> +		return -EACCES;
> +
>  	if (ukey) {
>  		key = memdup_user(ukey, map->key_size);
>  		if (IS_ERR(key)) {
> @@ -935,6 +959,10 @@ static int bpf_prog_load(union bpf_attr *attr)
>  	if (CHECK_ATTR(BPF_PROG_LOAD))
>  		return -EINVAL;
>  
> +	err = security_prog_load();
> +	if (err)
> +		return -EACCES;
> +
>  	if (attr->prog_flags & ~BPF_F_STRICT_ALIGNMENT)
>  		return -EINVAL;
>  

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Chenbo Feng]

On Thu, Aug 31, 2017 at 2:17 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Thu, 2017-08-31 at 13:56 -0700, Chenbo Feng wrote:
>> From: Chenbo Feng <fengc@google.com>
>>
>> Introduce a pointer into struct bpf_map to hold the security information
>> about the map. The actual security struct varies based on the security
>> models implemented. Place the LSM hooks before each of the unrestricted
>> eBPF operations, the map_update_elem and map_delete_elem operations are
>> checked by security_map_modify. The map_lookup_elem and map_get_next_key
>> operations are checked by securtiy_map_read.
>>
>> Signed-off-by: Chenbo Feng <fengc@google.com>
>> ---
>>  include/linux/bpf.h  |  3 +++
>>  kernel/bpf/syscall.c | 28 ++++++++++++++++++++++++++++
>>  2 files changed, 31 insertions(+)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index b69e7a5869ff..ca3e6ff7091d 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -53,6 +53,9 @@ struct bpf_map {
>>       struct work_struct work;
>>       atomic_t usercnt;
>>       struct bpf_map *inner_map_meta;
>> +#ifdef CONFIG_SECURITY
>> +     void *security;
>> +#endif
>>  };
>>
>>  /* function argument constraints */
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 045646da97cc..b15580bcf3b1 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -279,6 +279,10 @@ static int map_create(union bpf_attr *attr)
>>       if (err)
>>               return -EINVAL;
>>
>> +     err = security_map_create();
>> +     if (err)
>> +             return -EACCES;
>
> Any reason not to just return err?
>
> Mimi
>
Nope... return err might be better. I will fix this in next version.

Thanks
Chenbo
>> +
>>       /* find map type and init map: hashtable vs rbtree vs bloom vs ... */
>>       map = find_and_alloc_map(attr);
>>       if (IS_ERR(map))
>> @@ -291,6 +295,10 @@ static int map_create(union bpf_attr *attr)
>>       if (err)
>>               goto free_map_nouncharge;
>>
>> +     err = security_post_create(map);
>> +     if (err < 0)
>> +             goto free_map;
>> +
>>       err = bpf_map_alloc_id(map);
>>       if (err)
>>               goto free_map;
>> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
>>       if (IS_ERR(map))
>>               return PTR_ERR(map);
>>
>> +     err = security_map_read(map);
>> +     if (err)
>> +             return -EACCES;
>> +
>>       key = memdup_user(ukey, map->key_size);
>>       if (IS_ERR(key)) {
>>               err = PTR_ERR(key);
>> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
>>       if (IS_ERR(map))
>>               return PTR_ERR(map);
>>
>> +     err = security_map_modify(map);
>> +     if (err)
>> +             return -EACCES;
>> +
>>       key = memdup_user(ukey, map->key_size);
>>       if (IS_ERR(key)) {
>>               err = PTR_ERR(key);
>> @@ -573,6 +589,10 @@ static int map_delete_elem(union bpf_attr *attr)
>>       if (IS_ERR(map))
>>               return PTR_ERR(map);
>>
>> +     err = security_map_modify(map);
>> +     if (err)
>> +             return -EACCES;
>> +
>>       key = memdup_user(ukey, map->key_size);
>>       if (IS_ERR(key)) {
>>               err = PTR_ERR(key);
>> @@ -616,6 +636,10 @@ static int map_get_next_key(union bpf_attr *attr)
>>       if (IS_ERR(map))
>>               return PTR_ERR(map);
>>
>> +     err = security_map_read(map);
>> +     if (err)
>> +             return -EACCES;
>> +
>>       if (ukey) {
>>               key = memdup_user(ukey, map->key_size);
>>               if (IS_ERR(key)) {
>> @@ -935,6 +959,10 @@ static int bpf_prog_load(union bpf_attr *attr)
>>       if (CHECK_ATTR(BPF_PROG_LOAD))
>>               return -EINVAL;
>>
>> +     err = security_prog_load();
>> +     if (err)
>> +             return -EACCES;
>> +
>>       if (attr->prog_flags & ~BPF_F_STRICT_ALIGNMENT)
>>               return -EINVAL;
>>
>

[PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Chenbo Feng]

On Thu, Aug 31, 2017 at 2:17 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Thu, 2017-08-31 at 13:56 -0700, Chenbo Feng wrote:
>> From: Chenbo Feng <fengc@google.com>
>>
>> Introduce a pointer into struct bpf_map to hold the security information
>> about the map. The actual security struct varies based on the security
>> models implemented. Place the LSM hooks before each of the unrestricted
>> eBPF operations, the map_update_elem and map_delete_elem operations are
>> checked by security_map_modify. The map_lookup_elem and map_get_next_key
>> operations are checked by securtiy_map_read.
>>
>> Signed-off-by: Chenbo Feng <fengc@google.com>
>> ---
>>  include/linux/bpf.h  |  3 +++
>>  kernel/bpf/syscall.c | 28 ++++++++++++++++++++++++++++
>>  2 files changed, 31 insertions(+)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index b69e7a5869ff..ca3e6ff7091d 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -53,6 +53,9 @@ struct bpf_map {
>>       struct work_struct work;
>>       atomic_t usercnt;
>>       struct bpf_map *inner_map_meta;
>> +#ifdef CONFIG_SECURITY
>> +     void *security;
>> +#endif
>>  };
>>
>>  /* function argument constraints */
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 045646da97cc..b15580bcf3b1 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -279,6 +279,10 @@ static int map_create(union bpf_attr *attr)
>>       if (err)
>>               return -EINVAL;
>>
>> +     err = security_map_create();
>> +     if (err)
>> +             return -EACCES;
>
> Any reason not to just return err?
>
> Mimi
>
Nope... return err might be better. I will fix this in next version.

Thanks
Chenbo
>> +
>>       /* find map type and init map: hashtable vs rbtree vs bloom vs ... */
>>       map = find_and_alloc_map(attr);
>>       if (IS_ERR(map))
>> @@ -291,6 +295,10 @@ static int map_create(union bpf_attr *attr)
>>       if (err)
>>               goto free_map_nouncharge;
>>
>> +     err = security_post_create(map);
>> +     if (err < 0)
>> +             goto free_map;
>> +
>>       err = bpf_map_alloc_id(map);
>>       if (err)
>>               goto free_map;
>> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
>>       if (IS_ERR(map))
>>               return PTR_ERR(map);
>>
>> +     err = security_map_read(map);
>> +     if (err)
>> +             return -EACCES;
>> +
>>       key = memdup_user(ukey, map->key_size);
>>       if (IS_ERR(key)) {
>>               err = PTR_ERR(key);
>> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
>>       if (IS_ERR(map))
>>               return PTR_ERR(map);
>>
>> +     err = security_map_modify(map);
>> +     if (err)
>> +             return -EACCES;
>> +
>>       key = memdup_user(ukey, map->key_size);
>>       if (IS_ERR(key)) {
>>               err = PTR_ERR(key);
>> @@ -573,6 +589,10 @@ static int map_delete_elem(union bpf_attr *attr)
>>       if (IS_ERR(map))
>>               return PTR_ERR(map);
>>
>> +     err = security_map_modify(map);
>> +     if (err)
>> +             return -EACCES;
>> +
>>       key = memdup_user(ukey, map->key_size);
>>       if (IS_ERR(key)) {
>>               err = PTR_ERR(key);
>> @@ -616,6 +636,10 @@ static int map_get_next_key(union bpf_attr *attr)
>>       if (IS_ERR(map))
>>               return PTR_ERR(map);
>>
>> +     err = security_map_read(map);
>> +     if (err)
>> +             return -EACCES;
>> +
>>       if (ukey) {
>>               key = memdup_user(ukey, map->key_size);
>>               if (IS_ERR(key)) {
>> @@ -935,6 +959,10 @@ static int bpf_prog_load(union bpf_attr *attr)
>>       if (CHECK_ATTR(BPF_PROG_LOAD))
>>               return -EINVAL;
>>
>> +     err = security_prog_load();
>> +     if (err)
>> +             return -EACCES;
>> +
>>       if (attr->prog_flags & ~BPF_F_STRICT_ALIGNMENT)
>>               return -EINVAL;
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Daniel Borkmann]

On 08/31/2017 10:56 PM, Chenbo Feng wrote:
> From: Chenbo Feng <fengc@google.com>
>
> Introduce a pointer into struct bpf_map to hold the security information
> about the map. The actual security struct varies based on the security
> models implemented. Place the LSM hooks before each of the unrestricted
> eBPF operations, the map_update_elem and map_delete_elem operations are
> checked by security_map_modify. The map_lookup_elem and map_get_next_key
> operations are checked by securtiy_map_read.
>
> Signed-off-by: Chenbo Feng <fengc@google.com>

Against which tree is this by the way, net-next? There are
changes here which require a rebase of your set.

> ---
>   include/linux/bpf.h  |  3 +++
>   kernel/bpf/syscall.c | 28 ++++++++++++++++++++++++++++
>   2 files changed, 31 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index b69e7a5869ff..ca3e6ff7091d 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -53,6 +53,9 @@ struct bpf_map {
>   	struct work_struct work;
>   	atomic_t usercnt;
>   	struct bpf_map *inner_map_meta;
> +#ifdef CONFIG_SECURITY
> +	void *security;
> +#endif
>   };
>
>   /* function argument constraints */
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 045646da97cc..b15580bcf3b1 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -279,6 +279,10 @@ static int map_create(union bpf_attr *attr)
>   	if (err)
>   		return -EINVAL;
>
> +	err = security_map_create();

Seems a bit limited to me, don't you want to be able to
also differentiate between different map types? Same goes
for security_prog_load() wrt prog types, no?

> +	if (err)
> +		return -EACCES;
> +
>   	/* find map type and init map: hashtable vs rbtree vs bloom vs ... */
>   	map = find_and_alloc_map(attr);
>   	if (IS_ERR(map))
> @@ -291,6 +295,10 @@ static int map_create(union bpf_attr *attr)
>   	if (err)
>   		goto free_map_nouncharge;
>
> +	err = security_post_create(map);
> +	if (err < 0)
> +		goto free_map;
> +

So the hook you implement in patch 3/3 does:

+static int selinux_bpf_post_create(struct bpf_map *map)
+{
+	struct bpf_security_struct *bpfsec;
+
+	bpfsec = kzalloc(sizeof(*bpfsec), GFP_KERNEL);
+	if (!bpfsec)
+		return -ENOMEM;
+
+	bpfsec->sid = current_sid();
+	map->security = bpfsec;
+
+	return 0;
+}

Where do you kfree() bpfsec when the map gets released
normally or in error path?

>   	err = bpf_map_alloc_id(map);
>   	if (err)
>   		goto free_map;
> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
>   	if (IS_ERR(map))
>   		return PTR_ERR(map);
>
> +	err = security_map_read(map);
> +	if (err)
> +		return -EACCES;

How about actually dropping ref?

> +
>   	key = memdup_user(ukey, map->key_size);
>   	if (IS_ERR(key)) {
>   		err = PTR_ERR(key);
> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
>   	if (IS_ERR(map))
>   		return PTR_ERR(map);
>
> +	err = security_map_modify(map);
> +	if (err)
> +		return -EACCES;

Ditto ...

>   	key = memdup_user(ukey, map->key_size);
>   	if (IS_ERR(key)) {
>   		err = PTR_ERR(key);
> @@ -573,6 +589,10 @@ static int map_delete_elem(union bpf_attr *attr)
>   	if (IS_ERR(map))
>   		return PTR_ERR(map);
>
> +	err = security_map_modify(map);
> +	if (err)
> +		return -EACCES;

Ditto ...

>   	key = memdup_user(ukey, map->key_size);
>   	if (IS_ERR(key)) {
>   		err = PTR_ERR(key);
> @@ -616,6 +636,10 @@ static int map_get_next_key(union bpf_attr *attr)
>   	if (IS_ERR(map))
>   		return PTR_ERR(map);
>
> +	err = security_map_read(map);
> +	if (err)
> +		return -EACCES;

And once again here ... :(

>   	if (ukey) {
>   		key = memdup_user(ukey, map->key_size);
>   		if (IS_ERR(key)) {
> @@ -935,6 +959,10 @@ static int bpf_prog_load(union bpf_attr *attr)
>   	if (CHECK_ATTR(BPF_PROG_LOAD))
>   		return -EINVAL;
>
> +	err = security_prog_load();
> +	if (err)
> +		return -EACCES;
> +
>   	if (attr->prog_flags & ~BPF_F_STRICT_ALIGNMENT)
>   		return -EINVAL;
>
>

[PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Daniel Borkmann]

On 08/31/2017 10:56 PM, Chenbo Feng wrote:
> From: Chenbo Feng <fengc@google.com>
>
> Introduce a pointer into struct bpf_map to hold the security information
> about the map. The actual security struct varies based on the security
> models implemented. Place the LSM hooks before each of the unrestricted
> eBPF operations, the map_update_elem and map_delete_elem operations are
> checked by security_map_modify. The map_lookup_elem and map_get_next_key
> operations are checked by securtiy_map_read.
>
> Signed-off-by: Chenbo Feng <fengc@google.com>

Against which tree is this by the way, net-next? There are
changes here which require a rebase of your set.

> ---
>   include/linux/bpf.h  |  3 +++
>   kernel/bpf/syscall.c | 28 ++++++++++++++++++++++++++++
>   2 files changed, 31 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index b69e7a5869ff..ca3e6ff7091d 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -53,6 +53,9 @@ struct bpf_map {
>   	struct work_struct work;
>   	atomic_t usercnt;
>   	struct bpf_map *inner_map_meta;
> +#ifdef CONFIG_SECURITY
> +	void *security;
> +#endif
>   };
>
>   /* function argument constraints */
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 045646da97cc..b15580bcf3b1 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -279,6 +279,10 @@ static int map_create(union bpf_attr *attr)
>   	if (err)
>   		return -EINVAL;
>
> +	err = security_map_create();

Seems a bit limited to me, don't you want to be able to
also differentiate between different map types? Same goes
for security_prog_load() wrt prog types, no?

> +	if (err)
> +		return -EACCES;
> +
>   	/* find map type and init map: hashtable vs rbtree vs bloom vs ... */
>   	map = find_and_alloc_map(attr);
>   	if (IS_ERR(map))
> @@ -291,6 +295,10 @@ static int map_create(union bpf_attr *attr)
>   	if (err)
>   		goto free_map_nouncharge;
>
> +	err = security_post_create(map);
> +	if (err < 0)
> +		goto free_map;
> +

So the hook you implement in patch 3/3 does:

+static int selinux_bpf_post_create(struct bpf_map *map)
+{
+	struct bpf_security_struct *bpfsec;
+
+	bpfsec = kzalloc(sizeof(*bpfsec), GFP_KERNEL);
+	if (!bpfsec)
+		return -ENOMEM;
+
+	bpfsec->sid = current_sid();
+	map->security = bpfsec;
+
+	return 0;
+}

Where do you kfree() bpfsec when the map gets released
normally or in error path?

>   	err = bpf_map_alloc_id(map);
>   	if (err)
>   		goto free_map;
> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
>   	if (IS_ERR(map))
>   		return PTR_ERR(map);
>
> +	err = security_map_read(map);
> +	if (err)
> +		return -EACCES;

How about actually dropping ref?

> +
>   	key = memdup_user(ukey, map->key_size);
>   	if (IS_ERR(key)) {
>   		err = PTR_ERR(key);
> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
>   	if (IS_ERR(map))
>   		return PTR_ERR(map);
>
> +	err = security_map_modify(map);
> +	if (err)
> +		return -EACCES;

Ditto ...

>   	key = memdup_user(ukey, map->key_size);
>   	if (IS_ERR(key)) {
>   		err = PTR_ERR(key);
> @@ -573,6 +589,10 @@ static int map_delete_elem(union bpf_attr *attr)
>   	if (IS_ERR(map))
>   		return PTR_ERR(map);
>
> +	err = security_map_modify(map);
> +	if (err)
> +		return -EACCES;

Ditto ...

>   	key = memdup_user(ukey, map->key_size);
>   	if (IS_ERR(key)) {
>   		err = PTR_ERR(key);
> @@ -616,6 +636,10 @@ static int map_get_next_key(union bpf_attr *attr)
>   	if (IS_ERR(map))
>   		return PTR_ERR(map);
>
> +	err = security_map_read(map);
> +	if (err)
> +		return -EACCES;

And once again here ... :(

>   	if (ukey) {
>   		key = memdup_user(ukey, map->key_size);
>   		if (IS_ERR(key)) {
> @@ -935,6 +959,10 @@ static int bpf_prog_load(union bpf_attr *attr)
>   	if (CHECK_ATTR(BPF_PROG_LOAD))
>   		return -EINVAL;
>
> +	err = security_prog_load();
> +	if (err)
> +		return -EACCES;
> +
>   	if (attr->prog_flags & ~BPF_F_STRICT_ALIGNMENT)
>   		return -EINVAL;
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Chenbo Feng]

On Thu, Aug 31, 2017 at 3:38 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 08/31/2017 10:56 PM, Chenbo Feng wrote:
>>
>> From: Chenbo Feng <fengc@google.com>
>>
>> Introduce a pointer into struct bpf_map to hold the security information
>> about the map. The actual security struct varies based on the security
>> models implemented. Place the LSM hooks before each of the unrestricted
>> eBPF operations, the map_update_elem and map_delete_elem operations are
>> checked by security_map_modify. The map_lookup_elem and map_get_next_key
>> operations are checked by securtiy_map_read.
>>
>> Signed-off-by: Chenbo Feng <fengc@google.com>
>
>
> Against which tree is this by the way, net-next? There are
> changes here which require a rebase of your set.
>

This patch series is rebased on security subsystem since patch 1/3 is
a patch for
security system I assume. But I am not sure where this specific patch
should go in.
If I should submit this one to net-next, I will rebase it against
net-next and submit again.
>> ---
>>   include/linux/bpf.h  |  3 +++
>>   kernel/bpf/syscall.c | 28 ++++++++++++++++++++++++++++
>>   2 files changed, 31 insertions(+)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index b69e7a5869ff..ca3e6ff7091d 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -53,6 +53,9 @@ struct bpf_map {
>>         struct work_struct work;
>>         atomic_t usercnt;
>>         struct bpf_map *inner_map_meta;
>> +#ifdef CONFIG_SECURITY
>> +       void *security;
>> +#endif
>>   };
>>
>>   /* function argument constraints */
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 045646da97cc..b15580bcf3b1 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -279,6 +279,10 @@ static int map_create(union bpf_attr *attr)
>>         if (err)
>>                 return -EINVAL;
>>
>> +       err = security_map_create();
>
>
> Seems a bit limited to me, don't you want to be able to
> also differentiate between different map types? Same goes
> for security_prog_load() wrt prog types, no?
>
I don't want to introduce extra complexity into it if not necessary.
so I only included the
thing needed for the selinux implementation for now.  But I agree that the map
and program type information could be useful when people developing more complex
security checks. I will add a union bpf_attr *attr pointer into the
security hook functions
for future needs.
>> +       if (err)
>> +               return -EACCES;
>> +
>>         /* find map type and init map: hashtable vs rbtree vs bloom vs ...
>> */
>>         map = find_and_alloc_map(attr);
>>         if (IS_ERR(map))
>> @@ -291,6 +295,10 @@ static int map_create(union bpf_attr *attr)
>>         if (err)
>>                 goto free_map_nouncharge;
>>
>> +       err = security_post_create(map);
>> +       if (err < 0)
>> +               goto free_map;
>> +
>
>
> So the hook you implement in patch 3/3 does:
>
> +static int selinux_bpf_post_create(struct bpf_map *map)
> +{
> +       struct bpf_security_struct *bpfsec;
> +
> +       bpfsec = kzalloc(sizeof(*bpfsec), GFP_KERNEL);
> +       if (!bpfsec)
> +               return -ENOMEM;
> +
> +       bpfsec->sid = current_sid();
> +       map->security = bpfsec;
> +
> +       return 0;
> +}
>
> Where do you kfree() bpfsec when the map gets released
> normally or in error path?
>
Will add it in next version. Thanks for point it out.
>>         err = bpf_map_alloc_id(map);
>>         if (err)
>>                 goto free_map;
>> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
>>         if (IS_ERR(map))
>>                 return PTR_ERR(map);
>>
>> +       err = security_map_read(map);
>> +       if (err)
>> +               return -EACCES;
>
>
> How about actually dropping ref?
>
May bad, thanks again.
>> +
>>         key = memdup_user(ukey, map->key_size);
>>         if (IS_ERR(key)) {
>>                 err = PTR_ERR(key);
>> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
>>         if (IS_ERR(map))
>>                 return PTR_ERR(map);
>>
>> +       err = security_map_modify(map);
>> +       if (err)
>> +               return -EACCES;
>
>
> Ditto ...
>
>>         key = memdup_user(ukey, map->key_size);
>>         if (IS_ERR(key)) {
>>                 err = PTR_ERR(key);
>> @@ -573,6 +589,10 @@ static int map_delete_elem(union bpf_attr *attr)
>>         if (IS_ERR(map))
>>                 return PTR_ERR(map);
>>
>> +       err = security_map_modify(map);
>> +       if (err)
>> +               return -EACCES;
>
>
> Ditto ...
>
>>         key = memdup_user(ukey, map->key_size);
>>         if (IS_ERR(key)) {
>>                 err = PTR_ERR(key);
>> @@ -616,6 +636,10 @@ static int map_get_next_key(union bpf_attr *attr)
>>         if (IS_ERR(map))
>>                 return PTR_ERR(map);
>>
>> +       err = security_map_read(map);
>> +       if (err)
>> +               return -EACCES;
>
>
> And once again here ... :(
>
>
>>         if (ukey) {
>>                 key = memdup_user(ukey, map->key_size);
>>                 if (IS_ERR(key)) {
>> @@ -935,6 +959,10 @@ static int bpf_prog_load(union bpf_attr *attr)
>>         if (CHECK_ATTR(BPF_PROG_LOAD))
>>                 return -EINVAL;
>>
>> +       err = security_prog_load();
>> +       if (err)
>> +               return -EACCES;
>> +
>>         if (attr->prog_flags & ~BPF_F_STRICT_ALIGNMENT)
>>                 return -EINVAL;
>>
>>
>

[PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Chenbo Feng]

On Thu, Aug 31, 2017 at 3:38 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 08/31/2017 10:56 PM, Chenbo Feng wrote:
>>
>> From: Chenbo Feng <fengc@google.com>
>>
>> Introduce a pointer into struct bpf_map to hold the security information
>> about the map. The actual security struct varies based on the security
>> models implemented. Place the LSM hooks before each of the unrestricted
>> eBPF operations, the map_update_elem and map_delete_elem operations are
>> checked by security_map_modify. The map_lookup_elem and map_get_next_key
>> operations are checked by securtiy_map_read.
>>
>> Signed-off-by: Chenbo Feng <fengc@google.com>
>
>
> Against which tree is this by the way, net-next? There are
> changes here which require a rebase of your set.
>

This patch series is rebased on security subsystem since patch 1/3 is
a patch for
security system I assume. But I am not sure where this specific patch
should go in.
If I should submit this one to net-next, I will rebase it against
net-next and submit again.
>> ---
>>   include/linux/bpf.h  |  3 +++
>>   kernel/bpf/syscall.c | 28 ++++++++++++++++++++++++++++
>>   2 files changed, 31 insertions(+)
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index b69e7a5869ff..ca3e6ff7091d 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -53,6 +53,9 @@ struct bpf_map {
>>         struct work_struct work;
>>         atomic_t usercnt;
>>         struct bpf_map *inner_map_meta;
>> +#ifdef CONFIG_SECURITY
>> +       void *security;
>> +#endif
>>   };
>>
>>   /* function argument constraints */
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 045646da97cc..b15580bcf3b1 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -279,6 +279,10 @@ static int map_create(union bpf_attr *attr)
>>         if (err)
>>                 return -EINVAL;
>>
>> +       err = security_map_create();
>
>
> Seems a bit limited to me, don't you want to be able to
> also differentiate between different map types? Same goes
> for security_prog_load() wrt prog types, no?
>
I don't want to introduce extra complexity into it if not necessary.
so I only included the
thing needed for the selinux implementation for now.  But I agree that the map
and program type information could be useful when people developing more complex
security checks. I will add a union bpf_attr *attr pointer into the
security hook functions
for future needs.
>> +       if (err)
>> +               return -EACCES;
>> +
>>         /* find map type and init map: hashtable vs rbtree vs bloom vs ...
>> */
>>         map = find_and_alloc_map(attr);
>>         if (IS_ERR(map))
>> @@ -291,6 +295,10 @@ static int map_create(union bpf_attr *attr)
>>         if (err)
>>                 goto free_map_nouncharge;
>>
>> +       err = security_post_create(map);
>> +       if (err < 0)
>> +               goto free_map;
>> +
>
>
> So the hook you implement in patch 3/3 does:
>
> +static int selinux_bpf_post_create(struct bpf_map *map)
> +{
> +       struct bpf_security_struct *bpfsec;
> +
> +       bpfsec = kzalloc(sizeof(*bpfsec), GFP_KERNEL);
> +       if (!bpfsec)
> +               return -ENOMEM;
> +
> +       bpfsec->sid = current_sid();
> +       map->security = bpfsec;
> +
> +       return 0;
> +}
>
> Where do you kfree() bpfsec when the map gets released
> normally or in error path?
>
Will add it in next version. Thanks for point it out.
>>         err = bpf_map_alloc_id(map);
>>         if (err)
>>                 goto free_map;
>> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
>>         if (IS_ERR(map))
>>                 return PTR_ERR(map);
>>
>> +       err = security_map_read(map);
>> +       if (err)
>> +               return -EACCES;
>
>
> How about actually dropping ref?
>
May bad, thanks again.
>> +
>>         key = memdup_user(ukey, map->key_size);
>>         if (IS_ERR(key)) {
>>                 err = PTR_ERR(key);
>> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
>>         if (IS_ERR(map))
>>                 return PTR_ERR(map);
>>
>> +       err = security_map_modify(map);
>> +       if (err)
>> +               return -EACCES;
>
>
> Ditto ...
>
>>         key = memdup_user(ukey, map->key_size);
>>         if (IS_ERR(key)) {
>>                 err = PTR_ERR(key);
>> @@ -573,6 +589,10 @@ static int map_delete_elem(union bpf_attr *attr)
>>         if (IS_ERR(map))
>>                 return PTR_ERR(map);
>>
>> +       err = security_map_modify(map);
>> +       if (err)
>> +               return -EACCES;
>
>
> Ditto ...
>
>>         key = memdup_user(ukey, map->key_size);
>>         if (IS_ERR(key)) {
>>                 err = PTR_ERR(key);
>> @@ -616,6 +636,10 @@ static int map_get_next_key(union bpf_attr *attr)
>>         if (IS_ERR(map))
>>                 return PTR_ERR(map);
>>
>> +       err = security_map_read(map);
>> +       if (err)
>> +               return -EACCES;
>
>
> And once again here ... :(
>
>
>>         if (ukey) {
>>                 key = memdup_user(ukey, map->key_size);
>>                 if (IS_ERR(key)) {
>> @@ -935,6 +959,10 @@ static int bpf_prog_load(union bpf_attr *attr)
>>         if (CHECK_ATTR(BPF_PROG_LOAD))
>>                 return -EINVAL;
>>
>> +       err = security_prog_load();
>> +       if (err)
>> +               return -EACCES;
>> +
>>         if (attr->prog_flags & ~BPF_F_STRICT_ALIGNMENT)
>>                 return -EINVAL;
>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Alexei Starovoitov]

On Thu, Aug 31, 2017 at 01:56:34PM -0700, Chenbo Feng wrote:
> From: Chenbo Feng <fengc@google.com>
> 
> Introduce a pointer into struct bpf_map to hold the security information
> about the map. The actual security struct varies based on the security
> models implemented. Place the LSM hooks before each of the unrestricted
> eBPF operations, the map_update_elem and map_delete_elem operations are
> checked by security_map_modify. The map_lookup_elem and map_get_next_key
> operations are checked by securtiy_map_read.
> 
> Signed-off-by: Chenbo Feng <fengc@google.com>

...

> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
>  	if (IS_ERR(map))
>  		return PTR_ERR(map);
>  
> +	err = security_map_read(map);
> +	if (err)
> +		return -EACCES;
> +
>  	key = memdup_user(ukey, map->key_size);
>  	if (IS_ERR(key)) {
>  		err = PTR_ERR(key);
> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
>  	if (IS_ERR(map))
>  		return PTR_ERR(map);
>  
> +	err = security_map_modify(map);

I don't feel these extra hooks are really thought through.
With such hook you'll disallow map_update for given map. That's it.
The key/values etc won't be used in such security decision.
In such case you don't need such hooks in update/lookup at all.
Only in map_creation and object_get calls where FD can be received.
In other words I suggest to follow standard unix practices:
Do permissions checks in open() and allow read/write() if FD is valid.
Same here. Do permission checks in prog_load/map_create/obj_pin/get
and that will be enough to jail bpf subsystem.
bpf cmds that need to be fast (like lookup and update) should not
have security hooks.

[PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Alexei Starovoitov]

On Thu, Aug 31, 2017 at 01:56:34PM -0700, Chenbo Feng wrote:
> From: Chenbo Feng <fengc@google.com>
> 
> Introduce a pointer into struct bpf_map to hold the security information
> about the map. The actual security struct varies based on the security
> models implemented. Place the LSM hooks before each of the unrestricted
> eBPF operations, the map_update_elem and map_delete_elem operations are
> checked by security_map_modify. The map_lookup_elem and map_get_next_key
> operations are checked by securtiy_map_read.
> 
> Signed-off-by: Chenbo Feng <fengc@google.com>

...

> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
>  	if (IS_ERR(map))
>  		return PTR_ERR(map);
>  
> +	err = security_map_read(map);
> +	if (err)
> +		return -EACCES;
> +
>  	key = memdup_user(ukey, map->key_size);
>  	if (IS_ERR(key)) {
>  		err = PTR_ERR(key);
> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
>  	if (IS_ERR(map))
>  		return PTR_ERR(map);
>  
> +	err = security_map_modify(map);

I don't feel these extra hooks are really thought through.
With such hook you'll disallow map_update for given map. That's it.
The key/values etc won't be used in such security decision.
In such case you don't need such hooks in update/lookup at all.
Only in map_creation and object_get calls where FD can be received.
In other words I suggest to follow standard unix practices:
Do permissions checks in open() and allow read/write() if FD is valid.
Same here. Do permission checks in prog_load/map_create/obj_pin/get
and that will be enough to jail bpf subsystem.
bpf cmds that need to be fast (like lookup and update) should not
have security hooks.

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Jeffrey Vander Stoep]

On Thu, Aug 31, 2017 at 7:05 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Thu, Aug 31, 2017 at 01:56:34PM -0700, Chenbo Feng wrote:
>> From: Chenbo Feng <fengc@google.com>
>>
>> Introduce a pointer into struct bpf_map to hold the security information
>> about the map. The actual security struct varies based on the security
>> models implemented. Place the LSM hooks before each of the unrestricted
>> eBPF operations, the map_update_elem and map_delete_elem operations are
>> checked by security_map_modify. The map_lookup_elem and map_get_next_key
>> operations are checked by securtiy_map_read.
>>
>> Signed-off-by: Chenbo Feng <fengc@google.com>
>
> ...
>
>> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
>>       if (IS_ERR(map))
>>               return PTR_ERR(map);
>>
>> +     err = security_map_read(map);
>> +     if (err)
>> +             return -EACCES;
>> +
>>       key = memdup_user(ukey, map->key_size);
>>       if (IS_ERR(key)) {
>>               err = PTR_ERR(key);
>> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
>>       if (IS_ERR(map))
>>               return PTR_ERR(map);
>>
>> +     err = security_map_modify(map);
>
> I don't feel these extra hooks are really thought through.
> With such hook you'll disallow map_update for given map. That's it.
> The key/values etc won't be used in such security decision.
> In such case you don't need such hooks in update/lookup at all.
> Only in map_creation and object_get calls where FD can be received.
> In other words I suggest to follow standard unix practices:
> Do permissions checks in open() and allow read/write() if FD is valid.
> Same here. Do permission checks in prog_load/map_create/obj_pin/get
> and that will be enough to jail bpf subsystem.
> bpf cmds that need to be fast (like lookup and update) should not
> have security hooks.
>

I do think we want to distinguish between read/write (or read/modify)
for these objects. Essentially, we want to implement the example
described in patch 0/3 where eBPF objects can be passed to less
privileged processes which can read, but not modify the map. What
would be the best way to do this? Add a mode field to the bpf_map
struct?

[PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Jeffrey Vander Stoep]

On Thu, Aug 31, 2017 at 7:05 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Thu, Aug 31, 2017 at 01:56:34PM -0700, Chenbo Feng wrote:
>> From: Chenbo Feng <fengc@google.com>
>>
>> Introduce a pointer into struct bpf_map to hold the security information
>> about the map. The actual security struct varies based on the security
>> models implemented. Place the LSM hooks before each of the unrestricted
>> eBPF operations, the map_update_elem and map_delete_elem operations are
>> checked by security_map_modify. The map_lookup_elem and map_get_next_key
>> operations are checked by securtiy_map_read.
>>
>> Signed-off-by: Chenbo Feng <fengc@google.com>
>
> ...
>
>> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
>>       if (IS_ERR(map))
>>               return PTR_ERR(map);
>>
>> +     err = security_map_read(map);
>> +     if (err)
>> +             return -EACCES;
>> +
>>       key = memdup_user(ukey, map->key_size);
>>       if (IS_ERR(key)) {
>>               err = PTR_ERR(key);
>> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
>>       if (IS_ERR(map))
>>               return PTR_ERR(map);
>>
>> +     err = security_map_modify(map);
>
> I don't feel these extra hooks are really thought through.
> With such hook you'll disallow map_update for given map. That's it.
> The key/values etc won't be used in such security decision.
> In such case you don't need such hooks in update/lookup at all.
> Only in map_creation and object_get calls where FD can be received.
> In other words I suggest to follow standard unix practices:
> Do permissions checks in open() and allow read/write() if FD is valid.
> Same here. Do permission checks in prog_load/map_create/obj_pin/get
> and that will be enough to jail bpf subsystem.
> bpf cmds that need to be fast (like lookup and update) should not
> have security hooks.
>

I do think we want to distinguish between read/write (or read/modify)
for these objects. Essentially, we want to implement the example
described in patch 0/3 where eBPF objects can be passed to less
privileged processes which can read, but not modify the map. What
would be the best way to do this? Add a mode field to the bpf_map
struct?
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] security: bpf: Add eBPF LSM hooks to security module

[Stephen Smalley]

On Thu, 2017-08-31 at 13:56 -0700, Chenbo Feng wrote:
> From: Chenbo Feng <fengc@google.com>
> 
> Introduce 5 LSM hooks to provide finer granularity controls on eBPF
> related operations including create eBPF maps, modify and read eBPF
> maps
> content and load eBPF programs to the kernel. Hooks use the new
> security
> pointer inside the eBPF map struct to store the owner's security
> information and the different security modules can perform different
> checks based on the information stored inside the security field.
> 
> Signed-off-by: Chenbo Feng <fengc@google.com>
> ---
>  include/linux/lsm_hooks.h | 41
> +++++++++++++++++++++++++++++++++++++++++
>  include/linux/security.h  | 36 ++++++++++++++++++++++++++++++++++++
>  security/security.c       | 28 ++++++++++++++++++++++++++++
>  3 files changed, 105 insertions(+)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index ce02f76a6188..3aaf9a08a983 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1353,6 +1353,32 @@
>   *	@inode we wish to get the security context of.
>   *	@ctx is a pointer in which to place the allocated security
> context.
>   *	@ctxlen points to the place to put the length of @ctx.
> + *
> + * Security hooks for using the eBPF maps and programs
> functionalities through
> + * eBPF syscalls.
> + *
> + * @bpf_map_create:
> + *	Check permissions prior to creating a new bpf map.
> + *	Return 0 if the permission is granted.
> + *
> + * @bpf_map_modify:
> + *	Check permission prior to insert, update and delete map
> content.
> + *	@map pointer to the struct bpf_map that contains map
> information.
> + *	Return 0 if the permission is granted.
> + *
> + * @bpf_map_read:
> + *	Check permission prior to read a bpf map content.
> + *	@map pointer to the struct bpf_map that contains map
> information.
> + *	Return 0 if the permission is granted.
> + *
> + * @bpf_prog_load:
> + *	Check permission prior to load eBPF program.
> + *	Return 0 if the permission is granted.
> + *
> + * @bpf_post_create:
> + *	Initialize the bpf object security field inside struct
> bpf_maps and
> + *	it is used for future security checks.
> + *
>   */
>  union security_list_options {
>  	int (*binder_set_context_mgr)(struct task_struct *mgr);
> @@ -1685,6 +1711,14 @@ union security_list_options {
>  				struct audit_context *actx);
>  	void (*audit_rule_free)(void *lsmrule);
>  #endif /* CONFIG_AUDIT */
> +
> +#ifdef CONFIG_BPF_SYSCALL
> +	int (*bpf_map_create)(void);
> +	int (*bpf_map_read)(struct bpf_map *map);
> +	int (*bpf_map_modify)(struct bpf_map *map);
> +	int (*bpf_prog_load)(void);
> +	int (*bpf_post_create)(struct bpf_map *map);
> +#endif /* CONFIG_BPF_SYSCALL */
>  };
>  
>  struct security_hook_heads {
> @@ -1905,6 +1939,13 @@ struct security_hook_heads {
>  	struct list_head audit_rule_match;
>  	struct list_head audit_rule_free;
>  #endif /* CONFIG_AUDIT */
> +#ifdef CONFIG_BPF_SYSCALL
> +	struct list_head bpf_map_create;
> +	struct list_head bpf_map_read;
> +	struct list_head bpf_map_modify;
> +	struct list_head bpf_prog_load;
> +	struct list_head bpf_post_create;
> +#endif /* CONFIG_BPF_SYSCALL */
>  } __randomize_layout;
>  
>  /*
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 458e24bea2d4..0656a4f74d14 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -31,6 +31,7 @@
>  #include <linux/string.h>
>  #include <linux/mm.h>
>  #include <linux/fs.h>
> +#include <linux/bpf.h>
>  
>  struct linux_binprm;
>  struct cred;
> @@ -1735,6 +1736,41 @@ static inline void securityfs_remove(struct
> dentry *dentry)
>  
>  #endif
>  
> +#ifdef CONFIG_BPF_SYSCALL
> +#ifdef CONFIG_SECURITY
> +int security_map_create(void);
> +int security_map_modify(struct bpf_map *map);
> +int security_map_read(struct bpf_map *map);
> +int security_prog_load(void);
> +int security_post_create(struct bpf_map *map);
> +#else
> +static inline int security_map_create(void)
> +{
> +	return 0;
> +}
> +
> +static inline int security_map_read(struct bpf_map *map)
> +{
> +	return 0;
> +}
> +
> +static inline int security_map_modify(struct bpf_map *map)
> +{
> +	return 0;
> +}
> +
> +static inline int security_prog_load(void)
> +{
> +	return 0;
> +}
> +
> +static inline int security_post_create(struct bpf_map *map)
> +{
> +	return 0;
> +}
> +#endif /* CONFIG_SECURITY */
> +#endif /* CONFIG_BPF_SYSCALL */

These should be named consistently with the ones in lsm_hooks.h and
should unambiguously indicate that these are hooks for bpf
objects/operations, i.e. security_bpf_map_create(),
security_bpf_map_read(), etc.

Do you need this level of granularity?

Could you coalesce the map_create() and post_map_create() hooks into
one hook and just unwind the create in that case?

Why do you label bpf maps but not bpf progs?  Should we be controlling
the ability to attach/detach a bpf prog (partly controlled by
CAP_NET_ADMIN, but also somewhat broad in scope and doesn't allow
control based on who created the prog)?

Should there be a top-level security_bpf_use() hook and permission
check that limits ability to use bpf() at all?

> +
>  #ifdef CONFIG_SECURITY
>  
>  static inline char *alloc_secdata(void)
> diff --git a/security/security.c b/security/security.c
> index 55b5997e4b72..02272f93a89e 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -12,6 +12,7 @@
>   *	(at your option) any later version.
>   */
>  
> +#include <linux/bpf.h>
>  #include <linux/capability.h>
>  #include <linux/dcache.h>
>  #include <linux/module.h>
> @@ -1708,3 +1709,30 @@ int security_audit_rule_match(u32 secid, u32
> field, u32 op, void *lsmrule,
>  				actx);
>  }
>  #endif /* CONFIG_AUDIT */
> +
> +#ifdef CONFIG_BPF_SYSCALL
> +int security_map_create(void)
> +{
> +	return call_int_hook(bpf_map_create, 0);
> +}
> +
> +int security_map_modify(struct bpf_map *map)
> +{
> +	return call_int_hook(bpf_map_modify, 0, map);
> +}
> +
> +int security_map_read(struct bpf_map *map)
> +{
> +	return call_int_hook(bpf_map_read, 0, map);
> +}
> +
> +int security_prog_load(void)
> +{
> +	return call_int_hook(bpf_prog_load, 0);
> +}
> +
> +int security_post_create(struct bpf_map *map)
> +{
> +	return call_int_hook(bpf_post_create, 0, map);
> +}
> +#endif /* CONFIG_BPF_SYSCALL */

[PATCH 1/3] security: bpf: Add eBPF LSM hooks to security module

[Stephen Smalley]

On Thu, 2017-08-31 at 13:56 -0700, Chenbo Feng wrote:
> From: Chenbo Feng <fengc@google.com>
> 
> Introduce 5 LSM hooks to provide finer granularity controls on eBPF
> related operations including create eBPF maps, modify and read eBPF
> maps
> content and load eBPF programs to the kernel. Hooks use the new
> security
> pointer inside the eBPF map struct to store the owner's security
> information and the different security modules can perform different
> checks based on the information stored inside the security field.
> 
> Signed-off-by: Chenbo Feng <fengc@google.com>
> ---
> ?include/linux/lsm_hooks.h | 41
> +++++++++++++++++++++++++++++++++++++++++
> ?include/linux/security.h??| 36 ++++++++++++++++++++++++++++++++++++
> ?security/security.c???????| 28 ++++++++++++++++++++++++++++
> ?3 files changed, 105 insertions(+)
> 
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index ce02f76a6188..3aaf9a08a983 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1353,6 +1353,32 @@
> ? *	@inode we wish to get the security context of.
> ? *	@ctx is a pointer in which to place the allocated security
> context.
> ? *	@ctxlen points to the place to put the length of @ctx.
> + *
> + * Security hooks for using the eBPF maps and programs
> functionalities through
> + * eBPF syscalls.
> + *
> + * @bpf_map_create:
> + *	Check permissions prior to creating a new bpf map.
> + *	Return 0 if the permission is granted.
> + *
> + * @bpf_map_modify:
> + *	Check permission prior to insert, update and delete map
> content.
> + *	@map pointer to the struct bpf_map that contains map
> information.
> + *	Return 0 if the permission is granted.
> + *
> + * @bpf_map_read:
> + *	Check permission prior to read a bpf map content.
> + *	@map pointer to the struct bpf_map that contains map
> information.
> + *	Return 0 if the permission is granted.
> + *
> + * @bpf_prog_load:
> + *	Check permission prior to load eBPF program.
> + *	Return 0 if the permission is granted.
> + *
> + * @bpf_post_create:
> + *	Initialize the bpf object security field inside struct
> bpf_maps and
> + *	it is used for future security checks.
> + *
> ? */
> ?union security_list_options {
> ?	int (*binder_set_context_mgr)(struct task_struct *mgr);
> @@ -1685,6 +1711,14 @@ union security_list_options {
> ?				struct audit_context *actx);
> ?	void (*audit_rule_free)(void *lsmrule);
> ?#endif /* CONFIG_AUDIT */
> +
> +#ifdef CONFIG_BPF_SYSCALL
> +	int (*bpf_map_create)(void);
> +	int (*bpf_map_read)(struct bpf_map *map);
> +	int (*bpf_map_modify)(struct bpf_map *map);
> +	int (*bpf_prog_load)(void);
> +	int (*bpf_post_create)(struct bpf_map *map);
> +#endif /* CONFIG_BPF_SYSCALL */
> ?};
> ?
> ?struct security_hook_heads {
> @@ -1905,6 +1939,13 @@ struct security_hook_heads {
> ?	struct list_head audit_rule_match;
> ?	struct list_head audit_rule_free;
> ?#endif /* CONFIG_AUDIT */
> +#ifdef CONFIG_BPF_SYSCALL
> +	struct list_head bpf_map_create;
> +	struct list_head bpf_map_read;
> +	struct list_head bpf_map_modify;
> +	struct list_head bpf_prog_load;
> +	struct list_head bpf_post_create;
> +#endif /* CONFIG_BPF_SYSCALL */
> ?} __randomize_layout;
> ?
> ?/*
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 458e24bea2d4..0656a4f74d14 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -31,6 +31,7 @@
> ?#include <linux/string.h>
> ?#include <linux/mm.h>
> ?#include <linux/fs.h>
> +#include <linux/bpf.h>
> ?
> ?struct linux_binprm;
> ?struct cred;
> @@ -1735,6 +1736,41 @@ static inline void securityfs_remove(struct
> dentry *dentry)
> ?
> ?#endif
> ?
> +#ifdef CONFIG_BPF_SYSCALL
> +#ifdef CONFIG_SECURITY
> +int security_map_create(void);
> +int security_map_modify(struct bpf_map *map);
> +int security_map_read(struct bpf_map *map);
> +int security_prog_load(void);
> +int security_post_create(struct bpf_map *map);
> +#else
> +static inline int security_map_create(void)
> +{
> +	return 0;
> +}
> +
> +static inline int security_map_read(struct bpf_map *map)
> +{
> +	return 0;
> +}
> +
> +static inline int security_map_modify(struct bpf_map *map)
> +{
> +	return 0;
> +}
> +
> +static inline int security_prog_load(void)
> +{
> +	return 0;
> +}
> +
> +static inline int security_post_create(struct bpf_map *map)
> +{
> +	return 0;
> +}
> +#endif /* CONFIG_SECURITY */
> +#endif /* CONFIG_BPF_SYSCALL */

These should be named consistently with the ones in lsm_hooks.h and
should unambiguously indicate that these are hooks for bpf
objects/operations, i.e. security_bpf_map_create(),
security_bpf_map_read(), etc.

Do you need this level of granularity?

Could you coalesce the map_create() and post_map_create() hooks into
one hook and just unwind the create in that case?

Why do you label bpf maps but not bpf progs?  Should we be controlling
the ability to attach/detach a bpf prog (partly controlled by
CAP_NET_ADMIN, but also somewhat broad in scope and doesn't allow
control based on who created the prog)?

Should there be a top-level security_bpf_use() hook and permission
check that limits ability to use bpf() at all?

> +
> ?#ifdef CONFIG_SECURITY
> ?
> ?static inline char *alloc_secdata(void)
> diff --git a/security/security.c b/security/security.c
> index 55b5997e4b72..02272f93a89e 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -12,6 +12,7 @@
> ? *	(at your option) any later version.
> ? */
> ?
> +#include <linux/bpf.h>
> ?#include <linux/capability.h>
> ?#include <linux/dcache.h>
> ?#include <linux/module.h>
> @@ -1708,3 +1709,30 @@ int security_audit_rule_match(u32 secid, u32
> field, u32 op, void *lsmrule,
> ?				actx);
> ?}
> ?#endif /* CONFIG_AUDIT */
> +
> +#ifdef CONFIG_BPF_SYSCALL
> +int security_map_create(void)
> +{
> +	return call_int_hook(bpf_map_create, 0);
> +}
> +
> +int security_map_modify(struct bpf_map *map)
> +{
> +	return call_int_hook(bpf_map_modify, 0, map);
> +}
> +
> +int security_map_read(struct bpf_map *map)
> +{
> +	return call_int_hook(bpf_map_read, 0, map);
> +}
> +
> +int security_prog_load(void)
> +{
> +	return call_int_hook(bpf_prog_load, 0);
> +}
> +
> +int security_post_create(struct bpf_map *map)
> +{
> +	return call_int_hook(bpf_post_create, 0, map);
> +}
> +#endif /* CONFIG_BPF_SYSCALL */
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Chenbo Feng]

On Thu, Aug 31, 2017 at 7:05 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Thu, Aug 31, 2017 at 01:56:34PM -0700, Chenbo Feng wrote:
>> From: Chenbo Feng <fengc@google.com>
>>
>> Introduce a pointer into struct bpf_map to hold the security information
>> about the map. The actual security struct varies based on the security
>> models implemented. Place the LSM hooks before each of the unrestricted
>> eBPF operations, the map_update_elem and map_delete_elem operations are
>> checked by security_map_modify. The map_lookup_elem and map_get_next_key
>> operations are checked by securtiy_map_read.
>>
>> Signed-off-by: Chenbo Feng <fengc@google.com>
>
> ...
>
>> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
>>       if (IS_ERR(map))
>>               return PTR_ERR(map);
>>
>> +     err = security_map_read(map);
>> +     if (err)
>> +             return -EACCES;
>> +
>>       key = memdup_user(ukey, map->key_size);
>>       if (IS_ERR(key)) {
>>               err = PTR_ERR(key);
>> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
>>       if (IS_ERR(map))
>>               return PTR_ERR(map);
>>
>> +     err = security_map_modify(map);
>
> I don't feel these extra hooks are really thought through.
> With such hook you'll disallow map_update for given map. That's it.
> The key/values etc won't be used in such security decision.
> In such case you don't need such hooks in update/lookup at all.
> Only in map_creation and object_get calls where FD can be received.
> In other words I suggest to follow standard unix practices:
> Do permissions checks in open() and allow read/write() if FD is valid.
> Same here. Do permission checks in prog_load/map_create/obj_pin/get
> and that will be enough to jail bpf subsystem.
> bpf cmds that need to be fast (like lookup and update) should not
> have security hooks.
>
Thanks for pointing out this. I agree we should only do checks on
creating and passing
the object from one processes to another. And if we can still
distinguish the read/write operation
when obtaining the file fd, that would be great. But that may require
us to add a new mode
field into bpf_map struct and change the attribute struct when doing
the bpf syscall. How do you
think about this approach? Or we can do simple checks for
bpf_obj_create and bpf_obj_use when
creating the object and passing the object respectively but this
solution cannot distinguish map modify and
read.

[PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Chenbo Feng]

On Thu, Aug 31, 2017 at 7:05 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Thu, Aug 31, 2017 at 01:56:34PM -0700, Chenbo Feng wrote:
>> From: Chenbo Feng <fengc@google.com>
>>
>> Introduce a pointer into struct bpf_map to hold the security information
>> about the map. The actual security struct varies based on the security
>> models implemented. Place the LSM hooks before each of the unrestricted
>> eBPF operations, the map_update_elem and map_delete_elem operations are
>> checked by security_map_modify. The map_lookup_elem and map_get_next_key
>> operations are checked by securtiy_map_read.
>>
>> Signed-off-by: Chenbo Feng <fengc@google.com>
>
> ...
>
>> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
>>       if (IS_ERR(map))
>>               return PTR_ERR(map);
>>
>> +     err = security_map_read(map);
>> +     if (err)
>> +             return -EACCES;
>> +
>>       key = memdup_user(ukey, map->key_size);
>>       if (IS_ERR(key)) {
>>               err = PTR_ERR(key);
>> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
>>       if (IS_ERR(map))
>>               return PTR_ERR(map);
>>
>> +     err = security_map_modify(map);
>
> I don't feel these extra hooks are really thought through.
> With such hook you'll disallow map_update for given map. That's it.
> The key/values etc won't be used in such security decision.
> In such case you don't need such hooks in update/lookup at all.
> Only in map_creation and object_get calls where FD can be received.
> In other words I suggest to follow standard unix practices:
> Do permissions checks in open() and allow read/write() if FD is valid.
> Same here. Do permission checks in prog_load/map_create/obj_pin/get
> and that will be enough to jail bpf subsystem.
> bpf cmds that need to be fast (like lookup and update) should not
> have security hooks.
>
Thanks for pointing out this. I agree we should only do checks on
creating and passing
the object from one processes to another. And if we can still
distinguish the read/write operation
when obtaining the file fd, that would be great. But that may require
us to add a new mode
field into bpf_map struct and change the attribute struct when doing
the bpf syscall. How do you
think about this approach? Or we can do simple checks for
bpf_obj_create and bpf_obj_use when
creating the object and passing the object respectively but this
solution cannot distinguish map modify and
read.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] security: bpf: Add eBPF LSM hooks to security module

[Chenbo Feng]

On Fri, Sep 1, 2017 at 5:50 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Thu, 2017-08-31 at 13:56 -0700, Chenbo Feng wrote:
>> From: Chenbo Feng <fengc@google.com>
>>
>> Introduce 5 LSM hooks to provide finer granularity controls on eBPF
>> related operations including create eBPF maps, modify and read eBPF
>> maps
>> content and load eBPF programs to the kernel. Hooks use the new
>> security
>> pointer inside the eBPF map struct to store the owner's security
>> information and the different security modules can perform different
>> checks based on the information stored inside the security field.
>>
>> Signed-off-by: Chenbo Feng <fengc@google.com>
>> ---
>>  include/linux/lsm_hooks.h | 41
>> +++++++++++++++++++++++++++++++++++++++++
>>  include/linux/security.h  | 36 ++++++++++++++++++++++++++++++++++++
>>  security/security.c       | 28 ++++++++++++++++++++++++++++
>>  3 files changed, 105 insertions(+)
>>
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index ce02f76a6188..3aaf9a08a983 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -1353,6 +1353,32 @@
>>   *   @inode we wish to get the security context of.
>>   *   @ctx is a pointer in which to place the allocated security
>> context.
>>   *   @ctxlen points to the place to put the length of @ctx.
>> + *
>> + * Security hooks for using the eBPF maps and programs
>> functionalities through
>> + * eBPF syscalls.
>> + *
>> + * @bpf_map_create:
>> + *   Check permissions prior to creating a new bpf map.
>> + *   Return 0 if the permission is granted.
>> + *
>> + * @bpf_map_modify:
>> + *   Check permission prior to insert, update and delete map
>> content.
>> + *   @map pointer to the struct bpf_map that contains map
>> information.
>> + *   Return 0 if the permission is granted.
>> + *
>> + * @bpf_map_read:
>> + *   Check permission prior to read a bpf map content.
>> + *   @map pointer to the struct bpf_map that contains map
>> information.
>> + *   Return 0 if the permission is granted.
>> + *
>> + * @bpf_prog_load:
>> + *   Check permission prior to load eBPF program.
>> + *   Return 0 if the permission is granted.
>> + *
>> + * @bpf_post_create:
>> + *   Initialize the bpf object security field inside struct
>> bpf_maps and
>> + *   it is used for future security checks.
>> + *
>>   */
>>  union security_list_options {
>>       int (*binder_set_context_mgr)(struct task_struct *mgr);
>> @@ -1685,6 +1711,14 @@ union security_list_options {
>>                               struct audit_context *actx);
>>       void (*audit_rule_free)(void *lsmrule);
>>  #endif /* CONFIG_AUDIT */
>> +
>> +#ifdef CONFIG_BPF_SYSCALL
>> +     int (*bpf_map_create)(void);
>> +     int (*bpf_map_read)(struct bpf_map *map);
>> +     int (*bpf_map_modify)(struct bpf_map *map);
>> +     int (*bpf_prog_load)(void);
>> +     int (*bpf_post_create)(struct bpf_map *map);
>> +#endif /* CONFIG_BPF_SYSCALL */
>>  };
>>
>>  struct security_hook_heads {
>> @@ -1905,6 +1939,13 @@ struct security_hook_heads {
>>       struct list_head audit_rule_match;
>>       struct list_head audit_rule_free;
>>  #endif /* CONFIG_AUDIT */
>> +#ifdef CONFIG_BPF_SYSCALL
>> +     struct list_head bpf_map_create;
>> +     struct list_head bpf_map_read;
>> +     struct list_head bpf_map_modify;
>> +     struct list_head bpf_prog_load;
>> +     struct list_head bpf_post_create;
>> +#endif /* CONFIG_BPF_SYSCALL */
>>  } __randomize_layout;
>>
>>  /*
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 458e24bea2d4..0656a4f74d14 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -31,6 +31,7 @@
>>  #include <linux/string.h>
>>  #include <linux/mm.h>
>>  #include <linux/fs.h>
>> +#include <linux/bpf.h>
>>
>>  struct linux_binprm;
>>  struct cred;
>> @@ -1735,6 +1736,41 @@ static inline void securityfs_remove(struct
>> dentry *dentry)
>>
>>  #endif
>>
>> +#ifdef CONFIG_BPF_SYSCALL
>> +#ifdef CONFIG_SECURITY
>> +int security_map_create(void);
>> +int security_map_modify(struct bpf_map *map);
>> +int security_map_read(struct bpf_map *map);
>> +int security_prog_load(void);
>> +int security_post_create(struct bpf_map *map);
>> +#else
>> +static inline int security_map_create(void)
>> +{
>> +     return 0;
>> +}
>> +
>> +static inline int security_map_read(struct bpf_map *map)
>> +{
>> +     return 0;
>> +}
>> +
>> +static inline int security_map_modify(struct bpf_map *map)
>> +{
>> +     return 0;
>> +}
>> +
>> +static inline int security_prog_load(void)
>> +{
>> +     return 0;
>> +}
>> +
>> +static inline int security_post_create(struct bpf_map *map)
>> +{
>> +     return 0;
>> +}
>> +#endif /* CONFIG_SECURITY */
>> +#endif /* CONFIG_BPF_SYSCALL */
>
> These should be named consistently with the ones in lsm_hooks.h and
> should unambiguously indicate that these are hooks for bpf
> objects/operations, i.e. security_bpf_map_create(),
> security_bpf_map_read(), etc.
>
Thanks for pointing out, will fix this.
> Do you need this level of granularity?
>
The cover letter of this patch series described a possible use cases of
these lsm hooks and this level of granularity would be ideal to reach that
goal. We can also implement two hooks such as bpf_obj_create and
bpf_obj_use to restrict the creation and using when get the bpf fd from
kernel. But that will be less powerful and flexible.
> Could you coalesce the map_create() and post_map_create() hooks into
> one hook and just unwind the create in that case?
>
Okay, I will take a look on how to fix this.
> Why do you label bpf maps but not bpf progs?  Should we be controlling
> the ability to attach/detach a bpf prog (partly controlled by
> CAP_NET_ADMIN, but also somewhat broad in scope and doesn't allow
> control based on who created the prog)?
>
> Should there be a top-level security_bpf_use() hook and permission
> check that limits ability to use bpf() at all?
>
This could be useful but having additional lsm hooks check when reading
and write to eBPF maps may cause performance issue. Instead maybe we
could have a hook for creating eBPF object and retrieve object fd to restrict
the access.
>> +
>>  #ifdef CONFIG_SECURITY
>>
>>  static inline char *alloc_secdata(void)
>> diff --git a/security/security.c b/security/security.c
>> index 55b5997e4b72..02272f93a89e 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -12,6 +12,7 @@
>>   *   (at your option) any later version.
>>   */
>>
>> +#include <linux/bpf.h>
>>  #include <linux/capability.h>
>>  #include <linux/dcache.h>
>>  #include <linux/module.h>
>> @@ -1708,3 +1709,30 @@ int security_audit_rule_match(u32 secid, u32
>> field, u32 op, void *lsmrule,
>>                               actx);
>>  }
>>  #endif /* CONFIG_AUDIT */
>> +
>> +#ifdef CONFIG_BPF_SYSCALL
>> +int security_map_create(void)
>> +{
>> +     return call_int_hook(bpf_map_create, 0);
>> +}
>> +
>> +int security_map_modify(struct bpf_map *map)
>> +{
>> +     return call_int_hook(bpf_map_modify, 0, map);
>> +}
>> +
>> +int security_map_read(struct bpf_map *map)
>> +{
>> +     return call_int_hook(bpf_map_read, 0, map);
>> +}
>> +
>> +int security_prog_load(void)
>> +{
>> +     return call_int_hook(bpf_prog_load, 0);
>> +}
>> +
>> +int security_post_create(struct bpf_map *map)
>> +{
>> +     return call_int_hook(bpf_post_create, 0, map);
>> +}
>> +#endif /* CONFIG_BPF_SYSCALL */

[PATCH 1/3] security: bpf: Add eBPF LSM hooks to security module

[Chenbo Feng]

On Fri, Sep 1, 2017 at 5:50 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Thu, 2017-08-31 at 13:56 -0700, Chenbo Feng wrote:
>> From: Chenbo Feng <fengc@google.com>
>>
>> Introduce 5 LSM hooks to provide finer granularity controls on eBPF
>> related operations including create eBPF maps, modify and read eBPF
>> maps
>> content and load eBPF programs to the kernel. Hooks use the new
>> security
>> pointer inside the eBPF map struct to store the owner's security
>> information and the different security modules can perform different
>> checks based on the information stored inside the security field.
>>
>> Signed-off-by: Chenbo Feng <fengc@google.com>
>> ---
>>  include/linux/lsm_hooks.h | 41
>> +++++++++++++++++++++++++++++++++++++++++
>>  include/linux/security.h  | 36 ++++++++++++++++++++++++++++++++++++
>>  security/security.c       | 28 ++++++++++++++++++++++++++++
>>  3 files changed, 105 insertions(+)
>>
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index ce02f76a6188..3aaf9a08a983 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -1353,6 +1353,32 @@
>>   *   @inode we wish to get the security context of.
>>   *   @ctx is a pointer in which to place the allocated security
>> context.
>>   *   @ctxlen points to the place to put the length of @ctx.
>> + *
>> + * Security hooks for using the eBPF maps and programs
>> functionalities through
>> + * eBPF syscalls.
>> + *
>> + * @bpf_map_create:
>> + *   Check permissions prior to creating a new bpf map.
>> + *   Return 0 if the permission is granted.
>> + *
>> + * @bpf_map_modify:
>> + *   Check permission prior to insert, update and delete map
>> content.
>> + *   @map pointer to the struct bpf_map that contains map
>> information.
>> + *   Return 0 if the permission is granted.
>> + *
>> + * @bpf_map_read:
>> + *   Check permission prior to read a bpf map content.
>> + *   @map pointer to the struct bpf_map that contains map
>> information.
>> + *   Return 0 if the permission is granted.
>> + *
>> + * @bpf_prog_load:
>> + *   Check permission prior to load eBPF program.
>> + *   Return 0 if the permission is granted.
>> + *
>> + * @bpf_post_create:
>> + *   Initialize the bpf object security field inside struct
>> bpf_maps and
>> + *   it is used for future security checks.
>> + *
>>   */
>>  union security_list_options {
>>       int (*binder_set_context_mgr)(struct task_struct *mgr);
>> @@ -1685,6 +1711,14 @@ union security_list_options {
>>                               struct audit_context *actx);
>>       void (*audit_rule_free)(void *lsmrule);
>>  #endif /* CONFIG_AUDIT */
>> +
>> +#ifdef CONFIG_BPF_SYSCALL
>> +     int (*bpf_map_create)(void);
>> +     int (*bpf_map_read)(struct bpf_map *map);
>> +     int (*bpf_map_modify)(struct bpf_map *map);
>> +     int (*bpf_prog_load)(void);
>> +     int (*bpf_post_create)(struct bpf_map *map);
>> +#endif /* CONFIG_BPF_SYSCALL */
>>  };
>>
>>  struct security_hook_heads {
>> @@ -1905,6 +1939,13 @@ struct security_hook_heads {
>>       struct list_head audit_rule_match;
>>       struct list_head audit_rule_free;
>>  #endif /* CONFIG_AUDIT */
>> +#ifdef CONFIG_BPF_SYSCALL
>> +     struct list_head bpf_map_create;
>> +     struct list_head bpf_map_read;
>> +     struct list_head bpf_map_modify;
>> +     struct list_head bpf_prog_load;
>> +     struct list_head bpf_post_create;
>> +#endif /* CONFIG_BPF_SYSCALL */
>>  } __randomize_layout;
>>
>>  /*
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 458e24bea2d4..0656a4f74d14 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -31,6 +31,7 @@
>>  #include <linux/string.h>
>>  #include <linux/mm.h>
>>  #include <linux/fs.h>
>> +#include <linux/bpf.h>
>>
>>  struct linux_binprm;
>>  struct cred;
>> @@ -1735,6 +1736,41 @@ static inline void securityfs_remove(struct
>> dentry *dentry)
>>
>>  #endif
>>
>> +#ifdef CONFIG_BPF_SYSCALL
>> +#ifdef CONFIG_SECURITY
>> +int security_map_create(void);
>> +int security_map_modify(struct bpf_map *map);
>> +int security_map_read(struct bpf_map *map);
>> +int security_prog_load(void);
>> +int security_post_create(struct bpf_map *map);
>> +#else
>> +static inline int security_map_create(void)
>> +{
>> +     return 0;
>> +}
>> +
>> +static inline int security_map_read(struct bpf_map *map)
>> +{
>> +     return 0;
>> +}
>> +
>> +static inline int security_map_modify(struct bpf_map *map)
>> +{
>> +     return 0;
>> +}
>> +
>> +static inline int security_prog_load(void)
>> +{
>> +     return 0;
>> +}
>> +
>> +static inline int security_post_create(struct bpf_map *map)
>> +{
>> +     return 0;
>> +}
>> +#endif /* CONFIG_SECURITY */
>> +#endif /* CONFIG_BPF_SYSCALL */
>
> These should be named consistently with the ones in lsm_hooks.h and
> should unambiguously indicate that these are hooks for bpf
> objects/operations, i.e. security_bpf_map_create(),
> security_bpf_map_read(), etc.
>
Thanks for pointing out, will fix this.
> Do you need this level of granularity?
>
The cover letter of this patch series described a possible use cases of
these lsm hooks and this level of granularity would be ideal to reach that
goal. We can also implement two hooks such as bpf_obj_create and
bpf_obj_use to restrict the creation and using when get the bpf fd from
kernel. But that will be less powerful and flexible.
> Could you coalesce the map_create() and post_map_create() hooks into
> one hook and just unwind the create in that case?
>
Okay, I will take a look on how to fix this.
> Why do you label bpf maps but not bpf progs?  Should we be controlling
> the ability to attach/detach a bpf prog (partly controlled by
> CAP_NET_ADMIN, but also somewhat broad in scope and doesn't allow
> control based on who created the prog)?
>
> Should there be a top-level security_bpf_use() hook and permission
> check that limits ability to use bpf() at all?
>
This could be useful but having additional lsm hooks check when reading
and write to eBPF maps may cause performance issue. Instead maybe we
could have a hook for creating eBPF object and retrieve object fd to restrict
the access.
>> +
>>  #ifdef CONFIG_SECURITY
>>
>>  static inline char *alloc_secdata(void)
>> diff --git a/security/security.c b/security/security.c
>> index 55b5997e4b72..02272f93a89e 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -12,6 +12,7 @@
>>   *   (at your option) any later version.
>>   */
>>
>> +#include <linux/bpf.h>
>>  #include <linux/capability.h>
>>  #include <linux/dcache.h>
>>  #include <linux/module.h>
>> @@ -1708,3 +1709,30 @@ int security_audit_rule_match(u32 secid, u32
>> field, u32 op, void *lsmrule,
>>                               actx);
>>  }
>>  #endif /* CONFIG_AUDIT */
>> +
>> +#ifdef CONFIG_BPF_SYSCALL
>> +int security_map_create(void)
>> +{
>> +     return call_int_hook(bpf_map_create, 0);
>> +}
>> +
>> +int security_map_modify(struct bpf_map *map)
>> +{
>> +     return call_int_hook(bpf_map_modify, 0, map);
>> +}
>> +
>> +int security_map_read(struct bpf_map *map)
>> +{
>> +     return call_int_hook(bpf_map_read, 0, map);
>> +}
>> +
>> +int security_prog_load(void)
>> +{
>> +     return call_int_hook(bpf_prog_load, 0);
>> +}
>> +
>> +int security_post_create(struct bpf_map *map)
>> +{
>> +     return call_int_hook(bpf_post_create, 0, map);
>> +}
>> +#endif /* CONFIG_BPF_SYSCALL */
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Alexei Starovoitov]

On Tue, Sep 05, 2017 at 02:59:38PM -0700, Chenbo Feng wrote:
> On Thu, Aug 31, 2017 at 7:05 PM, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> > On Thu, Aug 31, 2017 at 01:56:34PM -0700, Chenbo Feng wrote:
> >> From: Chenbo Feng <fengc@google.com>
> >>
> >> Introduce a pointer into struct bpf_map to hold the security information
> >> about the map. The actual security struct varies based on the security
> >> models implemented. Place the LSM hooks before each of the unrestricted
> >> eBPF operations, the map_update_elem and map_delete_elem operations are
> >> checked by security_map_modify. The map_lookup_elem and map_get_next_key
> >> operations are checked by securtiy_map_read.
> >>
> >> Signed-off-by: Chenbo Feng <fengc@google.com>
> >
> > ...
> >
> >> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
> >>       if (IS_ERR(map))
> >>               return PTR_ERR(map);
> >>
> >> +     err = security_map_read(map);
> >> +     if (err)
> >> +             return -EACCES;
> >> +
> >>       key = memdup_user(ukey, map->key_size);
> >>       if (IS_ERR(key)) {
> >>               err = PTR_ERR(key);
> >> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
> >>       if (IS_ERR(map))
> >>               return PTR_ERR(map);
> >>
> >> +     err = security_map_modify(map);
> >
> > I don't feel these extra hooks are really thought through.
> > With such hook you'll disallow map_update for given map. That's it.
> > The key/values etc won't be used in such security decision.
> > In such case you don't need such hooks in update/lookup at all.
> > Only in map_creation and object_get calls where FD can be received.
> > In other words I suggest to follow standard unix practices:
> > Do permissions checks in open() and allow read/write() if FD is valid.
> > Same here. Do permission checks in prog_load/map_create/obj_pin/get
> > and that will be enough to jail bpf subsystem.
> > bpf cmds that need to be fast (like lookup and update) should not
> > have security hooks.
> >
> Thanks for pointing out this. I agree we should only do checks on
> creating and passing
> the object from one processes to another. And if we can still
> distinguish the read/write operation
> when obtaining the file fd, that would be great. But that may require
> us to add a new mode
> field into bpf_map struct and change the attribute struct when doing
> the bpf syscall. How do you
> think about this approach? Or we can do simple checks for
> bpf_obj_create and bpf_obj_use when
> creating the object and passing the object respectively but this
> solution cannot distinguish map modify and
> read.

iirc the idea to have read only maps was brought up in the past
(unfortunately no one took a stab at implementing it), but imo
that's better then relying on lsm to provide such restriction
and more secure, since even if you disable map_update via lsm,
the user can craft a program just to udpate the map from it.
For bpffs we already test for inode_permission(inode, MAY_WRITE);
during BPF_OBJ_GET command and we can extend this facility further.
Also we can allow dropping 'write' permissions from the map
(internally implemented via flag in struct bpf_map), so
update/delete operations won't be allowed.
This flag will be checked by syscall during map_update/delete
and by the verifier if such read-only map is used by the program
being loaded, so map_update/helpers won't be allowed in
such program.
Would also be good to support read-only programs as well
(progs that are read-only from kernel state point of view)
They won't be able to modify packets, maps, etc.

[PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map

[Alexei Starovoitov]

On Tue, Sep 05, 2017 at 02:59:38PM -0700, Chenbo Feng wrote:
> On Thu, Aug 31, 2017 at 7:05 PM, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> > On Thu, Aug 31, 2017 at 01:56:34PM -0700, Chenbo Feng wrote:
> >> From: Chenbo Feng <fengc@google.com>
> >>
> >> Introduce a pointer into struct bpf_map to hold the security information
> >> about the map. The actual security struct varies based on the security
> >> models implemented. Place the LSM hooks before each of the unrestricted
> >> eBPF operations, the map_update_elem and map_delete_elem operations are
> >> checked by security_map_modify. The map_lookup_elem and map_get_next_key
> >> operations are checked by securtiy_map_read.
> >>
> >> Signed-off-by: Chenbo Feng <fengc@google.com>
> >
> > ...
> >
> >> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
> >>       if (IS_ERR(map))
> >>               return PTR_ERR(map);
> >>
> >> +     err = security_map_read(map);
> >> +     if (err)
> >> +             return -EACCES;
> >> +
> >>       key = memdup_user(ukey, map->key_size);
> >>       if (IS_ERR(key)) {
> >>               err = PTR_ERR(key);
> >> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
> >>       if (IS_ERR(map))
> >>               return PTR_ERR(map);
> >>
> >> +     err = security_map_modify(map);
> >
> > I don't feel these extra hooks are really thought through.
> > With such hook you'll disallow map_update for given map. That's it.
> > The key/values etc won't be used in such security decision.
> > In such case you don't need such hooks in update/lookup at all.
> > Only in map_creation and object_get calls where FD can be received.
> > In other words I suggest to follow standard unix practices:
> > Do permissions checks in open() and allow read/write() if FD is valid.
> > Same here. Do permission checks in prog_load/map_create/obj_pin/get
> > and that will be enough to jail bpf subsystem.
> > bpf cmds that need to be fast (like lookup and update) should not
> > have security hooks.
> >
> Thanks for pointing out this. I agree we should only do checks on
> creating and passing
> the object from one processes to another. And if we can still
> distinguish the read/write operation
> when obtaining the file fd, that would be great. But that may require
> us to add a new mode
> field into bpf_map struct and change the attribute struct when doing
> the bpf syscall. How do you
> think about this approach? Or we can do simple checks for
> bpf_obj_create and bpf_obj_use when
> creating the object and passing the object respectively but this
> solution cannot distinguish map modify and
> read.

iirc the idea to have read only maps was brought up in the past
(unfortunately no one took a stab at implementing it), but imo
that's better then relying on lsm to provide such restriction
and more secure, since even if you disable map_update via lsm,
the user can craft a program just to udpate the map from it.
For bpffs we already test for inode_permission(inode, MAY_WRITE);
during BPF_OBJ_GET command and we can extend this facility further.
Also we can allow dropping 'write' permissions from the map
(internally implemented via flag in struct bpf_map), so
update/delete operations won't be allowed.
This flag will be checked by syscall during map_update/delete
and by the verifier if such read-only map is used by the program
being loaded, so map_update/helpers won't be allowed in
such program.
Would also be good to support read-only programs as well
(progs that are read-only from kernel state point of view)
They won't be able to modify packets, maps, etc.

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] security: bpf: Add eBPF LSM hooks to security module

[Stephen Smalley]

On Tue, 2017-09-05 at 15:24 -0700, Chenbo Feng via Selinux wrote:
> On Fri, Sep 1, 2017 at 5:50 AM, Stephen Smalley <sds@tycho.nsa.gov>
> wrote:
> > On Thu, 2017-08-31 at 13:56 -0700, Chenbo Feng wrote:
> > > From: Chenbo Feng <fengc@google.com>
> > > 
> > > Introduce 5 LSM hooks to provide finer granularity controls on
> > > eBPF
> > > related operations including create eBPF maps, modify and read
> > > eBPF
> > > maps
> > > content and load eBPF programs to the kernel. Hooks use the new
> > > security
> > > pointer inside the eBPF map struct to store the owner's security
> > > information and the different security modules can perform
> > > different
> > > checks based on the information stored inside the security field.
> > > 
> > > Signed-off-by: Chenbo Feng <fengc@google.com>
> > > ---
> > >  include/linux/lsm_hooks.h | 41
> > > +++++++++++++++++++++++++++++++++++++++++
> > >  include/linux/security.h  | 36
> > > ++++++++++++++++++++++++++++++++++++
> > >  security/security.c       | 28 ++++++++++++++++++++++++++++
> > >  3 files changed, 105 insertions(+)
> > > 
> > > diff --git a/include/linux/lsm_hooks.h
> > > b/include/linux/lsm_hooks.h
> > > index ce02f76a6188..3aaf9a08a983 100644
> > > --- a/include/linux/lsm_hooks.h
> > > +++ b/include/linux/lsm_hooks.h
> > > @@ -1353,6 +1353,32 @@
> > >   *   @inode we wish to get the security context of.
> > >   *   @ctx is a pointer in which to place the allocated security
> > > context.
> > >   *   @ctxlen points to the place to put the length of @ctx.
> > > + *
> > > + * Security hooks for using the eBPF maps and programs
> > > functionalities through
> > > + * eBPF syscalls.
> > > + *
> > > + * @bpf_map_create:
> > > + *   Check permissions prior to creating a new bpf map.
> > > + *   Return 0 if the permission is granted.
> > > + *
> > > + * @bpf_map_modify:
> > > + *   Check permission prior to insert, update and delete map
> > > content.
> > > + *   @map pointer to the struct bpf_map that contains map
> > > information.
> > > + *   Return 0 if the permission is granted.
> > > + *
> > > + * @bpf_map_read:
> > > + *   Check permission prior to read a bpf map content.
> > > + *   @map pointer to the struct bpf_map that contains map
> > > information.
> > > + *   Return 0 if the permission is granted.
> > > + *
> > > + * @bpf_prog_load:
> > > + *   Check permission prior to load eBPF program.
> > > + *   Return 0 if the permission is granted.
> > > + *
> > > + * @bpf_post_create:
> > > + *   Initialize the bpf object security field inside struct
> > > bpf_maps and
> > > + *   it is used for future security checks.
> > > + *
> > >   */
> > >  union security_list_options {
> > >       int (*binder_set_context_mgr)(struct task_struct *mgr);
> > > @@ -1685,6 +1711,14 @@ union security_list_options {
> > >                               struct audit_context *actx);
> > >       void (*audit_rule_free)(void *lsmrule);
> > >  #endif /* CONFIG_AUDIT */
> > > +
> > > +#ifdef CONFIG_BPF_SYSCALL
> > > +     int (*bpf_map_create)(void);
> > > +     int (*bpf_map_read)(struct bpf_map *map);
> > > +     int (*bpf_map_modify)(struct bpf_map *map);
> > > +     int (*bpf_prog_load)(void);
> > > +     int (*bpf_post_create)(struct bpf_map *map);
> > > +#endif /* CONFIG_BPF_SYSCALL */
> > >  };
> > > 
> > >  struct security_hook_heads {
> > > @@ -1905,6 +1939,13 @@ struct security_hook_heads {
> > >       struct list_head audit_rule_match;
> > >       struct list_head audit_rule_free;
> > >  #endif /* CONFIG_AUDIT */
> > > +#ifdef CONFIG_BPF_SYSCALL
> > > +     struct list_head bpf_map_create;
> > > +     struct list_head bpf_map_read;
> > > +     struct list_head bpf_map_modify;
> > > +     struct list_head bpf_prog_load;
> > > +     struct list_head bpf_post_create;
> > > +#endif /* CONFIG_BPF_SYSCALL */
> > >  } __randomize_layout;
> > > 
> > >  /*
> > > diff --git a/include/linux/security.h b/include/linux/security.h
> > > index 458e24bea2d4..0656a4f74d14 100644
> > > --- a/include/linux/security.h
> > > +++ b/include/linux/security.h
> > > @@ -31,6 +31,7 @@
> > >  #include <linux/string.h>
> > >  #include <linux/mm.h>
> > >  #include <linux/fs.h>
> > > +#include <linux/bpf.h>
> > > 
> > >  struct linux_binprm;
> > >  struct cred;
> > > @@ -1735,6 +1736,41 @@ static inline void
> > > securityfs_remove(struct
> > > dentry *dentry)
> > > 
> > >  #endif
> > > 
> > > +#ifdef CONFIG_BPF_SYSCALL
> > > +#ifdef CONFIG_SECURITY
> > > +int security_map_create(void);
> > > +int security_map_modify(struct bpf_map *map);
> > > +int security_map_read(struct bpf_map *map);
> > > +int security_prog_load(void);
> > > +int security_post_create(struct bpf_map *map);
> > > +#else
> > > +static inline int security_map_create(void)
> > > +{
> > > +     return 0;
> > > +}
> > > +
> > > +static inline int security_map_read(struct bpf_map *map)
> > > +{
> > > +     return 0;
> > > +}
> > > +
> > > +static inline int security_map_modify(struct bpf_map *map)
> > > +{
> > > +     return 0;
> > > +}
> > > +
> > > +static inline int security_prog_load(void)
> > > +{
> > > +     return 0;
> > > +}
> > > +
> > > +static inline int security_post_create(struct bpf_map *map)
> > > +{
> > > +     return 0;
> > > +}
> > > +#endif /* CONFIG_SECURITY */
> > > +#endif /* CONFIG_BPF_SYSCALL */
> > 
> > These should be named consistently with the ones in lsm_hooks.h and
> > should unambiguously indicate that these are hooks for bpf
> > objects/operations, i.e. security_bpf_map_create(),
> > security_bpf_map_read(), etc.
> > 
> 
> Thanks for pointing out, will fix this.
> > Do you need this level of granularity?
> > 
> 
> The cover letter of this patch series described a possible use cases
> of
> these lsm hooks and this level of granularity would be ideal to reach
> that
> goal. We can also implement two hooks such as bpf_obj_create and
> bpf_obj_use to restrict the creation and using when get the bpf fd
> from
> kernel. But that will be less powerful and flexible.
> > Could you coalesce the map_create() and post_map_create() hooks
> > into
> > one hook and just unwind the create in that case?
> > 
> 
> Okay, I will take a look on how to fix this.

Also, what you called security_post_create() would normally be called
something like security_bpf_alloc_security(), and would have a
corresponding security_bpf_free_security() hook too.  However, whether
or not you still need this security field and hook at all is unclear to
me, given the direction the discussion has gone.

> > Why do you label bpf maps but not bpf progs?  Should we be
> > controlling
> > the ability to attach/detach a bpf prog (partly controlled by
> > CAP_NET_ADMIN, but also somewhat broad in scope and doesn't allow
> > control based on who created the prog)?
> > 
> > Should there be a top-level security_bpf_use() hook and permission
> > check that limits ability to use bpf() at all?
> > 
> 
> This could be useful but having additional lsm hooks check when
> reading
> and write to eBPF maps may cause performance issue. Instead maybe we
> could have a hook for creating eBPF object and retrieve object fd to
> restrict
> the access.
> > > +
> > >  #ifdef CONFIG_SECURITY
> > > 
> > >  static inline char *alloc_secdata(void)
> > > diff --git a/security/security.c b/security/security.c
> > > index 55b5997e4b72..02272f93a89e 100644
> > > --- a/security/security.c
> > > +++ b/security/security.c
> > > @@ -12,6 +12,7 @@
> > >   *   (at your option) any later version.
> > >   */
> > > 
> > > +#include <linux/bpf.h>
> > >  #include <linux/capability.h>
> > >  #include <linux/dcache.h>
> > >  #include <linux/module.h>
> > > @@ -1708,3 +1709,30 @@ int security_audit_rule_match(u32 secid,
> > > u32
> > > field, u32 op, void *lsmrule,
> > >                               actx);
> > >  }
> > >  #endif /* CONFIG_AUDIT */
> > > +
> > > +#ifdef CONFIG_BPF_SYSCALL
> > > +int security_map_create(void)
> > > +{
> > > +     return call_int_hook(bpf_map_create, 0);
> > > +}
> > > +
> > > +int security_map_modify(struct bpf_map *map)
> > > +{
> > > +     return call_int_hook(bpf_map_modify, 0, map);
> > > +}
> > > +
> > > +int security_map_read(struct bpf_map *map)
> > > +{
> > > +     return call_int_hook(bpf_map_read, 0, map);
> > > +}
> > > +
> > > +int security_prog_load(void)
> > > +{
> > > +     return call_int_hook(bpf_prog_load, 0);
> > > +}
> > > +
> > > +int security_post_create(struct bpf_map *map)
> > > +{
> > > +     return call_int_hook(bpf_post_create, 0, map);
> > > +}
> > > +#endif /* CONFIG_BPF_SYSCALL */

[PATCH 1/3] security: bpf: Add eBPF LSM hooks to security module

[Stephen Smalley]

On Tue, 2017-09-05 at 15:24 -0700, Chenbo Feng via Selinux wrote:
> On Fri, Sep 1, 2017 at 5:50 AM, Stephen Smalley <sds@tycho.nsa.gov>
> wrote:
> > On Thu, 2017-08-31 at 13:56 -0700, Chenbo Feng wrote:
> > > From: Chenbo Feng <fengc@google.com>
> > > 
> > > Introduce 5 LSM hooks to provide finer granularity controls on
> > > eBPF
> > > related operations including create eBPF maps, modify and read
> > > eBPF
> > > maps
> > > content and load eBPF programs to the kernel. Hooks use the new
> > > security
> > > pointer inside the eBPF map struct to store the owner's security
> > > information and the different security modules can perform
> > > different
> > > checks based on the information stored inside the security field.
> > > 
> > > Signed-off-by: Chenbo Feng <fengc@google.com>
> > > ---
> > > ?include/linux/lsm_hooks.h | 41
> > > +++++++++++++++++++++++++++++++++++++++++
> > > ?include/linux/security.h??| 36
> > > ++++++++++++++++++++++++++++++++++++
> > > ?security/security.c???????| 28 ++++++++++++++++++++++++++++
> > > ?3 files changed, 105 insertions(+)
> > > 
> > > diff --git a/include/linux/lsm_hooks.h
> > > b/include/linux/lsm_hooks.h
> > > index ce02f76a6188..3aaf9a08a983 100644
> > > --- a/include/linux/lsm_hooks.h
> > > +++ b/include/linux/lsm_hooks.h
> > > @@ -1353,6 +1353,32 @@
> > > ? *???@inode we wish to get the security context of.
> > > ? *???@ctx is a pointer in which to place the allocated security
> > > context.
> > > ? *???@ctxlen points to the place to put the length of @ctx.
> > > + *
> > > + * Security hooks for using the eBPF maps and programs
> > > functionalities through
> > > + * eBPF syscalls.
> > > + *
> > > + * @bpf_map_create:
> > > + *???Check permissions prior to creating a new bpf map.
> > > + *???Return 0 if the permission is granted.
> > > + *
> > > + * @bpf_map_modify:
> > > + *???Check permission prior to insert, update and delete map
> > > content.
> > > + *???@map pointer to the struct bpf_map that contains map
> > > information.
> > > + *???Return 0 if the permission is granted.
> > > + *
> > > + * @bpf_map_read:
> > > + *???Check permission prior to read a bpf map content.
> > > + *???@map pointer to the struct bpf_map that contains map
> > > information.
> > > + *???Return 0 if the permission is granted.
> > > + *
> > > + * @bpf_prog_load:
> > > + *???Check permission prior to load eBPF program.
> > > + *???Return 0 if the permission is granted.
> > > + *
> > > + * @bpf_post_create:
> > > + *???Initialize the bpf object security field inside struct
> > > bpf_maps and
> > > + *???it is used for future security checks.
> > > + *
> > > ? */
> > > ?union security_list_options {
> > > ??????int (*binder_set_context_mgr)(struct task_struct *mgr);
> > > @@ -1685,6 +1711,14 @@ union security_list_options {
> > > ??????????????????????????????struct audit_context *actx);
> > > ??????void (*audit_rule_free)(void *lsmrule);
> > > ?#endif /* CONFIG_AUDIT */
> > > +
> > > +#ifdef CONFIG_BPF_SYSCALL
> > > +?????int (*bpf_map_create)(void);
> > > +?????int (*bpf_map_read)(struct bpf_map *map);
> > > +?????int (*bpf_map_modify)(struct bpf_map *map);
> > > +?????int (*bpf_prog_load)(void);
> > > +?????int (*bpf_post_create)(struct bpf_map *map);
> > > +#endif /* CONFIG_BPF_SYSCALL */
> > > ?};
> > > 
> > > ?struct security_hook_heads {
> > > @@ -1905,6 +1939,13 @@ struct security_hook_heads {
> > > ??????struct list_head audit_rule_match;
> > > ??????struct list_head audit_rule_free;
> > > ?#endif /* CONFIG_AUDIT */
> > > +#ifdef CONFIG_BPF_SYSCALL
> > > +?????struct list_head bpf_map_create;
> > > +?????struct list_head bpf_map_read;
> > > +?????struct list_head bpf_map_modify;
> > > +?????struct list_head bpf_prog_load;
> > > +?????struct list_head bpf_post_create;
> > > +#endif /* CONFIG_BPF_SYSCALL */
> > > ?} __randomize_layout;
> > > 
> > > ?/*
> > > diff --git a/include/linux/security.h b/include/linux/security.h
> > > index 458e24bea2d4..0656a4f74d14 100644
> > > --- a/include/linux/security.h
> > > +++ b/include/linux/security.h
> > > @@ -31,6 +31,7 @@
> > > ?#include <linux/string.h>
> > > ?#include <linux/mm.h>
> > > ?#include <linux/fs.h>
> > > +#include <linux/bpf.h>
> > > 
> > > ?struct linux_binprm;
> > > ?struct cred;
> > > @@ -1735,6 +1736,41 @@ static inline void
> > > securityfs_remove(struct
> > > dentry *dentry)
> > > 
> > > ?#endif
> > > 
> > > +#ifdef CONFIG_BPF_SYSCALL
> > > +#ifdef CONFIG_SECURITY
> > > +int security_map_create(void);
> > > +int security_map_modify(struct bpf_map *map);
> > > +int security_map_read(struct bpf_map *map);
> > > +int security_prog_load(void);
> > > +int security_post_create(struct bpf_map *map);
> > > +#else
> > > +static inline int security_map_create(void)
> > > +{
> > > +?????return 0;
> > > +}
> > > +
> > > +static inline int security_map_read(struct bpf_map *map)
> > > +{
> > > +?????return 0;
> > > +}
> > > +
> > > +static inline int security_map_modify(struct bpf_map *map)
> > > +{
> > > +?????return 0;
> > > +}
> > > +
> > > +static inline int security_prog_load(void)
> > > +{
> > > +?????return 0;
> > > +}
> > > +
> > > +static inline int security_post_create(struct bpf_map *map)
> > > +{
> > > +?????return 0;
> > > +}
> > > +#endif /* CONFIG_SECURITY */
> > > +#endif /* CONFIG_BPF_SYSCALL */
> > 
> > These should be named consistently with the ones in lsm_hooks.h and
> > should unambiguously indicate that these are hooks for bpf
> > objects/operations, i.e. security_bpf_map_create(),
> > security_bpf_map_read(), etc.
> > 
> 
> Thanks for pointing out, will fix this.
> > Do you need this level of granularity?
> > 
> 
> The cover letter of this patch series described a possible use cases
> of
> these lsm hooks and this level of granularity would be ideal to reach
> that
> goal. We can also implement two hooks such as bpf_obj_create and
> bpf_obj_use to restrict the creation and using when get the bpf fd
> from
> kernel. But that will be less powerful and flexible.
> > Could you coalesce the map_create() and post_map_create() hooks
> > into
> > one hook and just unwind the create in that case?
> > 
> 
> Okay, I will take a look on how to fix this.

Also, what you called security_post_create() would normally be called
something like security_bpf_alloc_security(), and would have a
corresponding security_bpf_free_security() hook too.  However, whether
or not you still need this security field and hook at all is unclear to
me, given the direction the discussion has gone.

> > Why do you label bpf maps but not bpf progs???Should we be
> > controlling
> > the ability to attach/detach a bpf prog (partly controlled by
> > CAP_NET_ADMIN, but also somewhat broad in scope and doesn't allow
> > control based on who created the prog)?
> > 
> > Should there be a top-level security_bpf_use() hook and permission
> > check that limits ability to use bpf() at all?
> > 
> 
> This could be useful but having additional lsm hooks check when
> reading
> and write to eBPF maps may cause performance issue. Instead maybe we
> could have a hook for creating eBPF object and retrieve object fd to
> restrict
> the access.
> > > +
> > > ?#ifdef CONFIG_SECURITY
> > > 
> > > ?static inline char *alloc_secdata(void)
> > > diff --git a/security/security.c b/security/security.c
> > > index 55b5997e4b72..02272f93a89e 100644
> > > --- a/security/security.c
> > > +++ b/security/security.c
> > > @@ -12,6 +12,7 @@
> > > ? *???(at your option) any later version.
> > > ? */
> > > 
> > > +#include <linux/bpf.h>
> > > ?#include <linux/capability.h>
> > > ?#include <linux/dcache.h>
> > > ?#include <linux/module.h>
> > > @@ -1708,3 +1709,30 @@ int security_audit_rule_match(u32 secid,
> > > u32
> > > field, u32 op, void *lsmrule,
> > > ??????????????????????????????actx);
> > > ?}
> > > ?#endif /* CONFIG_AUDIT */
> > > +
> > > +#ifdef CONFIG_BPF_SYSCALL
> > > +int security_map_create(void)
> > > +{
> > > +?????return call_int_hook(bpf_map_create, 0);
> > > +}
> > > +
> > > +int security_map_modify(struct bpf_map *map)
> > > +{
> > > +?????return call_int_hook(bpf_map_modify, 0, map);
> > > +}
> > > +
> > > +int security_map_read(struct bpf_map *map)
> > > +{
> > > +?????return call_int_hook(bpf_map_read, 0, map);
> > > +}
> > > +
> > > +int security_prog_load(void)
> > > +{
> > > +?????return call_int_hook(bpf_prog_load, 0);
> > > +}
> > > +
> > > +int security_post_create(struct bpf_map *map)
> > > +{
> > > +?????return call_int_hook(bpf_post_create, 0, map);
> > > +}
> > > +#endif /* CONFIG_BPF_SYSCALL */
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html