[CRIU] [PATCH] lsm: add a --lsm-profile flag
Pavel Emelyanov
xemul at parallels.com
Fri Dec 4 04:35:22 PST 2015
On 12/03/2015 08:58 PM, Tycho Andersen wrote:
> In LXD, we use the container name in the LSM profile. If the container name
> is changed on migrate (on the host side), we want to use a different LSM
> profile name (a. la. --cgroup-root). This flag adds that support.
Hi, Tycho.
Hopefully you don't plan to have it in 1.8 :) The release is in 3 days
2 of which are weekends.
And some more comments inline.
> Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
> ---
> cr-restore.c | 36 +++++++++++++++++++++---------------
> crtools.c | 6 ++++++
> include/cr_options.h | 5 +++++
> include/lsm.h | 3 ++-
> lsm.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++--
> 5 files changed, 81 insertions(+), 18 deletions(-)
>
> diff --git a/cr-restore.c b/cr-restore.c
> index 1c0c641..3c636b9 100644
> --- a/cr-restore.c
> +++ b/cr-restore.c
> @@ -2827,29 +2827,35 @@ static int sigreturn_restore(pid_t pid, CoreEntry *core)
> if (!creds)
> goto err;
>
> - if (creds->lsm_profile) {
> - char *rendered;
> + if (creds->lsm_profile || opts.lsm_supplied) {
> + char *rendered, *profile;
> int ret;
>
> - if (validate_lsm(creds) < 0)
> + profile = creds->lsm_profile;
> + if (opts.lsm_supplied)
> + profile = opts.lsm_profile;
> +
> + if (validate_lsm(profile) < 0)
> return -1;
>
> - ret = render_lsm_profile(creds->lsm_profile, &rendered);
> - if (ret < 0) {
> - goto err_nv;
> - }
> + if (profile) {
Is this if required? I cannot find the combination of creds->lsm_profile and
opts.lsm_profile which can lead to NULL here :(
> + ret = render_lsm_profile(profile, &rendered);
> + if (ret < 0) {
> + goto err_nv;
> + }
> +
> + lsm_pos = rst_mem_cpos(RM_PRIVATE);
> + lsm_profile_len = strlen(rendered);
> + lsm = rst_mem_alloc(lsm_profile_len + 1, RM_PRIVATE);
> + if (!lsm) {
> + xfree(rendered);
> + goto err_nv;
> + }
>
> - lsm_pos = rst_mem_cpos(RM_PRIVATE);
> - lsm_profile_len = strlen(rendered);
> - lsm = rst_mem_alloc(lsm_profile_len + 1, RM_PRIVATE);
> - if (!lsm) {
> + strncpy(lsm, rendered, lsm_profile_len);
> xfree(rendered);
> - goto err_nv;
> }
>
> - strncpy(lsm, rendered, lsm_profile_len);
> - xfree(rendered);
> -
> }
>
> if (seccomp_filters_get_rst_pos(core, &n_seccomp_filters, &seccomp_filter_pos) < 0)
> diff --git a/crtools.c b/crtools.c
> index d3812a1..68756a0 100644
> --- a/crtools.c
> +++ b/crtools.c
> @@ -41,6 +41,7 @@
> #include "security.h"
> #include "irmap.h"
> #include "fault-injection.h"
> +#include "lsm.h"
>
> #include "setproctitle.h"
>
> @@ -253,6 +254,7 @@ int main(int argc, char *argv[], char *envp[])
> { "freeze-cgroup", required_argument, 0, 1068 },
> { "ghost-limit", required_argument, 0, 1069 },
> { "irmap-scan-path", required_argument, 0, 1070 },
> + { "lsm-profile", required_argument, 0, 1071 },
> { },
> };
>
> @@ -498,6 +500,10 @@ int main(int argc, char *argv[], char *envp[])
> if (irmap_scan_path_add(optarg))
> return -1;
> break;
> + case 1071:
> + if (parse_lsm_arg(optarg) < 0)
> + return -1;
> + break;
> case 'M':
> {
> char *aux;
> diff --git a/include/cr_options.h b/include/cr_options.h
> index eac7283..f31078a 100644
> --- a/include/cr_options.h
> +++ b/include/cr_options.h
> @@ -5,6 +5,8 @@
>
> #include "list.h"
>
> +#include "protobuf/inventory.pb-c.h"
> +
> /*
> * CPU capability options.
> */
> @@ -95,6 +97,9 @@ struct cr_options {
> bool overlayfs;
> size_t ghost_limit;
> struct list_head irmap_scan_paths;
> + bool lsm_supplied;
> + Lsmtype force_lsm;
This field is unused in the patch.
> + char *lsm_profile;
> };
>
> extern struct cr_options opts;
> diff --git a/include/lsm.h b/include/lsm.h
> index 1409acd..bd13ef7 100644
> --- a/include/lsm.h
> +++ b/include/lsm.h
> @@ -23,7 +23,7 @@ extern int collect_lsm_profile(pid_t, CredsEntry *);
> * Validate that the LSM profiles can be correctly applied (must happen after
> * pstree is set up).
> */
> -int validate_lsm(CredsEntry *ce);
> +int validate_lsm(char *profile);
>
> /*
> * Render the profile name in the way that the LSM wants it written to
> @@ -31,4 +31,5 @@ int validate_lsm(CredsEntry *ce);
> */
> int render_lsm_profile(char *profile, char **val);
>
> +extern int parse_lsm_arg(char *arg);
> #endif /* __CR_LSM_H__ */
> diff --git a/lsm.c b/lsm.c
> index c960611..dac1094 100644
> --- a/lsm.c
> +++ b/lsm.c
> @@ -7,6 +7,7 @@
> #include "config.h"
> #include "pstree.h"
> #include "util.h"
> +#include "cr_options.h"
>
> #include "protobuf.h"
> #include "protobuf/inventory.pb-c.h"
> @@ -106,6 +107,9 @@ static int selinux_get_label(pid_t pid, char **output)
>
> void kerndat_lsm(void)
> {
> + if (name)
> + return;
Please, explain why this if is required.
> +
> if (access("/sys/kernel/security/apparmor", F_OK) == 0) {
> get_label = apparmor_get_label;
> lsmtype = LSMTYPE__APPARMOR;
> @@ -156,7 +160,7 @@ int collect_lsm_profile(pid_t pid, CredsEntry *ce)
> // in inventory.c
> extern Lsmtype image_lsm;
>
> -int validate_lsm(CredsEntry *ce)
> +int validate_lsm(char *lsm_profile)
> {
> if (image_lsm == LSMTYPE__NO_LSM || image_lsm == lsmtype)
> return 0;
> @@ -166,7 +170,7 @@ int validate_lsm(CredsEntry *ce)
> * specified an LSM profile. If not, we won't restore anything anyway,
> * so it's fine.
> */
> - if (ce->lsm_profile) {
> + if (lsm_profile) {
> pr_err("mismatched lsm types and lsm profile specified\n");
> return -1;
> }
> @@ -197,3 +201,44 @@ int render_lsm_profile(char *profile, char **val)
>
> return 0;
> }
> +
> +int parse_lsm_arg(char *arg)
> +{
> + char *aux;
> +
> + kerndat_lsm();
> +
> + aux = strchr(arg, ':');
> + if (aux == NULL) {
> + pr_err("invalid argument %s for --lsm-profile", arg);
> + return -1;
> + }
> +
> + *aux = '\0';
> + aux++;
> +
> + if (strcmp(arg, "apparmor") == 0) {
> + if (lsmtype != LSMTYPE__APPARMOR) {
> + pr_err("apparmor LSM specified but apparmor not supported by kernel\n");
> + return -1;
> + }
> +
> + opts.lsm_profile = aux;
> + } else if (strcmp(arg, "selinux") == 0) {
> + if (lsmtype != LSMTYPE__SELINUX) {
> + pr_err("selinux LSM specified but selinux not supported by kernel\n");
> + return -1;
> + }
> +
> + opts.lsm_profile = aux;
> + } else if (strcmp(arg, "none") == 0) {
> + opts.lsm_profile = NULL;
> + } else {
> + pr_err("unknown lsm %s\n", arg);
> + return -1;
> + }
> +
> + opts.lsm_supplied = true;
> +
> + return 0;
> +}
>
More information about the CRIU
mailing list