Re: [PATCH] Smack: SMACK_IOCLOADACCESS

Casey Schaufler
2011-08-26T22:01:56Z

On 8/26/2011 10:01 AM, Eric Paris wrote:
> On 08/26/2011 12:05 PM, Sakkinen, Jarkko wrote:
>> On Fri, Aug 26, 2011 at 3:50 PM, Eric Paris <eparis@parisplace.org> wrote:
>>> On Fri, Aug 26, 2011 at 1:52 AM, Jarkko Sakkinen
>>> <jarkko.sakkinen@intel.com> wrote:
>>>> IOCTL call for /smack/load that takes access rule in
>>>> the same format as they are written into /smack/load.
>>>> Sets errno to zero if access is allowed and to EACCES
>>>> if not.
>>>>
>>>> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@intel.com>
>>> [SELinux maintainer here, but Casey knew to already take what I say
>>> with a grain of salt]
>>>
>>> I'm not telling you to do anything differently, just telling you what
>>> SELinux does, and why we do it. SELinux has a file in selinuxfs
>>> called 'access.' The file can be opened and one can write a rule into
>>> the file. One then calls read and gets back a structure which
>>> contains all of the permissions information allowed for the
>>> source/target/class. In SELinux we calculate all of the permissions
>>> for the tuple at once so providing all of the information at once can
>>> make a lot of sense. libselinux provides libraries that will cache
>>> these decisions in the userspace program and quickly answer the same
>>> (or similar) questions later.
>>>
>>> http://userspace.selinuxproject.org/trac/browser/libselinux/src/compute_av.c
>> Thank you for this information. One thing that concerns
>> me in this approach is the scenario where things serialize
>> to the following sequence:
>>
>> - Process A does open()
>> - Process B does open()
>> - Process A does write()
>> - Process B does write()
>> - Process A does read()
>> - ... (sequence continues)
>>
>> What's the end result?
> SELinux attaches the information needed to the struct file private area
> inside the kernel using the kernel provided fs/libfs.c functions
> simple_transation_*. Which means that 2 processes have no issues
> interfering with each other. A multi threaded or misbehaving
> application may get EBUSY on write() if another write()/read() combo is
> in progress. Its nice that the kernel has libraries which solve this
> problem for us!
>
> I don't know SMACK internals, but if one ever wants to have SMACK
> userspace object managers the ability for the interface to only be able
> to return a single value might be an eventual bottleneck.

Better to have a single question answered as required and with
complete accuracy than to carry around the baggage necessary to
maintain a cached duplicate of the kernel's rules and all the
bookkeeping that requires. SELinux libraries probably have to
make a system call just to determine if the caches they are
maintaining are out of date.

>
> Like I said, do whatever you guys think is best, but I'm constantly
> going to point out and ask for LSM similarities when possible!

I'm going to argue that in this case the interface puts
excessive burden on the user space code understanding a
peculiar behavior and that an ioctl is significantly more
appropriate to the task at hand. Even if there is precedence
for doing it using transaction IO the scheme smells of foul
sorcery.

> -Eric
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>