Re: New module: tpe

chrisat_private

* Chris Wright (chrisat_private) wrote:
> * Niki Rahimi (narahimiat_private) wrote:
> > Chris Wright wrote:
> > 
> > >Great, at first glance this looks very nice.  It'd be great to merge this
> > >into the LSM tree, is that ok?
> > 
> > Thanks! That would be fine by me.
> 
> OK, I'll go ahead and commit it as is, and then we can work on any
> cleanups that might be necessary.

Alright, after some minor license changes to make sure it's Dual BSD/GPL
licensed to honor the original copyright holders license choice, the
thing is committed to the LSM tree.

I made some minor changes:

1) kern_tpe.h -> tpe.h
2) lsm-tpe-readme.txt -> lsm/tpe.txt
3) MODULE_LICNESE("GPL") -> MODULE_LICENSE("Dual BSD/GPL")

Below are some suggestions for cleanup:

1) kern_tpe.h -> tpe.h

2) in kern_tpe.h all the functions that aren't intended to be inlined
should be moved to tpe.c.  Every function should be made static, as none
of them are needed outside the scope of tpe.[ch].

3) i'm not convinced the list needs to be sorted since the TRUSTED_PATH
test will succeed for most cases in the typical $PATH.  personally,
i'd consider making the search simpler, but since the work is done,
then at least do one thing...remove the recursive search algorithm.
I know it's only got one int on the stack, and being a O(log N) algo,
it won't go the depth of the list.  But recursive is just a bad idea if
it can be avoided.  The kernel stack is a very finite resource.  Granted,
the search function will always be called from a fairly shallow stack,
but still.

4)  i haven't looked at the full fs implementation, but my hunch is that
there is much more code than is needed.  is it true that the fs is just
for reading and writing to the tpe uid list?  if so, you don't need to be
able to do anything other than read and write to a single file in the root
dir (perhaps two separate files if you want).  this means that the dir
operations look overkill.  i don't think you need to be able to create
anything in the root dir.  so the mount should populate the root dir
with the read/wirte list entrypoints, and that's all i'd expect to see.

5) this code is smp unsafe, the list can be read and modified by
different tasks simultaneously and should be protected.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net
_______________________________________________
linux-security-module mailing list
linux-security-moduleat_private
http://mail.wirex.com/mailman/listinfo/linux-security-module