Re: [PATCH 0/3] Smack: add support for modification of existing rules, restructure rules list showing in smackfs

[Casey Schaufler <casey@schaufler-ca.com>]

On 11/27/2012 9:40 AM, Rafal Krypa wrote:
> The following three patches are intended to introduce in-place
> modification of Smack rules. Until now Smack supported only
> overwriting of existing rules. To change permitted access for a given
> subject and object, user had to read list of rules to get current
> accesses, modify it and write modified rule back to kernel. This way
> was inefficient, non-atomic and unnecessarily difficult.
> New interface is intended to ease such modifications.
>
> I have prepared three patches:
> 1. Use RCU functions and read locking in smackfs seq list operations
>
> Because rule lists will now get modified by list_replace_rcu(), this
> one is intended to assure RCU reader critical sections in smackfs.

The whole change to RCU is predicated on the use of list_replace_rcu.
But there is no reason to use list_replace_rcu. The existing code assigns
an integer value into an existing list entry, and there is no reason
not to continue doing so. The patch results in code that is working
harder than it has to.

Don't create a new list entry and replace the old list entry.
Do modify the access field of the existing entry.
This patch is completely unnecessary.

> 2. Remove global master list of rules
>
> This is for avoiding having to modify rules in two places (per subject
> list and the global list). The master list was redundant and kept up
> for backward compatibility with previous smackfs seq operations code.

There is no need to modify the rule in two places. The master list
contains pointers to the same rule structures as the per-subject list.
This is another reason not to use list_replace_rcu. You can get rid
of the master list for the shear joy of it, but there isn't any need
to do so to complete the work at hand.

> 3. Add support for modification of existing rules
>
> The actual patch with new interface.
> A previous version of this one has posted previously
> (http://thread.gmane.org/gmane.linux.documentation/6759),
> but was proven to be wrong.

This can be made much simpler. It should duplicate the code for load2
with the exception of how the access is determined. Changes to reduce
code duplication are of course appropriate.

>
> Rafal Krypa (3):
>   Smack: use RCU functions and read locking in smackfs seq list
>     operations
>   Smack: remove global master list of rules
>   Smack: add support for modification of existing rules
>
>  Documentation/security/Smack.txt |   11 ++
>  security/smack/smackfs.c         |  362 ++++++++++++++++++++++++--------------
>  2 files changed, 239 insertions(+), 134 deletions(-)
>