On Mon, Oct 13, 2014 at 04:57:13PM +0200, Michal Sekletar wrote: > We need original socket_fd around otherwise label_get_child_mls_label fails > with > -EINVAL return code. > --- > src/core/execute.c | 58 > ++++++++++++++++++++++++++++-------------------------- > 1 file changed, 30 insertions(+), 28 deletions(-) > > diff --git a/src/core/execute.c b/src/core/execute.c > index b165b33..d64fa7c 100644 > --- a/src/core/execute.c > +++ b/src/core/execute.c > @@ -1578,6 +1578,36 @@ static int exec_child(ExecCommand *command, > } > } > I don't like the fact that now SELINUX context would be applied before closing of the fds, and other contexts after. I think it would be much nicer to simply extract getting the label, i.e. this part:
> + if (params->selinux_context_net && socket_fd >= 0) { > + _cleanup_free_ char *label = NULL; > + > + err = label_get_child_mls_label(socket_fd, > command->path, &label); > + if (err < 0) { > + *error = EXIT_SELINUX_CONTEXT; > + return err; > + } above the closing of the fds, and keep the rest where it was. > +#ifdef HAVE_SELINUX > + if (params->apply_permissions) { > + if (use_selinux()) { > + if (context->selinux_context) { > + err = setexeccon(context->selinux_context); > + if (err < 0 && > !context->selinux_context_ignore) { > + *error = EXIT_SELINUX_CONTEXT; > + return err; > + } > + } > + > + if (params->selinux_context_net && socket_fd >= 0) { > + _cleanup_free_ char *label = NULL; > + > + err = label_get_child_mls_label(socket_fd, > command->path, &label); > + if (err < 0) { > + *error = EXIT_SELINUX_CONTEXT; > + return err; > + } > + > + err = setexeccon(label); > + if (err < 0) { > + *error = EXIT_SELINUX_CONTEXT; > + return err; > + } > + } > + } > + } > +#endif BTW, it is quite confusing for me that setexeccon() is called twice. IIUC, this is because "only the descurity level is used from the information provided the peer". Is this correct? Could you add a comment to the code that explains this? Zbyszek > + > /* We repeat the fd closing here, to make sure that > * nothing is leaked from the PAM modules. Note that > * we are more aggressive this time since socket_fd > @@ -1665,34 +1695,6 @@ static int exec_child(ExecCommand *command, > } > #endif > > -#ifdef HAVE_SELINUX > - if (use_selinux()) { > - if (context->selinux_context) { > - err = setexeccon(context->selinux_context); > - if (err < 0 && > !context->selinux_context_ignore) { > - *error = EXIT_SELINUX_CONTEXT; > - return err; > - } > - } > - > - if (params->selinux_context_net && socket_fd >= 0) { > - _cleanup_free_ char *label = NULL; > - > - err = label_get_child_mls_label(socket_fd, > command->path, &label); > - if (err < 0) { > - *error = EXIT_SELINUX_CONTEXT; > - return err; > - } > - > - err = setexeccon(label); > - if (err < 0) { > - *error = EXIT_SELINUX_CONTEXT; > - return err; > - } > - } > - } > -#endif > - > #ifdef HAVE_APPARMOR > if (context->apparmor_profile && use_apparmor()) { > err = aa_change_onexec(context->apparmor_profile); _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel