On Tue, May 19, 2015 at 12:56 AM, Michael Kerrisk (man-pages) <mtk.manpages@xxxxxxxxx> wrote: > Hi Andy, > > Thanks for this patch. There are some broken pieces though. Also, > I have some minor questions about the API design. See below. > > On 05/15/2015 08:43 AM, Andy Lutomirski wrote: >> Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx> >> --- >> >> There was no v1. I'm calling this v2 to keep it in sync with the kernel >> patch versioning. >> >> man2/prctl.2 | 10 ++++++++++ >> man7/capabilities.7 | 32 ++++++++++++++++++++++++++------ >> 2 files changed, 36 insertions(+), 6 deletions(-) >> >> diff --git a/man2/prctl.2 b/man2/prctl.2 >> index b352f6283624..5861e3aefe9a 100644 >> --- a/man2/prctl.2 >> +++ b/man2/prctl.2 >> @@ -949,6 +949,16 @@ had been called. >> For further information on Intel MPX, see the kernel source file >> .IR Documentation/x86/intel_mpx.txt . >> .\" >> +.TP >> +.BR PR_CAP_AMBIENT " (since Linux 4.2)" >> +Reads or changes the ambient capability set. If arg2 is PR_CAP_AMBIENT_RAISE, >> +then the capability specified in arg3 is added to the ambient set. This will >> +fail, returning EPERM, if the capability is not already both permitted and >> +inheritable or if the SECBIT_NO_CAP_AMBIENT_RAISE securebit is set. If arg2 >> +is PR_CAP_AMBIENT_LOWER, then the capability specified in arg3 is removed >> +from the ambient set. If arg2 is PR_CAP_AMBIENT_GET, then >> +.BR prctl (2) >> +will return 1 if the capability in arg3 is in the ambient set and 0 if not. > > Some API design questions: > > 1. We already have prctl() operations that work on some capability sets: > PR_CAPBSET_READ and PR_CAPBSET_DROP. These don't use arg3; the operation > is directly encoded in the first argument of prctl(). Just to keep some > consistency, why not do things the same way for these new operations? I'm torn. On the one hand, consistency is nice. On the other hand, prctl is a mess and trying to organize new additions seems like a good idea. > > Also, you could opt for some consistency in the naming, so using "READ" > rather than "GET", for example. On the other hand, both "READ" and "GET" > are suboptimal names: this is really a test operation. So, maybe a > clean break to a good name, PR_CAP_AMBIENT_IS_SET, is best? I like IS_SET. > > Thus: > > prctl(PR_CAP_AMBIENT_READ, cap, 0, 0, 0); // or PR_CAP_AMBIENT_IS_SET? > prctl(PR_CAP_AMBIENT_RAISE, cap, 0, 0, 0); > prctl(PR_CAP_AMBIENT_LOWER, cap, 0, 0, 0); > > 2. In terms of the API design, would it be useful to have a prctl() operation > that clears the entire ambient set? > > prctl(PR_CAP_AMBIENT_ZERO, 0, 0, 0, 0); // or PR_CAP_AMBIENT_EMPTY? Seems like a good idea. How about _CLEAR? > >> .SH RETURN VALUE >> On success, >> .BR PR_GET_DUMPABLE , >> diff --git a/man7/capabilities.7 b/man7/capabilities.7 >> index d75ec65de05b..dae62f0be3b7 100644 >> --- a/man7/capabilities.7 >> +++ b/man7/capabilities.7 >> @@ -697,13 +697,26 @@ a program whose associated file capabilities grant that capability). >> .IR Inheritable : >> This is a set of capabilities preserved across an >> .BR execve (2). >> -It provides a mechanism for a process to assign capabilities >> -to the permitted set of the new program during an >> -.BR execve (2). >> +Inheritable capabilities remain inheritable when executing any program, >> +and inheritable capabilities are added to the permitted set when executing >> +a program that has the corresponding bits set in the file inheritable set. >> +When executing programs without file capabilities, ambient capabilities > > That last line is incomplete. Something needs adding/removing. Whoops, will fix. > >> .TP >> .IR Effective : >> This is the set of capabilities used by the kernel to >> perform permission checks for the thread. >> +.TP >> +.IR Ambient " (since Linux 4.2) :" > > Minor knit: s/ :/:/ for next version. Will fix. > >> +This is a set of capabilities that are preserved across an >> +.BR execve (2) >> +of a program that does not have file capabilities. The ambient capability >> +set obeys the invariant that no capability can ever be ambient if it is >> +not both permitted and inheritable. Ambient capabilities are, with some >> +exceptions, preserved in the permitted set and added to the effective >> +set when >> +.BR execve (2) >> +is called. The ambient capability set is modified using >> +.BR prctl (2). > > I think it would be helpful to add a couple of sentences here on why the > ambient set is useful (i.e., explain what deficiencies in the pre-existing > API are addressed by the addition of this set--a brief piece from your > 1/2 patch, for example). Will do. --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-api" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
