Re: [PATCH] security_ops locking

dwheelerat_private

James Morris said:

 > The patch below implements locking of for the security module
 > registration interfaces, covering writing & testing of the 
security_ops pointer.

This patch solves a DIFFERENT problem than the one I'm concerned about;
it DOES NOT solve the problem I raised earlier if there are multiple
CPUs which don't guarantee atomic pointer writes.

The patch you've presented primarily solves the problem
of "multiple simultaneous
requests to load or unload a module". That's probably not necessary
in most circumstances, since an administrator who's trusted to
install security modules could be trusted to not try to load/unload
simultaneously (indeed, they'd probably happen in series).
Still, it's a cheap and simple protection against a case that,
if it DOES happen, could be disastrous if unprotected.
It also solves the problem on the nasty architectures if there's
only one CPU, since you're suspending IRQs.
So I'd support adding such a patch to LSM.

But this patch DOES NOT FULLY SOLVE THE PROBLEM OF POINTER WRITES BEING
NON-ATOMIC on the obnoxious CPU architectures if there's > 1 CPU.
Here's a trace, on an arch where pointer writes aren't atomic:
1. CPU#1 calls register_security
2. CPU#1 calls spin_lock_irqsave
3. CPU#1 checks, and later begins executing:
         security_ops = ops;
4.     While CPU#1 has written only part of security_ops,
        CPU#2 calls a hook:
          security_ops->pick_a_hook().
        Note that security_ops doesn't hold either the old value
        (the previous value of security_ops) nor the new value (ops),
        so CPU#2's kernel begins running at a rather unplanned location.

Quiz Question: How long will it take to reboot?

Grabbing the irq spinlock _DOES_ handle the weird architectures
if there's only one CPU; with only one CPU, blocking irq's really
does make it atomic.

Perhaps this is "the best that can be done easily" - it handles atomic
pointer write architectures (including the x86, yes?), and any
single-CPU system.  I'd love to see a full answer; on those
nasty architectures, suspending the other CPUs may be the only
way to go.

--- David A. Wheeler
     dwheelerat_private

_______________________________________________
linux-security-module mailing list
linux-security-moduleat_private
http://mail.wirex.com/mailman/listinfo/linux-security-module