On 2017-08-24 12:06, Kees Cook 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.

Is this series based against -next with the changes that touch commoncap.c?

This series is against pcmoore's audit/next tree (I know I'm missing two
commits but they pose no conflict.).

Which -next tree are you talking about?  I might guess
linux-security/next or linux-next/master (I have at least a dozen "next"
in my git repo config.)

I did eventually find your patches in sfr's tree and in your for-next/kspp branch.

I'll have a look at the commoncap.c changes including the elimination of cap_effective.

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

No.  I will look into doing that.  Thanks for the suggestion.

I see that bprm->cap_effective has vanished, so that will affect at least one hunk.

Kees Cook

- RGB

--
Richard Guy Briggs [off-list ref]
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
--
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