On Thu, Aug 24, 2017 at 3:06 PM, Kees Cook [off-list ref] wrote:

On Thu, Aug 24, 2017 at 9:37 AM, Serge E. Hallyn [off-list ref] wrote:

quoted

Quoting Richard Guy Briggs (rgb at redhat.com):

quoted

On 2017-08-24 11:03, Serge E. Hallyn wrote:

quoted

Quoting Richard Guy Briggs (rgb at redhat.com):

quoted

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 <redacted>
---
 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.

quoted

quoted

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