Re: [PATCH v2 1/2] security: Add hook to invalidate inode security labels

2015-10-05 Thread Casey Schaufler
On 10/4/2015 12:19 PM, Andreas Gruenbacher wrote:
> Add a hook to invalidate an inode's security label when the cached
> information becomes invalid.

Where is this used? If I need to do the same for Smack
or any other module, how would I know that it works right?

>
> Implement the new hook in selinux: set a flag when a security label becomes
> invalid.  When hitting a security label which has been marked as invalid in
> inode_has_perm, try reloading the label.
>
> If an inode does not have any dentries attached, we cannot reload its
> security label because we cannot use the getxattr inode operation.  In that
> case, continue using the old, invalid label until a dentry becomes
> available.
>
> Signed-off-by: Andreas Gruenbacher 
> Cc: Paul Moore 
> Cc: Stephen Smalley 
> Cc: Eric Paris 
> Cc: selinux@tycho.nsa.gov
> ---
>  include/linux/lsm_hooks.h |  6 ++
>  include/linux/security.h  |  5 +
>  security/security.c   |  8 
>  security/selinux/hooks.c  | 23 +--
>  security/selinux/include/objsec.h |  3 ++-
>  5 files changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index ec3a6ba..945ae1d 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1261,6 +1261,10 @@
>   *   audit_rule_init.
>   *   @rule contains the allocated rule
>   *
> + * @inode_invalidate_secctx:
> + *   Notify the security module that it must revalidate the security context
> + *   of an inode.
> + *
>   * @inode_notifysecctx:
>   *   Notify the security module of what the security context of an inode
>   *   should be.  Initializes the incore security context managed by the
> @@ -1516,6 +1520,7 @@ union security_list_options {
>   int (*secctx_to_secid)(const char *secdata, u32 seclen, u32 *secid);
>   void (*release_secctx)(char *secdata, u32 seclen);
>  
> + void (*inode_invalidate_secctx)(struct inode *inode);
>   int (*inode_notifysecctx)(struct inode *inode, void *ctx, u32 ctxlen);
>   int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen);
>   int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen);
> @@ -1757,6 +1762,7 @@ struct security_hook_heads {
>   struct list_head secid_to_secctx;
>   struct list_head secctx_to_secid;
>   struct list_head release_secctx;
> + struct list_head inode_invalidate_secctx;
>   struct list_head inode_notifysecctx;
>   struct list_head inode_setsecctx;
>   struct list_head inode_getsecctx;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 2f4c1f7..9692571 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -353,6 +353,7 @@ int security_secid_to_secctx(u32 secid, char **secdata, 
> u32 *seclen);
>  int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
>  void security_release_secctx(char *secdata, u32 seclen);
>  
> +void security_inode_invalidate_secctx(struct inode *inode);
>  int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
>  int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
>  int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
> @@ -1093,6 +1094,10 @@ static inline void security_release_secctx(char 
> *secdata, u32 seclen)
>  {
>  }
>  
> +static inline void security_inode_invalidate_secctx(struct inode *inode)
> +{
> +}
> +
>  static inline int security_inode_notifysecctx(struct inode *inode, void 
> *ctx, u32 ctxlen)
>  {
>   return -EOPNOTSUPP;
> diff --git a/security/security.c b/security/security.c
> index 46f405c..e4371cd 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1161,6 +1161,12 @@ void security_release_secctx(char *secdata, u32 seclen)
>  }
>  EXPORT_SYMBOL(security_release_secctx);
>  
> +void security_inode_invalidate_secctx(struct inode *inode)
> +{
> + call_void_hook(inode_invalidate_secctx, inode);
> +}
> +EXPORT_SYMBOL(security_inode_invalidate_secctx);
> +
>  int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen)
>  {
>   return call_int_hook(inode_notifysecctx, 0, inode, ctx, ctxlen);
> @@ -1763,6 +1769,8 @@ struct security_hook_heads security_hook_heads = {
>   LIST_HEAD_INIT(security_hook_heads.secctx_to_secid),
>   .release_secctx =
>   LIST_HEAD_INIT(security_hook_heads.release_secctx),
> + .inode_invalidate_secctx =
> + LIST_HEAD_INIT(security_hook_heads.inode_invalidate_secctx),
>   .inode_notifysecctx =
>   LIST_HEAD_INIT(security_hook_heads.inode_notifysecctx),
>   .inode_setsecctx =
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index e4369d8..c5e4ca8 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1293,11 +1293,11 @@ static 

[RFC PATCH v2 1/5] kdbus: add creator credentials to the endpoints

2015-10-05 Thread Paul Moore
In order to effectively enforce LSM based access controls we need to
have more information about the kdbus endpoint creator than the
uid/gid currently stored in the kdbus_node_type struct.  This patch
replaces the uid/gid values with a reference to the node creator's
credential struct which serves the needs of both the kdbus DAC access
controls as well as the LSM's access controls.

Two macros have also been created, kdbus_node_[uid,gid](), which can
be used to easily extract the euid/egid information from the new
credential reference.  The effective uid/gid is used as it was used
in all areas of the previous kdbus code except for areas where the
uid/gid was never set beyond the basic initialization to zero/root;
I expect this was a bug that was never caught as the node creator in
these cases was always expect to be root.

Signed-off-by: Paul Moore 

---
ChangeLog:
- v2
 * Initial draft
---
 ipc/kdbus/bus.c  |   13 +
 ipc/kdbus/endpoint.c |   14 --
 ipc/kdbus/endpoint.h |3 +--
 ipc/kdbus/fs.c   |4 ++--
 ipc/kdbus/node.c |   11 ---
 ipc/kdbus/node.h |5 +++--
 6 files changed, 19 insertions(+), 31 deletions(-)

diff --git a/ipc/kdbus/bus.c b/ipc/kdbus/bus.c
index a67f825..0cb9501 100644
--- a/ipc/kdbus/bus.c
+++ b/ipc/kdbus/bus.c
@@ -65,8 +65,7 @@ static void kdbus_bus_release(struct kdbus_node *node, bool 
was_active)
 static struct kdbus_bus *kdbus_bus_new(struct kdbus_domain *domain,
   const char *name,
   struct kdbus_bloom_parameter *bloom,
-  const u64 *pattach_owner,
-  u64 flags, kuid_t uid, kgid_t gid)
+  const u64 *pattach_owner, u64 flags)
 {
struct kdbus_bus *b;
u64 attach_owner;
@@ -81,7 +80,8 @@ static struct kdbus_bus *kdbus_bus_new(struct kdbus_domain 
*domain,
if (ret < 0)
return ERR_PTR(ret);
 
-   ret = kdbus_verify_uid_prefix(name, domain->user_namespace, uid);
+   ret = kdbus_verify_uid_prefix(name, domain->user_namespace,
+ current_euid());
if (ret < 0)
return ERR_PTR(ret);
 
@@ -93,8 +93,6 @@ static struct kdbus_bus *kdbus_bus_new(struct kdbus_domain 
*domain,
 
b->node.free_cb = kdbus_bus_free;
b->node.release_cb = kdbus_bus_release;
-   b->node.uid = uid;
-   b->node.gid = gid;
b->node.mode = S_IRUSR | S_IXUSR;
 
if (flags & (KDBUS_MAKE_ACCESS_GROUP | KDBUS_MAKE_ACCESS_WORLD))
@@ -374,7 +372,7 @@ struct kdbus_bus *kdbus_cmd_bus_make(struct kdbus_domain 
*domain,
bus = kdbus_bus_new(domain,
argv[1].item->str, [2].item->bloom_parameter,
argv[3].item ? argv[3].item->data64 : NULL,
-   cmd->flags, current_euid(), current_egid());
+   cmd->flags);
if (IS_ERR(bus)) {
ret = PTR_ERR(bus);
bus = NULL;
@@ -393,8 +391,7 @@ struct kdbus_bus *kdbus_cmd_bus_make(struct kdbus_domain 
*domain,
goto exit;
}
 
-   ep = kdbus_ep_new(bus, "bus", cmd->flags, bus->node.uid, bus->node.gid,
- false);
+   ep = kdbus_ep_new(bus, "bus", cmd->flags, false);
if (IS_ERR(ep)) {
ret = PTR_ERR(ep);
ep = NULL;
diff --git a/ipc/kdbus/endpoint.c b/ipc/kdbus/endpoint.c
index 44e7a20..1ba5d51 100644
--- a/ipc/kdbus/endpoint.c
+++ b/ipc/kdbus/endpoint.c
@@ -74,8 +74,6 @@ static void kdbus_ep_release(struct kdbus_node *node, bool 
was_active)
  * @bus:   The bus this endpoint will be created for
  * @name:  The name of the endpoint
  * @access:The access flags for this node (KDBUS_MAKE_ACCESS_*)
- * @uid:   The uid of the node
- * @gid:   The gid of the node
  * @is_custom: Whether this is a custom endpoint
  *
  * This function will create a new endpoint with the given
@@ -84,8 +82,7 @@ static void kdbus_ep_release(struct kdbus_node *node, bool 
was_active)
  * Return: a new kdbus_ep on success, ERR_PTR on failure.
  */
 struct kdbus_ep *kdbus_ep_new(struct kdbus_bus *bus, const char *name,
- unsigned int access, kuid_t uid, kgid_t gid,
- bool is_custom)
+ unsigned int access, bool is_custom)
 {
struct kdbus_ep *e;
int ret;
@@ -96,7 +93,7 @@ struct kdbus_ep *kdbus_ep_new(struct kdbus_bus *bus, const 
char *name,
 */
if (is_custom) {
ret = kdbus_verify_uid_prefix(name, bus->domain->user_namespace,
- uid);
+ current_euid());
if (ret < 0)
return ERR_PTR(ret);

[RFC PATCH v2 3/5] lsm: add support for auditing kdbus service names

2015-10-05 Thread Paul Moore
The kdbus service names will be recorded using 'service', similar to
the existing dbus audit records.

Signed-off-by: Paul Moore 

---
ChangeLog:
- v2
 * Initial draft
---
 include/linux/lsm_audit.h |2 ++
 security/lsm_audit.c  |4 
 2 files changed, 6 insertions(+)

diff --git a/include/linux/lsm_audit.h b/include/linux/lsm_audit.h
index ffb9c9d..d6a656f 100644
--- a/include/linux/lsm_audit.h
+++ b/include/linux/lsm_audit.h
@@ -59,6 +59,7 @@ struct common_audit_data {
 #define LSM_AUDIT_DATA_INODE   9
 #define LSM_AUDIT_DATA_DENTRY  10
 #define LSM_AUDIT_DATA_IOCTL_OP11
+#define LSM_AUDIT_DATA_KDBUS   12
union   {
struct path path;
struct dentry *dentry;
@@ -75,6 +76,7 @@ struct common_audit_data {
 #endif
char *kmod_name;
struct lsm_ioctlop_audit *op;
+   const char *kdbus_name;
} u;
/* this union contains LSM specific data */
union {
diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index 9f6c649..d7af41d 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -397,6 +397,10 @@ static void dump_common_audit_data(struct audit_buffer *ab,
audit_log_format(ab, " kmod=");
audit_log_untrustedstring(ab, a->u.kmod_name);
break;
+   case LSM_AUDIT_DATA_KDBUS:
+   audit_log_format(ab, " service=");
+   audit_log_untrustedstring(ab, a->u.kdbus_name);
+   break;
} /* switch (a->type) */
 }
 

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


[RFC PATCH v2 2/5] lsm: introduce hooks for kdbus

2015-10-05 Thread Paul Moore
Add LSM access control hooks to kdbus; several new hooks are added and
the existing security_file_receive() hook is reused.  The new hooks
are listed below:

 * security_kdbus_conn_new
   Check if the current task is allowed to create a new kdbus
   connection.
 * security_kdbus_own_name
   Check if a connection is allowed to own a kdbus service name.
 * security_kdbus_conn_talk
   Check if a connection is allowed to talk to a kdbus peer.
 * security_kdbus_conn_see
   Check if a connection can see a kdbus peer.
 * security_kdbus_conn_see_name
   Check if a connection can see a kdbus service name.
 * security_kdbus_conn_see_notification
   Check if a connection can receive notifications.
 * security_kdbus_proc_permission
   Check if a connection can access another task's pid namespace info.
 * security_kdbus_init_inode
   Set the security label on a kdbusfs inode

Signed-off-by: Paul Moore 

---
ChangeLog:
- v2
 * Implemented suggestions by Stephen Smalley
   * call security_kdbus_conn_new() sooner
   * reworked hook inside kdbus_conn_policy_own_name()
   * fixed if-conditional in kdbus_conn_policy_talk()
   * reworked hook inside kdbus_conn_policy_see_name_unlocked()
   * reworked hook inside kdbus_conn_policy_see()
   * reworked hook inside kdbus_conn_policy_see_notification()
   * added the security_kdbus_init_inode() hook
- v1
 * Initial draft
---
 include/linux/security.h |  126 ++
 ipc/kdbus/connection.c   |   73 +--
 ipc/kdbus/fs.c   |6 ++
 ipc/kdbus/message.c  |   19 +--
 ipc/kdbus/metadata.c |6 +-
 security/security.c  |   50 ++
 6 files changed, 245 insertions(+), 35 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 18264ea..7992663 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -53,6 +53,9 @@ struct msg_queue;
 struct xattr;
 struct xfrm_sec_ctx;
 struct mm_struct;
+struct kdbus_creds;
+struct kdbus_pids;
+struct pid;
 
 /* Maximum number of letters for an LSM name string */
 #define SECURITY_NAME_MAX  10
@@ -1300,6 +1303,45 @@ static inline void security_free_mnt_opts(struct 
security_mnt_opts *opts)
  * @file contains the struct file being transferred.
  * @to contains the task_struct for the receiving task.
  *
+ * @kdbus_conn_new
+ * Check if the current task is allowed to create a new kdbus connection.
+ * @creds credentials for the new connection
+ * @fake_creds kdbus faked credentials
+ * @fake_pids kdbus faked pids
+ * @fake_seclabel kdbus faked security label
+ * @owner kdbus owner
+ * @privileged kdbus privileged
+ * @is_activator kdbus activator boolean
+ * @is_monitor kdbus monitor boolean
+ * @is_policy_holder kdbus policy holder boolean
+ * @kdbus_own_name
+ * Check if a connection is allowed to own a kdbus service name.
+ * @creds requestor's credentials
+ * @name service name
+ * @kdbus_conn_talk
+ * Check if a connection is allowed to talk to a kdbus peer.
+ * @creds requestor's credentials
+ * @creds_peer peer credentials
+ * @kdbus_conn_see
+ * Check if a connection can see a kdbus peer.
+ * @creds requestor's credentials
+ * @creds_peer peer credentials
+ * @kdbus_conn_see_name
+ * Check if a connection can see a kdbus service name.
+ * @creds requestor's credentials
+ * @name service name
+ * @kdbus_conn_see_notification
+ * Check if a connection can receive notifications.
+ * @creds requestor's credentials
+ * @kdbus_proc_permission
+ * Check if a connection can access another task's pid namespace info.
+ * @cred requestor's credentials
+ * @pid target task's pid struct
+ * @kdbus_init_inode
+ * Set the security label on a kdbusfs inode
+ * @inode kdbusfs inode
+ * @creds inode owner credentials
+ *
  * @ptrace_access_check:
  * Check permission before allowing the current process to trace the
  * @child process.
@@ -1468,6 +1510,22 @@ struct security_operations {
int (*binder_transfer_file) (struct task_struct *from,
 struct task_struct *to, struct file *file);
 
+   int (*kdbus_conn_new)(const struct cred *creds,
+ const struct kdbus_creds *fake_creds,
+ const struct kdbus_pids *fake_pids,
+ const char *fake_seclabel,
+ bool owner, bool privileged, bool is_activator,
+ bool is_monitor, bool is_policy_holder);
+   int (*kdbus_own_name)(const struct cred *creds, const char *name);
+   int (*kdbus_conn_talk)(const struct cred *creds,
+  const struct cred *creds_peer);
+   int (*kdbus_conn_see)(const struct cred *creds,
+ const struct cred *creds_peer);
+   int (*kdbus_conn_see_name)(const 

[RFC PATCH v2 5/5] selinux: introduce kdbus access controls

2015-10-05 Thread Paul Moore
Add the SELinux access control implementation for the new kdbus LSM
hooks using the new kdbus object class and the following permissions:

 [NOTE: permissions below are based on kdbus code from Aug 2015]

 * kdbus:impersonate
   Send a different security label to kdbus peers.
 * kdbus:fakecreds
   Send different DAC credentials to kdbus peers.
 * kdbus:fakepids
   Send a different PID to kdbus peers.
 * kdbus:owner
   Act as a kdbus bus owner.
 * kdbus:privileged
   Act as a privileged endpoint.
 * kdbus:activator
   Act as a kdbus activator.
 * kdbus:monitor
   Act as a kdbus monitor.
 * kdbus:policy_holder
   Act as a kdbus policy holder.
 * kdbus:connect
   Create a new kdbus connection.
 * kdbus:own
   Own a kdbus service name.
 * kdbus:talk
   Talk between two kdbus endpoints.
 * kdbus:see
   See another kdbus endpoint.
 * kdbus:see_name
   See a kdbus service name.
 * kdbus:see_notification
   See a kdbus notification.

Signed-off-by: Paul Moore 

---
ChangeLog:
- v2
 * Add the selinux_kdbus_init_inode() hook
 * Add some very basic info on the permissions to the description
 * Add kdbus service name auditing in the AVC records
- v1
 * Initial draft
---
 security/selinux/hooks.c|  152 +++
 security/selinux/include/classmap.h |4 +
 2 files changed, 154 insertions(+), 2 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 4d7e602..29341dd 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -9,8 +9,10 @@
  *   James Morris 
  *
  *  Copyright (C) 2001,2002 Networks Associates Technology, Inc.
- *  Copyright (C) 2003-2008 Red Hat, Inc., James Morris 
- *Eric Paris 
+ *  Copyright (C) 2003-2008,2015 Red Hat, Inc.
+ * James Morris 
+ * Eric Paris 
+ * Paul Moore 
  *  Copyright (C) 2004-2005 Trusted Computer Solutions, Inc.
  * 
  *  Copyright (C) 2006, 2007, 2009 Hewlett-Packard Development Company, L.P.
@@ -1987,6 +1989,143 @@ static int selinux_binder_transfer_file(struct 
task_struct *from,
);
 }
 
+static int selinux_kdbus_conn_new(const struct cred *creds,
+ const struct kdbus_creds *fake_creds,
+ const struct kdbus_pids *fake_pids,
+ const char *fake_seclabel,
+ bool owner, bool privileged,
+ bool is_activator, bool is_monitor,
+ bool is_policy_holder)
+{
+   int rc;
+   u32 tsid = current_sid();
+   u32 av = KDBUS__CONNECT;
+
+   if (fake_creds)
+   av |= KDBUS__FAKECREDS;
+   if (fake_pids)
+   av |= KDBUS__FAKEPIDS;
+   if (owner)
+   av |= KDBUS__OWNER;
+   if (privileged)
+   av |= KDBUS__PRIVILEGED;
+   if (is_activator)
+   av |= KDBUS__ACTIVATOR;
+   if (is_monitor)
+   av |= KDBUS__MONITOR;
+   if (is_policy_holder)
+   av |= KDBUS__POLICY_HOLDER;
+
+   rc = avc_has_perm(tsid, cred_sid(creds), SECCLASS_KDBUS, av, NULL);
+   if (rc)
+   return rc;
+
+   if (fake_seclabel) {
+   u32 sid;
+   if (security_context_to_sid(fake_seclabel,
+   strlen(fake_seclabel),
+   , GFP_KERNEL))
+   return -EINVAL;
+
+   rc = avc_has_perm(tsid, sid,
+ SECCLASS_KDBUS, KDBUS__IMPERSONATE, NULL);
+   }
+
+   return rc;
+}
+
+static int selinux_kdbus_own_name(const struct cred *creds, const char *name)
+{
+   int rc;
+   u32 name_sid;
+   struct common_audit_data ad;
+
+   rc = security_kdbus_sid(name, _sid);
+   if (rc)
+   return rc;
+
+   ad.type = LSM_AUDIT_DATA_KDBUS;
+   ad.u.kdbus_name = name;
+
+   return avc_has_perm(cred_sid(creds), name_sid,
+   SECCLASS_KDBUS, KDBUS__OWN, );
+}
+
+static int selinux_kdbus_conn_talk(const struct cred *creds,
+  const struct cred *creds_to)
+{
+   return avc_has_perm(cred_sid(creds), cred_sid(creds_to),
+   SECCLASS_KDBUS, KDBUS__TALK, NULL);
+}
+
+static int selinux_kdbus_conn_see(const struct cred *creds,
+ const struct cred *creds_whom)
+{
+   return avc_has_perm(cred_sid(creds), cred_sid(creds_whom),
+   SECCLASS_KDBUS, KDBUS__SEE, NULL);
+}
+
+static int selinux_kdbus_conn_see_name(const struct cred *creds,
+  

[PATCH] security: selinux: Use a kmem_cache for allocation struct file_security_struct

2015-10-05 Thread Sangwoo
The size of struct file_security_struct is 16byte at my setup.
But, the real allocation size for per each file_security_struct
is 64bytes in my setup that kmalloc min size is 64bytes
because ARCH_DMA_MINALIGN is 64.

This allocation is called every times at file allocation(alloc_file()).
So, the total slack memory size(allocated size - request size)
is increased exponentially.

E.g) Min Kmalloc Size : 64bytes, Unit : bytes
  Allocated Size | Request Size | Slack Size | Allocation Count
---
 770048  |192512|   577536   |  12032

At the result, this change reduce memory usage 42bytes per each
file_security_struct

Signed-off-by: Sangwoo 
---
 security/selinux/hooks.c |8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 3f8d567..c20e082 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -126,6 +126,7 @@ int selinux_enabled = 1;
 #endif
 
 static struct kmem_cache *sel_inode_cache;
+static struct kmem_cache *file_security_cache;
 
 /**
  * selinux_secmark_enabled - Check to see if SECMARK is currently enabled
@@ -287,7 +288,7 @@ static int file_alloc_security(struct file *file)
struct file_security_struct *fsec;
u32 sid = current_sid();
 
-   fsec = kzalloc(sizeof(struct file_security_struct), GFP_KERNEL);
+   fsec = kmem_cache_zalloc(file_security_cache, GFP_KERNEL);
if (!fsec)
return -ENOMEM;
 
@@ -302,7 +303,7 @@ static void file_free_security(struct file *file)
 {
struct file_security_struct *fsec = file->f_security;
file->f_security = NULL;
-   kfree(fsec);
+   kmem_cache_free(file_security_cache, fsec);
 }
 
 static int superblock_alloc_security(struct super_block *sb)
@@ -6086,6 +6087,9 @@ static __init int selinux_init(void)
sel_inode_cache = kmem_cache_create("selinux_inode_security",
sizeof(struct 
inode_security_struct),
0, SLAB_PANIC, NULL);
+   file_security_cache = kmem_cache_create("selinux_file_security",
+   sizeof(struct file_security_struct),
+   0, SLAB_PANIC, NULL);
avc_init();
 
security_add_hooks(selinux_hooks, ARRAY_SIZE(selinux_hooks));
-- 
1.7.9.5

___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.


Re: MAP_STACK and execstack

2015-10-05 Thread Stephen Smalley

On 10/02/2015 04:44 PM, Nick Kralevich wrote:

Currently, SELinux implements the "execstack" capability using the
following code:

security/selinux/hooks.c
function: selinux_file_mprotect()

} else if (!vma->vm_file &&
   vma->vm_start <= vma->vm_mm->start_stack &&
   vma->vm_end >= vma->vm_mm->start_stack) {
 rc = current_has_perm(current, PROCESS__EXECSTACK);
}

However, at least on Android, this check doesn't work for pthread
allocated stacks. Those stacks are allocated in libc via mmap(), and
aren't accounted for in the kernel as stack pages. As a result,
attempting to mprotect(PROT_EXEC) a pthread stack page requires the
"execmem" permission, not the "execstack" permission.

"man mmap" defines MAP_STACK, which is currently a no-op in the kernel
indicating that the memory is intended to be used as a stack. In
theory, Android's libc could set this flag for memory intended to be
used as a stack, but doing so is useless if the kernel ignores it.

Is there any reason why SELinux shouldn't use MAP_STACK to determine
whether the execmem or execstack capability is checked? In Android,
this would be a net security win, since nobody is granted execstack
today.


To clarify, execstack is a check on mprotect() in addition to (not 
instead of) execmem when the mapping is part of the stack.  execmem is 
always required.


At the time execstack was added, /proc/pid/maps / show_map_vma only 
identified the main process stack and the same logic was used to 
identify it for the execstack check.


Since that time, /proc/pid/maps has been augmented to also identify 
thread stacks, so the logic from show_map_vma could be reused for the 
execstack check as well.


Using MAP_STACK as the basis for the check seems less desirable, as it 
is entirely at the discretion of the program (so can be easily omitted, 
thereby making the check opt-in), is only passed on mmap() not 
mprotect() and doesn't appear to be saved anywhere, so it isn't 
available on mprotect() calls, and is ignored by the kernel.  The commit 
that added it said:


commit cd98a04a59e2f94fa64d5bf1e26498d27427d5e7
Author: Ingo Molnar 
Date:   Wed Aug 13 18:02:18 2008 +0200

x86: add MAP_STACK mmap flag

as per this discussion:

   http://lkml.org/lkml/2008/8/12/423

Pardo reported that 64-bit threaded apps, if their stacks exceed the
combined size of ~4GB, slow down drastically in pthread_create() - 
because

glibc uses MAP_32BIT to allocate the stacks. The use of MAP_32BIT is
a legacy hack - to speed up context switching on certain early model
64-bit P4 CPUs.

So introduce a new flag to be used by glibc instead, to not constrain
64-bit apps like this.

glibc can switch to this new flag straight away - it will be ignored
by the kernel. If those old CPUs ever matter to anyone, support for
it can be implemented.
___
Selinux mailing list
Selinux@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.