Re: Updated stacker.c (the real one, ignore the last post)

dwheelerat_private

Seth Arnold said:
>David, thanks for starting work on the stacking module. At this point,
>you know the LSM interface much better than I do, but I thought the
>sys_security() had been designed to allow userspace to contact a
>specific module through the module numeric id. (Which is computed with
>the md5sum of the module name, by convention.) The stacking module
>should be able to just call every stacked module with the paramters
>unchanged and allow the stacked module to recognize that the call was
>intended for some other security module, and return with a specific
>errno.

Actually, the version I posted does _exactly_ that already, in the
line that says:
  RETURN_ERROR_IF_ANY_ERROR(sys_security(id,call,args));

The problem isn't a matter of efficiency.
The problem is that when I call _all_ the modules, I don't know
which module's return value to return.  If there are 4 loaded modules,
I currently call 4 modules, and I get 4 results.
Quiz: which result should I return to the caller?
If the caller ignores the return value then you're fine,
but if the value actually has meaning, then I have to answer the question.

Also, I think it's a mistake to assume that sys_security
returns an "errno".  sys_security merely returns an int, and nowhere
is there any promise of some more specific meaning.

>This will give errors:
>       for (module_p =3D stacked_modules; module_p; module_p->next) { \
use this instead:
>       for (module_p =3D stacked_modules; module_p; module_p =3D module_p->next=
) { \

Thanks!!! In this _particular_ case I happened to find this independently,
but this is EXACTLY why I posted, so that other eyes and brains could
start to examine the sanity of this.  Thanks for noting these bad
loop statements.

>This is as far as I made it before I had to leave; as a final parting
>comment, I suggest that your stacking module should use some form of
>locking to protect the module stack, and acquire that lock when
>traversing the stack to compute error returns. (I suggest using the RCU
>locking mechanism, presented at OLS, as that would make the stack
>traversal very cheap, while pushing the locking costs onto the stack
>change procedures, which is likely going to be very rare in a system's
>uptime. A first cut should probably use a simpler locking primitive, but
>mark it with a TODO for some enterprising chap in the future. :)

I'm really trying to NOT lock except where necessary.
Assuming that writing a pointer is atomic, I think I can avoid
many cases where locking is needed by carefully approaching the problem.
I only _append_ when I add new modules, and the operation that makes it
"real" writes a single pointer value.
When removing a module, the operation writes a simple pointer value that
causes the removal (other threads using the removed module will
continue obliviously).

Of course, that assumes that writing a pointer value is atomic;
perhaps that't not true on some 64-bit processors.
Is this a bad assumption? If it is, is there any way to indicate
"this write must be atomic?"

There _is_ a race condition when a module is installed that will cause
the internal dummy module to turn off (this is marked as one of the
TODO items).  I don't know if race condition actually matters - if it does,
I'll need to lock the list for those operations that use
pseudo_dummy as well as the installation.  See the TODO comments
about this race condition.  If I can't be sure it's irrelevant,
I'll have to put locks there.  Comments welcome.

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