shadow-maint / shadow

Upstream shadow tree
Other
287 stars 227 forks source link

lib/idmapping.c: Use long constants in prctl(2) #1011

Closed alejandro-colomar closed 1 week ago

alejandro-colomar commented 1 month ago

The prctl(2) system-call wrapper is implemented as a variadic function. This makes it important to pass arguments to it of the right type (and more importantly of the right width), to avoid undefined behavior.

While at it, check errors with ==-1, not <0, which is more explicit.

See-also: prctl(2), PR_SET_KEEPCAPS(2const) Link: https://lore.kernel.org/linux-man/ddbdyaiptesjalgfmztxideej67e3yaob7ucsmbf6qvriwxiif@dohhxrqgwhrf/T/#med306b5b003f9cc7cc2de69fcdd7ee2d056d0954 Cc: @xry111


See also the manual page for this call, recently split from prctl(2):

Link: https://www.alejandro-colomar.es/share/dist/man-pages/git/HEAD/man-pages-HEAD.pdf#PR_SET_KEEPCAPS.2const

Here's an excerpt:

SYNOPSIS
     #include <sys/prctl.h>

     int prctl(PR_SET_KEEPCAPS, long state, 0L, 0L, 0L);

DESCRIPTION
     Set  the  state  of the calling thread’s "keep capabilities" flag.
     The effect of this flag is described  in  capabilities(7).   state
     must  be  either  0L  (clear  the flag) or 1L (set the flag).  The
     "keep capabilities" value will be reset to 0 on  subsequent  calls
     to execve(2).

RETURN VALUE
     On success, 0 is returned.  On error, -1 is returned, and errno is
     set to indicate the error.

Edit: further modified the page (there's no need to pass all arguments to this prctl(2) operation):

SYNOPSIS
     #include <linux/prctl.h>  /* Definition of PR_* constants */
     #include <sys/prctl.h>

     int prctl(PR_SET_KEEPCAPS, long state);

DESCRIPTION
     Set  the  state of the calling thread’s "keep capabilities" flag.  The ef‐
     fect of this flag is described in capabilities(7).  state must  be  either
     0L  (clear  the flag) or 1L (set the flag).  The "keep capabilities" value
     will be reset to 0 on subsequent calls to execve(2).

RETURN VALUE
     On success, 0 is returned.  On error, -1 is returned, and errno is set  to
     indicate the error.

Revisions:

v1b - Rebase ``` $ git range-diff master..gh/prctl shadow/master..prctl 1: 5a06dc1a = 1: 3cea29d8 lib/idmapping.c: Use long constants in prctl(2) ```
v2 - Remove 0s ``` $ git range-diff alx/master gh/prctl prctl 1: 3cea29d8 ! 1: 204189a7 lib/idmapping.c: Use long constants in prctl(2) @@ Metadata Author: Alejandro Colomar ## Commit message ## - lib/idmapping.c: Use long constants in prctl(2) + lib/idmapping.c: Use long constants in prctl(2), and remove 0s The prctl(2) system-call wrapper is implemented as a variadic function. This makes it important to pass arguments to it of the right type (and @@ Commit message While at it, check errors with ==-1, not <0, which is more explicit. + Also, PR_SET_KEEPCAPS(2const) doesn't need all arguments, so it can be + called with just two of them; remove unnecessary 0s. + See-also: prctl(2), PR_SET_KEEPCAPS(2const) Link: Cc: Xi Ruoyao @@ lib/idmapping.c: void write_mapping(int proc_dir_fd, int ranges, const struct ma /* Align setuid- and fscaps-based new{g,u}idmap behavior. */ if (geteuid() == 0 && geteuid() != ruid) { - if (prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0) < 0) { -+ if (prctl(PR_SET_KEEPCAPS, 1L, 0L, 0L, 0L) == -1) { ++ if (prctl(PR_SET_KEEPCAPS, 1L) == -1) { fprintf(log_get_logfd(), _("%s: Could not prctl(PR_SET_KEEPCAPS)\n"), log_get_progname()); exit(EXIT_FAILURE); } ```
v2b - Rebase - Cc @lslebodn ``` $ git range-diff alx/master..gh/prctl shadow/master..prctl 1: 204189a7 ! 1: dbdb211a lib/idmapping.c: Use long constants in prctl(2), and remove 0s @@ Commit message See-also: prctl(2), PR_SET_KEEPCAPS(2const) Link: Cc: Xi Ruoyao + Cc: Lukas Slebodnik Signed-off-by: Alejandro Colomar ## lib/idmapping.c ## ```
v2c - Rebase ``` $ git range-diff master..gh/prctl shadow/master..prctl 1: dbdb211a = 1: 80efeeba lib/idmapping.c: Use long constants in prctl(2), and remove 0s ```