Re: [patch 0/15] lsm stacking v0.3: intro

[Tony Jones <tonyj@suse.de>]

Hi Serge

> > 5) /*
> >  * Workarounds for the fact that get and setprocattr are used only by
> >  * selinux.  (Maybe)
> >  */
> > 
> > No complaints on selinux getting to avoid the (module), they are intree.
> > Just a FYI that SubDomain/AppArmor uses these hooks also.
> 
> And is it ok with using the "some_data (apparmor)" convention?

Yes.  Our use of setprocattr is thru a library fn so I just made the change
there.  We'll have to change our user tools (that read /proc/pid/attr/current
but thats fine too).

> The special handling of selinux is intended to be temporary, due to the
> large base of installed userspace which hasn't yet been updated.  I
> would imagine that at some point that code would go away.

I assumed it was due to this. Doesn't inconvenience us any and it it helps 
SELinux it's fine w/ me.


Of more concern is ps -Z (pstools).

We had to have the pstools maintainer extend the set of characters that it
considered valid from the getprocattr.    I forget the details but IIRC he 
wanted to know (for ?documentation?) every character that could be returned 
by our getprocattr hook (which for us is pretty much any character thats 
valid in a pathname -- though IIRC we forgot one).

Anyway, I'm guessing (at least with pstools 3.2.5) that '(' is not one of
the valid characters. IIRC ps gives up when it sees a "non-valid" character.

I wrote a trivial little lsm which just returns 'foobar' for any getprocattr.

# cat /proc/2322/attr/current 
unconstrained (subdomain)
foobar (foobar)

# ps -Z -p 2322
LABEL                             PID TTY          TIME CMD
unconstrained                    2322 ttyS0    00:00:00 bash

Even if ps did return them all, I think it could create a usability problem.  
There was another LSM (forget which) which wanted to return a large blob 
from getprocattr, I recall like a page? in size which obviously caused problems
for ps -Z both in terms of content and especially length.

> > I noticed the conditional CONFIG_SECURITY_STACKER code went away, previously
> > it would look at the value chain head only for the !case. But this comment
> > still remains.
> 
> Yes, after I added the unlink function, it started to seem that the
> special cases for !CONFIG_SECURITY_STACKER wouldn't be any faster than
> the stacker versions.  They still might be, but I'll have to think about
> it.  If I just ditch those, then I can probably ditch the whole

Esp since James' suggestion would impact it. I'd imagine you would always want
array[0] for this case, no?

I was just pointing out the legacy comment. Thats all.

Thanks again

Tony