[PATCH V3 02/10] capabilities: intuitive names for cap gain status

[paul@paul-moore.com (Paul Moore)]

On Thu, Aug 24, 2017 at 3:06 PM, Kees Cook <keescook@chromium.org> wrote:
> On Thu, Aug 24, 2017 at 9:37 AM, Serge E. Hallyn <serge@hallyn.com> wrote:
>> Quoting Richard Guy Briggs (rgb at redhat.com):
>>> On 2017-08-24 11:03, Serge E. Hallyn wrote:
>>> > Quoting Richard Guy Briggs (rgb at redhat.com):
>>> > > Introduce macros cap_gained, cap_grew, cap_full to make the use of the
>>> > > negation of is_subset() easier to read and analyse.
>>> > >
>>> > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
>>> > > ---
>>> > >  security/commoncap.c |   16 ++++++++++------
>>> > >  1 files changed, 10 insertions(+), 6 deletions(-)
>>> > >
>>> > > diff --git a/security/commoncap.c b/security/commoncap.c
>>> > > index b7fbf77..6f05ec0 100644
>>> > > --- a/security/commoncap.c
>>> > > +++ b/security/commoncap.c
>>> > > @@ -513,6 +513,12 @@ void handle_privileged_root(struct linux_binprm *bprm, bool has_cap, bool *effec
>>> > >           *effective = true;
>>> > >  }
>>> > >
>>> >
>>> > It's subjective and so might be just me, but I think I'd find it easier
>>> > to read if it was cap_gained(source, target, field) and cap_grew(cred, source, target)
>>>
>>> In more than one place, I wanted to put the parameter that I was trying
>>> to read aloud closest to the function name to make reading it flow
>>> better, leaving the parameters less critical to comprehension towards
>>> the end.
>>
>> And I see that in the final patch it looks nicer the way you have it.
>>
>>> > This looks correct though, so either way
>>> >
>>> > Reviewed-by: Serge Hallyn <serge@hallyn.com>
>>>
>>> Thanks.  Did you want to put this through, or send it through Paul's
>>> audit tree?
>>
>> If Paul's around I'm happy to have it go through his tree.

Since Serge is okay with these I'll take a closer look and if it all
looks good I can pull it in to the audit tree (no objections from me
on the last revision, although I remember it being much smaller).

That said, since we are already at -rc6, I'm going to defer merging
this into audit/next *after* the upcoming merge window.  We are right
at where I normally draw the line and considering the scope and nature
of this patchset I think having a full RC cycle in linux-next would be
a good thing.

> Is this series based against -next with the changes that touch commoncap.c?
>
> Also, did you validate this with the existing LTP tests and selftests?
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=for-next/secureexec&id=ee67ae7ef6ff499137292ac8a9dfe86096796283

Another reason for keeping this in the queue a bit longer.  Richard,
can you do this testing before the upcoming merge window closes?

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