On Sun, 2013-09-15 at 20:04 -0700, Davidlohr Bueso wrote:
> This patchset deals with the selinux and rmid races Manfred found on
> the ipc scaling work that has been going on. It specifically addresses
> shared mem and msg queues. While semaphores still need updated, I want
> to make sure these are correct first. Also, Manfred had already sent out
> a patchset that deals with a race in sem complex operations. So any changes
> should be on top of his.
>
> Patches 1 and 2 deal with shared memory.
> Patches 3 and 4 deal with msg queues.
> Specific details about each race and its fix are in the corresponding
> patches.
>
> Note that Linus suggested a good alternative to patches 1 and 3: use
> kfree_rcu() and delay the freeing of the security structure. I would
> much prefer that approach to doing security checks with the lock held,
> but I want to leave the patches out and ready in case we go with the
> later solution.
Cc'ing selinux/security people - folks, could you please confirm that
this change is ok and won't break anything related to security?
Thanks!
8<-------------------------------------------
From: Davidlohr Bueso <davidl...@hp.com>
Date: Thu, 19 Sep 2013 09:32:05 -0700
Subject: [PATCH] selinux: rely on rcu to free ipc isec
Currently, IPC mechanisms do security and auditing related checks under
RCU. However, since selinux can free the security structure, through
selinux_[sem,msg_queue,shm]_free_security(), we can race if the structure
is freed before other tasks are done with it, creating a use-after-free
condition. Manfred illustrates this nicely, for example with shared mem:
--> do_shmat calls rcu_read_lock()
--> do_shmat calls shm_object_check().
Checks that the object is still valid - but doesn't acquire any locks.
Then it returns.
--> do_shmat calls security_shm_shmat (e.g. selinux_shm_shmat)
--> selinux_shm_shmat calls ipc_has_perm()
--> ipc_has_perm accesses ipc_perms->security
shm_close()
--> shm_close acquires rw_mutex & shm_lock
--> shm_close calls shm_destroy
--> shm_destroy calls security_shm_free (e.g. selinux_shm_free_security)
--> selinux_shm_free_security calls ipc_free_security(&shp->shm_perm)
--> ipc_free_security calls kfree(ipc_perms->security)
There are two solutions to this: (i) perform the security checking and whatever
IPC operation(s) atomically (that is, with the kern_ipc_perm.lock held), or
(ii) delay the freeing of the isec structure after all RCU readers are done.
This patch takes the second approach, which, at least from a performance
perspective,
is more practical than the first as it keeps the IPC critical regions smaller.
I have tested this patch with IPC testcases from LTP on both my quad-core
laptop and on a 64 core NUMA server. In both cases selinux is enabled, and
tests pass for both voluntary and forced preemption models.
Signed-off-by: Davidlohr Bueso <davidl...@hp.com>
---
security/selinux/hooks.c | 9 ++++++++-
security/selinux/include/objsec.h | 5 +++--
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index a5091ec..179ffe9 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4892,8 +4892,15 @@ static int ipc_alloc_security(struct task_struct *task,
static void ipc_free_security(struct kern_ipc_perm *perm)
{
struct ipc_security_struct *isec = perm->security;
+
+ /*
+ * All IPC mechanisms do security and audit
+ * checking under RCU: wait a grace period before
+ * freeing isec, otherwise we can run into a
+ * use-after-free scenario.
+ */
+ kfree_rcu(isec, rcu);
perm->security = NULL;
- kfree(isec);
}
static int msg_msg_alloc_security(struct msg_msg *msg)
diff --git a/security/selinux/include/objsec.h
b/security/selinux/include/objsec.h
index aa47bca..38d17b7 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -70,8 +70,9 @@ struct msg_security_struct {
};
struct ipc_security_struct {
- u16 sclass; /* security class of this object */
- u32 sid; /* SID of IPC resource */
+ u16 sclass; /* security class of this object */
+ u32 sid; /* SID of IPC resource */
+ struct rcu_head rcu; /* rcu struct for selinux based IPC security */
};
struct netif_security_struct {
--
1.7.11.7
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
