Re: Another version of stacker.c (URL attached) - Locking.

lachlanat_private

David Wheeler wrote:
> Below is some discussion on locking; I now plan to
> take a very conservative and "safe" approach for at least
> the first version of "Stacker".  Comments welcome.
> 
> Lachlan McIlroy wrote:
> 
>> dwheelerat_private wrote:
>>
>>
>>
>> David,
>>
>> There is a problem with unloading subordinate modules while
>> another process is executing one of its hooks.  This can even
>> happen on uniprocessor systems if a process is sleeping in a
>> hook.  When the module is removed from memory the other
>> process will cause an oops or panic when it continues.
> 
> 
> If writing a pointer is atomic,
> the "stacker" module ITSELF won't cause a panic.
> When a module is removed, the current code intentionally
> does NOT change the "next" value of the linked list of
> modules, and the module information is never free'd.
> That way, walking through the list will not cause a
> problem.
> 
> That being said, there _IS_ reason for your concern.
> Stacker _itself_ may be okay, but it's quite
> plausible that a stacked module may not handle the
> case of "mod_unreg()" returning, followed later by
> a call to one of its hooks.  I presume that's what you
> meant.  Indeed, handling module
> unloading is in general problem, and this is just yet
> another example.
I was referring to the case where a process calls a hook
before the module is unloaded and doesn't return until
after the module has finished unloading.  The process's
stack will have a reference to a function that no longer
exists in memory.

> 
> I think for the moment I'll take a very conservative
> approach to locking.  Anything that reads or writes
> the state of Stacker (i.e., the linked list of modules
> or the value of pseudo_dummy) must hold a lock on that
> first (read or write, depending on how they access it).
> It will be some sort of reader/writer lock (e.g., allow
> multiple simultaneous readers, but a writer will OWN it).
If you have module A and module B registered with Stacker
and a process is sleeping in one of module A's hooks then
the read lock will remain held and you wont be able to
remove module A or module B.

As an alternative to using locks will this work?

in each hook:

if (module_p->active) {
	atomic_inc (&module_p->counter);
	res = module_p->security_ops->CALL;
	atomic_dec (&module_p->counter);
}

when unregistering a module:

	...
	module_p->active = 0;
	while (atomic_read (&module_p->counter))
		schedule ();
	/* clean up module */
	...

The module's entry will have to remain in the list but
it's memory can be freed.

Lachlan

> 
> That means that EVERY CALL to EVERY HOOK will grab and
> hold the reader/writer lock; this will slow
> things down, but it's much simpler to reason about and
> show correct.  It completely eliminates the race condition
> I was concerned about (not being able to atomically set
> both the linked list and the value of pseudo_dummy).
> It handles the case of those (nasty $#%#) architectures
> where pointer-writing isn't atomic.
> It also means that when unregistering returns, there really
> will be no more calls to its hooks (the case you were
> concerned about).  Finally, it lets me cleanly free the memory
> inside stacker, which isn't really important but is a
> nice-to-have.
> 
> I really like the summary of locking approaches at:
> http://www.linuxjournal.com/article.php?sid=5833.
> I think my two options are a Reader/Write semaphore
> or a big-reader lock (brlocks).  In the long term a
> brlock is probably more appropriate, because writing
> is an extremely rare operation, and brlocks are more
> efficient when grabbing a read lock.  However, it looks like I'd
> have to get someone (Ingo?) to register this use of brlocks.
> So, for the moment, I'll probably insert reader/writer
> semaphors, and later switch to brlocks if there's interest.
> 
> Any comments?
> 
> 
> --- David A. Wheeler
>     dwheelerat_private
> 
> _______________________________________________
> linux-security-module mailing list
> linux-security-moduleat_private
> http://mail.wirex.com/mailman/listinfo/linux-security-module
> 
> 

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