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

On 2017-08-24 12:06, Kees Cook wrote: > On Thu, Aug 24, 2017 at 9:37 AM, Serge E. Hallyn <serge(a)hallyn.com&gt; wrote: > > Quoting Richard Guy Briggs (rgb(a)redhat.com): > >> On 2017-08-24 11:03, Serge E. Hallyn wrote: > >> > Quoting Richard Guy Briggs (rgb(a)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(a)redhat.com&gt; > >> > > --- > >> > > 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(a)hallyn.com&gt; > >> > >> 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=... 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