| Message ID | 20190709142908.13776-1-tz.stoyanov@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] trace-cmd: Add option to execute traced process as given user | expand |
On Tue, 2019-07-09 at 17:29 +0300, Tzvetomir Stoyanov (VMware) wrote: > A new trace-cmd record option is added: "--user". If a comnmand is > specified > as trce-cmd argument, it is executed in the context of the given > user. > > Suggested-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com> > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > --- > [ > v3 changes: > "--user" does not depend on option -F, it works with any > command, > specified as trace-cmd argument. > v2 changes: > - Check for errors in change_user(). If an error occurs while > changing the user, the message is printed and the traced > process is not executed. > ] > > > Documentation/trace-cmd-record.1.txt | 4 +++ > tracecmd/trace-record.c | 47 > ++++++++++++++++++++++++++-- > tracecmd/trace-usage.c | 1 + > 3 files changed, 49 insertions(+), 3 deletions(-) > > diff --git a/Documentation/trace-cmd-record.1.txt > b/Documentation/trace-cmd-record.1.txt > index 4a59de9..6a0f975 100644 > --- a/Documentation/trace-cmd-record.1.txt > +++ b/Documentation/trace-cmd-record.1.txt > @@ -122,6 +122,10 @@ OPTIONS > *--mmap*:: > Used with either *-F* or *-P*, save the traced process memory > map into > the trace.dat file. > + > +*--user*:: > + Execute the specified *command* as given user. > + > *-C* 'clock':: > Set the trace clock to "clock". > > diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c > index 48081d4..acadd7a 100644 > --- a/tracecmd/trace-record.c > +++ b/tracecmd/trace-record.c > @@ -33,6 +33,8 @@ > #include <errno.h> > #include <limits.h> > #include <libgen.h> > +#include <pwd.h> > +#include <grp.h> > > #include "version.h" > #include "trace-local.h" > @@ -208,6 +210,7 @@ struct common_record_context { > struct buffer_instance *instance; > const char *output; > char *date2ts; > + char *user; > int data_flags; > > int record_all; > @@ -1417,7 +1420,34 @@ static void trace_or_sleep(enum trace_type > type) > sleep(10); > } > > -static void run_cmd(enum trace_type type, int argc, char **argv) > +static int change_user(char *user) Nit: `const char *user` would be more appropriate. > +{ > + struct passwd *pwd; > + > + if (!user) > + return 0; > + > + pwd = getpwnam(user); > + if (!pwd) > + return -1; > + if (initgroups(user, pwd->pw_gid) < 0) > + return -1; > + if (setgid(pwd->pw_gid) < 0) > + return -1; > + if (setuid(pwd->pw_uid) < 0) > + return -1; > + > + if (setenv("HOME", pwd->pw_dir, 1) < 0) > + return -1; > + if (setenv("USER", pwd->pw_name, 1) < 0) > + return -1; > + if (setenv("LOGNAME", pwd->pw_name, 1) < 0) > + return -1; > + > + return 0; > +} > + > +static void run_cmd(enum trace_type type, char *user, int argc, char > **argv) Nit: same as above. > { > int status; > int pid; > @@ -1438,6 +1468,10 @@ static void run_cmd(enum trace_type type, int > argc, char **argv) > dup2(save_stdout, 1); > close(save_stdout); > } > + > + if (change_user(user) < 0) > + die("Failed to change user to %s", user); > + > if (execvp(argv[0], argv)) { > fprintf(stderr, "\n********************\n"); > fprintf(stderr, " Unable to exec %s\n", > argv[0]); > @@ -4548,6 +4582,7 @@ void update_first_instance(struct > buffer_instance *instance, int topt) > } > > enum { > + OPT_user = 243, > OPT_mmap = 244, > OPT_quiet = 245, > OPT_debug = 246, > @@ -4780,6 +4815,7 @@ static void parse_record_options(int argc, > {"quiet", no_argument, NULL, OPT_quiet}, > {"help", no_argument, NULL, '?'}, > {"mmap", no_argument, NULL, OPT_mmap}, > + {"user", required_argument, NULL, OPT_user}, > {"module", required_argument, NULL, > OPT_module}, > {NULL, 0, NULL, 0} > }; > @@ -5011,6 +5047,9 @@ static void parse_record_options(int argc, > case 'i': > ignore_event_not_found = 1; > break; > + case OPT_user: > + ctx->user = strdup(optarg); This is missing a check for allocation failure. Other than that, looks great. Reviewed-by: Slavomir Kaslev <kaslevs@vmware.com> Cheers, -- Slavi
On Tue, 9 Jul 2019 15:06:42 +0000 Slavomir Kaslev <kaslevs@vmware.com> wrote: > This is missing a check for allocation failure. > > Other than that, looks great. > > Reviewed-by: Slavomir Kaslev <kaslevs@vmware.com> I agree with everything Slavomir said. Tzvetomir, care to send a v4? Thanks! -- Steve
diff --git a/Documentation/trace-cmd-record.1.txt b/Documentation/trace-cmd-record.1.txt index 4a59de9..6a0f975 100644 --- a/Documentation/trace-cmd-record.1.txt +++ b/Documentation/trace-cmd-record.1.txt @@ -122,6 +122,10 @@ OPTIONS *--mmap*:: Used with either *-F* or *-P*, save the traced process memory map into the trace.dat file. + +*--user*:: + Execute the specified *command* as given user. + *-C* 'clock':: Set the trace clock to "clock". diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c index 48081d4..acadd7a 100644 --- a/tracecmd/trace-record.c +++ b/tracecmd/trace-record.c @@ -33,6 +33,8 @@ #include <errno.h> #include <limits.h> #include <libgen.h> +#include <pwd.h> +#include <grp.h> #include "version.h" #include "trace-local.h" @@ -208,6 +210,7 @@ struct common_record_context { struct buffer_instance *instance; const char *output; char *date2ts; + char *user; int data_flags; int record_all; @@ -1417,7 +1420,34 @@ static void trace_or_sleep(enum trace_type type) sleep(10); } -static void run_cmd(enum trace_type type, int argc, char **argv) +static int change_user(char *user) +{ + struct passwd *pwd; + + if (!user) + return 0; + + pwd = getpwnam(user); + if (!pwd) + return -1; + if (initgroups(user, pwd->pw_gid) < 0) + return -1; + if (setgid(pwd->pw_gid) < 0) + return -1; + if (setuid(pwd->pw_uid) < 0) + return -1; + + if (setenv("HOME", pwd->pw_dir, 1) < 0) + return -1; + if (setenv("USER", pwd->pw_name, 1) < 0) + return -1; + if (setenv("LOGNAME", pwd->pw_name, 1) < 0) + return -1; + + return 0; +} + +static void run_cmd(enum trace_type type, char *user, int argc, char **argv) { int status; int pid; @@ -1438,6 +1468,10 @@ static void run_cmd(enum trace_type type, int argc, char **argv) dup2(save_stdout, 1); close(save_stdout); } + + if (change_user(user) < 0) + die("Failed to change user to %s", user); + if (execvp(argv[0], argv)) { fprintf(stderr, "\n********************\n"); fprintf(stderr, " Unable to exec %s\n", argv[0]); @@ -4548,6 +4582,7 @@ void update_first_instance(struct buffer_instance *instance, int topt) } enum { + OPT_user = 243, OPT_mmap = 244, OPT_quiet = 245, OPT_debug = 246, @@ -4780,6 +4815,7 @@ static void parse_record_options(int argc, {"quiet", no_argument, NULL, OPT_quiet}, {"help", no_argument, NULL, '?'}, {"mmap", no_argument, NULL, OPT_mmap}, + {"user", required_argument, NULL, OPT_user}, {"module", required_argument, NULL, OPT_module}, {NULL, 0, NULL, 0} }; @@ -5011,6 +5047,9 @@ static void parse_record_options(int argc, case 'i': ignore_event_not_found = 1; break; + case OPT_user: + ctx->user = strdup(optarg); + break; case OPT_mmap: get_mmap = 1; break; @@ -5079,7 +5118,6 @@ static void parse_record_options(int argc, if (!ctx->filtered && ctx->instance->filter_mod) add_func(&ctx->instance->filter_funcs, ctx->instance->filter_mod, "*"); - if (filter_task && get_mmap) do_ptrace = 1; @@ -5097,6 +5135,9 @@ static void parse_record_options(int argc, "Did you mean 'record'?"); ctx->run_command = 1; } + if (ctx->user && !ctx->run_command) + warning("--user %s is ignored, no command is specified", + ctx->user); } static enum trace_type get_trace_cmd_type(enum trace_cmd cmd) @@ -5237,7 +5278,7 @@ static void record_trace(int argc, char **argv, } if (ctx->run_command) - run_cmd(type, (argc - optind) - 1, &argv[optind + 1]); + run_cmd(type, ctx->user, (argc - optind) - 1, &argv[optind + 1]); else { update_task_filter(); tracecmd_enable_tracing(); diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c index c658ede..e3e8186 100644 --- a/tracecmd/trace-usage.c +++ b/tracecmd/trace-usage.c @@ -58,6 +58,7 @@ static struct usage_help usage_help[] = { " --max-graph-depth limit function_graph depth\n" " --no-filter include trace-cmd threads in the trace\n" " --mmap used with -F or -P, save the traced process memory map into the trace.dat file\n" + " --user execute the specified [command ...] as given user\n" }, { "start",
A new trace-cmd record option is added: "--user". If a comnmand is specified as trce-cmd argument, it is executed in the context of the given user. Suggested-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> --- [ v3 changes: "--user" does not depend on option -F, it works with any command, specified as trace-cmd argument. v2 changes: - Check for errors in change_user(). If an error occurs while changing the user, the message is printed and the traced process is not executed. ] Documentation/trace-cmd-record.1.txt | 4 +++ tracecmd/trace-record.c | 47 ++++++++++++++++++++++++++-- tracecmd/trace-usage.c | 1 + 3 files changed, 49 insertions(+), 3 deletions(-)