[PATCH v2 3/9] setpriv: dump user and group info

[Patrick Steinhardt]

On Tue, Jul 04, 2017 at 07:06:10PM +0200, Denys Vlasenko wrote:
> On Sun, Jul 2, 2017 at 3:42 PM, Patrick Steinhardt <ps at pks.im> wrote:
> > setpriv from util-linux has an option to dump the current state
> > regarding privilege settings via `--dump`. It prints out information on
> > the real and effective user and group IDs, supplementary groups, the
> > no-new-privs flag, the capability sets as well as secure bits.
> >
> > This patch is the start of supporting this mode. To make introduction of
> > the `--dump` easier to reason about, its introduction has been split
> > into multiple patches. This particular one introduces the ability to
> > print out user and group information of the current process.
> > ---
> >  util-linux/setpriv.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 69 insertions(+), 2 deletions(-)
> >
> > diff --git a/util-linux/setpriv.c b/util-linux/setpriv.c
> > index d586f4bef..dabc6d355 100644
> > --- a/util-linux/setpriv.c
> > +++ b/util-linux/setpriv.c
> > @@ -15,6 +15,14 @@
> >  //config:      help
> >  //config:        Run a program with different Linux privilege settings.
> >  //config:        Requires kernel >= 3.5
> > +//config:
> > +//config:config FEATURE_SETPRIV_DUMP
> > +//config:      bool "Support dumping current privilege state"
> > +//config:      default y
> > +//config:      depends on SETPRIV
> > +//config:      help
> > +//config:              Enables the "--dump" switch to print out the current privilege
> > +//config:              state. This is helpful for diagnosing problems.
> 
> Please use consistent style. All other help is indented by two spaces.
> 
> >  #include <sys/prctl.h>
> > +#include <unistd.h>
> >  #include "libbb.h"
> 
> Not needed.
> 
> > +       if (getresuid(&ruid, &euid, &suid) < 0)
> > +               bb_simple_perror_msg_and_die("getresgid");
> > +
> > +       if (getresgid(&rgid, &egid, &sgid) < 0)
> > +               bb_simple_perror_msg_and_die("getresgid");
> > +
> > +       if ((ngids = getgroups(0, NULL)) < 0)
> > +               bb_simple_perror_msg_and_die("getgroups");
> 
> These never fail on Linux.
> 
> > +       gids = xmalloc(ngids * sizeof(*gids));
> > +       if ((ngids = getgroups(ngids, gids)) < 0)
> > +               bb_simple_perror_msg_and_die("getgroups");
> 
> It's high time to have a wrapper for this allocation thing,
> let's call it bb_getgroups()
> 
> > +               int i;
> > +               for (i = 0; i < ngids; i++) {
> > +                       if (i)
> > +                               putchar(',');
> > +                       printf("%d", gids[i]);
> 
> Fewer branches - less code:
> 
> +               const char *fmt = ",%u" + 1;
> +               int i;
> +               for (i = 0; i < ngids; i++) {
> +                       printf(fmt, (unsigned)gids[i]);
> +                       fmt = ",%u";
> +               }
> 
> > +       free(gids);
> > +       return 0;
> 
> Should be:
> 
> +       if (ENABLE_FEATURE_CLEAN_UP)
> +               free(gids);
> +       return EXIT_SUCCESS;
> 
> 
> Applied with edits. Thank you.
> 
> It's better to include "make bloatcheck" results. For this commit,
> I'm getting
> 
>     function                                             old     new   delta
>     setpriv_main                                          89     322    +233
>     getresuid                                              -      41     +41
>     getresgid                                              -      41     +41
>     static.setpriv_longopts                               22      29      +7
>     packed_usage                                       31675   31669      -6
>     ------------------------------------------------------------------------------
>     (add/remove: 4/0 grow/shrink: 2/1 up/down: 322/-6)
> Total: 316 bytes
> 
> Also, you have no "signed-off" line in your patches. If you want them to
> go under your name, please add it.
> 
> Can you rediff and resend remaining patches against current git?

Will do, probably tomorrow. Thanks for fixing up and applying the rest!

Regards
Patrick
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20170705/8d201d90/attachment.asc>