Merge pull request #3639 from brauner/2021-01-28/fixes

src/lxc/attach.c

@@ -476,7 +476,7 @@ static int lxc_attach_set_environment(struct attach_context *ctx,
 		return log_warn(-1, "Failed to set environment variable");
 
 	/* Set container environment variables.*/
-	if (ctx && ctx->container && ctx->container->lxc_conf) {
+	if (ctx->container->lxc_conf) {
 		lxc_list_for_each(iterator, &ctx->container->lxc_conf->environment) {
 			char *env_tmp;
 
@@ -929,8 +929,7 @@ __noreturn static void do_attach(struct attach_payload *ap)
 		TRACE("Set %s LSM label to \"%s\"", ctx->lsm_ops->name, ctx->lsm_label);
 	}
 
-	if ((ctx->container && conf && conf->no_new_privs) ||
-	    (options->attach_flags & LXC_ATTACH_NO_NEW_PRIVS)) {
+	if (conf->no_new_privs || (options->attach_flags & LXC_ATTACH_NO_NEW_PRIVS)) {
 		ret = prctl(PR_SET_NO_NEW_PRIVS, prctl_arg(1), prctl_arg(0),
 			    prctl_arg(0), prctl_arg(0));
 		if (ret < 0)
@@ -939,7 +938,7 @@ __noreturn static void do_attach(struct attach_payload *ap)
 		TRACE("Set PR_SET_NO_NEW_PRIVS");
 	}
 
-	if (ctx->container && conf && conf->seccomp.seccomp) {
+	if (conf->seccomp.seccomp) {
 		ret = lxc_seccomp_load(conf);
 		if (ret < 0)
 			goto on_error;

src/lxc/cgroups/cgfsng.c

@@ -695,6 +695,8 @@ static struct hierarchy *add_hierarchy(struct hierarchy ***h, char **clist, char
 	int newentry;
 
 	new = zalloc(sizeof(*new));
+	if (!new)
+		return ret_set_errno(NULL, ENOMEM);
 	new->controllers = clist;
 	new->mountpoint = mountpoint;
 	new->container_base_path = container_base_path;
@@ -2662,8 +2664,8 @@ __cgfsng_ops static int cgfsng_set(struct cgroup_ops *ops,
 	struct hierarchy *h;
 	int ret = -1;
 
-	if (!ops)
-		return ret_set_errno(-1, ENOENT);
+	if (!ops || !key || !value || !name || !lxcpath)
+		return ret_errno(ENOENT);
 
 	controller = must_copy_string(key);
 	p = strchr(controller, '.');
@@ -2963,12 +2965,12 @@ __cgfsng_ops static bool cgfsng_setup_limits(struct cgroup_ops *ops,
 __cgfsng_ops static bool cgfsng_devices_activate(struct cgroup_ops *ops, struct lxc_handler *handler)
 {
 #ifdef HAVE_STRUCT_BPF_CGROUP_DEV_CTX
-	__do_bpf_program_free struct bpf_program *devices = NULL;
+	__do_bpf_program_free struct bpf_program *prog = NULL;
 	int ret;
 	struct lxc_conf *conf;
 	struct hierarchy *unified;
 	struct lxc_list *it;
-	struct bpf_program *devices_old;
+	struct bpf_program *prog_old;
 
 	if (!ops)
 		return ret_set_errno(false, ENOENT);
@@ -2988,18 +2990,18 @@ __cgfsng_ops static bool cgfsng_devices_activate(struct cgroup_ops *ops, struct
 	    !unified->container_full_path || lxc_list_empty(&conf->devices))
 		return true;
 
-	devices = bpf_program_new(BPF_PROG_TYPE_CGROUP_DEVICE);
-	if (!devices)
+	prog = bpf_program_new(BPF_PROG_TYPE_CGROUP_DEVICE);
+	if (!prog)
 		return log_error_errno(false, ENOMEM, "Failed to create new bpf program");
 
-	ret = bpf_program_init(devices);
+	ret = bpf_program_init(prog);
 	if (ret)
 		return log_error_errno(false, ENOMEM, "Failed to initialize bpf program");
 
 	lxc_list_for_each(it, &conf->devices) {
 		struct device_item *cur = it->elem;
 
-		ret = bpf_program_append_device(devices, cur);
+		ret = bpf_program_append_device(prog, cur);
 		if (ret)
 			return log_error_errno(false, ENOMEM, "Failed to add new rule to bpf device program: type %c, major %d, minor %d, access %s, allow %d, global_rule %d",
 					       cur->type,
@@ -3017,20 +3019,20 @@ __cgfsng_ops static bool cgfsng_devices_activate(struct cgroup_ops *ops, struct
 		      cur->global_rule);
 	}
 
-	ret = bpf_program_finalize(devices);
+	ret = bpf_program_finalize(prog);
 	if (ret)
 		return log_error_errno(false, ENOMEM, "Failed to finalize bpf program");
 
-	ret = bpf_program_cgroup_attach(devices, BPF_CGROUP_DEVICE,
+	ret = bpf_program_cgroup_attach(prog, BPF_CGROUP_DEVICE,
 					unified->container_limit_path,
 					BPF_F_ALLOW_MULTI);
 	if (ret)
 		return log_error_errno(false, ENOMEM, "Failed to attach bpf program");
 
 	/* Replace old bpf program. */
-	devices_old = move_ptr(ops->cgroup2_devices);
-	ops->cgroup2_devices = move_ptr(devices);
-	devices = move_ptr(devices_old);
+	prog_old = move_ptr(ops->cgroup2_devices);
+	ops->cgroup2_devices = move_ptr(prog);
+	prog = move_ptr(prog_old);
 #endif
 	return true;
 }
@@ -3283,6 +3285,8 @@ static int cg_hybrid_init(struct cgroup_ops *ops, bool relative, bool unprivileg
 		}
 
 		new = add_hierarchy(&ops->hierarchies, move_ptr(controller_list), move_ptr(mountpoint), move_ptr(base_cgroup), type);
+		if (!new)
+			return log_error_errno(-1, errno, "Failed to add cgroup hierarchy");
 		if (type == CGROUP2_SUPER_MAGIC && !ops->unified) {
 			if (unprivileged)
 				cg_unified_delegate(&new->cgroup2_chown);
@@ -3333,9 +3337,9 @@ static int cg_unified_init(struct cgroup_ops *ops, bool relative,
 {
 	__do_close int cgroup_root_fd = -EBADF;
 	__do_free char *base_cgroup = NULL, *controllers_path = NULL;
+	__do_free_string_list char **delegatable;
+	__do_free struct hierarchy *new = NULL;
 	int ret;
-	char **delegatable;
-	struct hierarchy *new;
 
 	ret = unified_cgroup_hierarchy();
 	if (ret == -ENOMEDIUM)
@@ -3375,10 +3379,13 @@ static int cg_unified_init(struct cgroup_ops *ops, bool relative,
 	 */
 
 	new = add_hierarchy(&ops->hierarchies,
-			    delegatable,
+			    move_ptr(delegatable),
 			    must_copy_string(DEFAULT_CGROUP_MOUNTPOINT),
 			    move_ptr(base_cgroup),
 			    CGROUP2_SUPER_MAGIC);
+	if (!new)
+		return log_error_errno(-1, errno, "Failed to add unified cgroup hierarchy");
+
 	if (unprivileged)
 		cg_unified_delegate(&new->cgroup2_chown);
 
@@ -3386,7 +3393,7 @@ static int cg_unified_init(struct cgroup_ops *ops, bool relative,
 		new->bpf_device_controller = 1;
 
 	ops->cgroup_layout = CGROUP_LAYOUT_UNIFIED;
-	ops->unified = new;
+	ops->unified = move_ptr(new);
 
 	return CGROUP2_SUPER_MAGIC;
 }
@@ -3438,11 +3445,10 @@ struct cgroup_ops *cgfsng_ops_init(struct lxc_conf *conf)
 {
 	__do_free struct cgroup_ops *cgfsng_ops = NULL;
 
-	cgfsng_ops = malloc(sizeof(struct cgroup_ops));
+	cgfsng_ops = zalloc(sizeof(struct cgroup_ops));
 	if (!cgfsng_ops)
 		return ret_set_errno(NULL, ENOMEM);
 
-	memset(cgfsng_ops, 0, sizeof(struct cgroup_ops));
 	cgfsng_ops->cgroup_layout = CGROUP_LAYOUT_UNKNOWN;
 
 	if (cg_init(cgfsng_ops, conf))

src/lxc/cgroups/cgroup2_devices.c

@@ -27,6 +27,7 @@
 
 lxc_log_define(cgroup2_devices, cgroup);
 
+#define BPF_LOG_BUF_SIZE (1 << 23) /* 8MB */
 #ifndef BPF_LOG_LEVEL1
 #define BPF_LOG_LEVEL1 1
 #endif
@@ -52,8 +53,9 @@ static int bpf_program_add_instructions(struct bpf_program *prog,
 	new_insn = realloc(prog->instructions, sizeof(struct bpf_insn) * (count + prog->n_instructions));
 	if (!new_insn)
 		return log_error_errno(-1, ENOMEM, "Failed to reallocate bpf cgroup program");
-
 	prog->instructions = new_insn;
+	memset(prog->instructions + prog->n_instructions, 0,
+	       sizeof(struct bpf_insn) * count);
 	memcpy(prog->instructions + prog->n_instructions, instructions,
 	       sizeof(struct bpf_insn) * count);
 	prog->n_instructions += count;
@@ -179,7 +181,7 @@ struct bpf_program *bpf_program_new(uint32_t prog_type)
 
 	prog = zalloc(sizeof(struct bpf_program));
 	if (!prog)
-		return NULL;
+		return ret_set_errno(NULL, ENOMEM);
 
 	prog->prog_type = prog_type;
 	prog->kernel_fd = -EBADF;
@@ -245,10 +247,10 @@ int bpf_program_append_device(struct bpf_program *prog, struct device_item *devi
 	if (device_type > 0)
 		jump_nr++;
 
-	if (device->major != -1)
+	if (device->major >= 0)
 		jump_nr++;
 
-	if (device->minor != -1)
+	if (device->minor >= 0)
 		jump_nr++;
 
 	if (!bpf_device_all_access(access_mask)) {
@@ -320,30 +322,36 @@ int bpf_program_finalize(struct bpf_program *prog)
 	return bpf_program_add_instructions(prog, ins, ARRAY_SIZE(ins));
 }
 
-static int bpf_program_load_kernel(struct bpf_program *prog, char *log_buf,
-				   __u32 log_size, __u32 log_level)
+static int bpf_program_load_kernel(struct bpf_program *prog)
 {
-	union bpf_attr attr;
+	__do_free char *log_buf = NULL;
+	__u32 log_level = 0, log_size = 0;
+	union bpf_attr *attr;
 
-	if ((log_size != 0 && !log_buf) || (log_size == 0 && log_buf))
-		return ret_errno(EINVAL);
-
-	if (prog->kernel_fd >= 0) {
-		memset(log_buf, 0, log_size);
+	if (prog->kernel_fd >= 0)
 		return 0;
+
+	if (lxc_log_trace()) {
+		log_buf = zalloc(BPF_LOG_BUF_SIZE);
+		if (!log_buf) {
+			WARN("Failed to allocate bpf log buffer");
+		} else {
+			log_level = BPF_LOG_LEVEL;
+			log_size = BPF_LOG_BUF_SIZE;
+		}
 	}
 
-	attr = (union bpf_attr){
+	attr = &(union bpf_attr){
 		.prog_type	= prog->prog_type,
-		.insns		= PTR_TO_UINT64(prog->instructions),
+		.insns		= PTR_TO_U64(prog->instructions),
 		.insn_cnt	= prog->n_instructions,
-		.license	= PTR_TO_UINT64("GPL"),
-		.log_buf	= PTR_TO_UINT64(log_buf),
+		.license	= PTR_TO_U64("GPL"),
+		.log_buf	= PTR_TO_U64(log_buf),
 		.log_level	= log_level,
 		.log_size	= log_size,
 	};
 
-	prog->kernel_fd = bpf(BPF_PROG_LOAD, &attr, sizeof(attr));
+	prog->kernel_fd = bpf(BPF_PROG_LOAD, attr, sizeof(*attr));
 	if (prog->kernel_fd < 0)
 		return log_error_errno(-1, errno, "Failed to load bpf program: %s",
 				       log_buf ?: "(null)");
@@ -357,7 +365,7 @@ int bpf_program_cgroup_attach(struct bpf_program *prog, int type,
 {
 	__do_close int fd = -EBADF;
 	__do_free char *copy = NULL;
-	union bpf_attr attr;
+	union bpf_attr *attr;
 	int ret;
 
 	if (!path || !prog)
@@ -377,7 +385,7 @@ int bpf_program_cgroup_attach(struct bpf_program *prog, int type,
 			return true;
 	}
 
-	ret = bpf_program_load_kernel(prog, NULL, 0, 0);
+	ret = bpf_program_load_kernel(prog);
 	if (ret < 0)
 		return log_error_errno(-1, ret, "Failed to load bpf program");
 
@@ -389,14 +397,14 @@ int bpf_program_cgroup_attach(struct bpf_program *prog, int type,
 	if (fd < 0)
 		return log_error_errno(-1, errno, "Failed to open cgroup path %s", path);
 
-	attr = (union bpf_attr){
+	attr = &(union bpf_attr){
 		.attach_type	= type,
 		.target_fd	= fd,
 		.attach_bpf_fd	= prog->kernel_fd,
 		.attach_flags	= flags,
 	};
 
-	ret = bpf(BPF_PROG_ATTACH, &attr, sizeof(attr));
+	ret = bpf(BPF_PROG_ATTACH, attr, sizeof(*attr));
 	if (ret < 0)
 		return log_error_errno(-1, errno, "Failed to attach bpf program");
 
@@ -425,15 +433,15 @@ int bpf_program_cgroup_detach(struct bpf_program *prog)
 			return log_error_errno(-1, errno, "Failed to open attach cgroup %s",
 					       prog->attached_path);
 	} else {
-		union bpf_attr attr;
+		union bpf_attr *attr;
 
-		attr = (union bpf_attr){
+		attr = &(union bpf_attr){
 			.attach_type	= prog->attached_type,
 			.target_fd	= fd,
 			.attach_bpf_fd	= prog->kernel_fd,
 		};
 
-		ret = bpf(BPF_PROG_DETACH, &attr, sizeof(attr));
+		ret = bpf(BPF_PROG_DETACH, attr, sizeof(*attr));
 		if (ret < 0)
 			return log_error_errno(-1, errno, "Failed to detach bpf program from cgroup %s",
 					       prog->attached_path);
@@ -536,11 +544,15 @@ bool bpf_devices_cgroup_supported(void)
 	if (!prog)
 		return log_trace(false, "Failed to allocate new bpf device cgroup program");
 
+	ret = bpf_program_init(prog);
+	if (ret)
+		return log_error_errno(false, ENOMEM, "Failed to initialize bpf program");
+
 	ret = bpf_program_add_instructions(prog, dummy, ARRAY_SIZE(dummy));
 	if (ret < 0)
 		return log_trace(false, "Failed to add new instructions to bpf device cgroup program");
 
-	ret = bpf_program_load_kernel(prog, NULL, 0, 0);
+	ret = bpf_program_load_kernel(prog);
 	if (ret < 0)
 		return log_trace(false, "Failed to load new bpf device cgroup program");
 

src/lxc/cgroups/cgroup2_devices.h

@@ -19,6 +19,7 @@
 #include "compiler.h"
 #include "conf.h"
 #include "config.h"
+#include "memory_utils.h"
 #include "syscall_numbers.h"
 
 #ifdef HAVE_STRUCT_BPF_CGROUP_DEV_CTX
@@ -65,14 +66,6 @@ __hidden extern void bpf_program_free(struct bpf_program *prog);
 __hidden extern void bpf_device_program_free(struct cgroup_ops *ops);
 __hidden extern bool bpf_devices_cgroup_supported(void);
 
-static inline void __auto_bpf_program_free__(struct bpf_program **prog)
-{
-	if (*prog) {
-		bpf_program_free(*prog);
-		*prog = NULL;
-	}
-}
-
 __hidden extern int bpf_list_add_device(struct lxc_conf *conf, struct device_item *device);
 
 #else /* !HAVE_STRUCT_BPF_CGROUP_DEV_CTX */
@@ -129,10 +122,6 @@ static inline bool bpf_devices_cgroup_supported(void)
 	return false;
 }
 
-static inline void __auto_bpf_program_free__(struct bpf_program **prog)
-{
-}
-
 static inline int bpf_list_add_device(struct lxc_conf *conf,
 				      struct device_item *device)
 {
@@ -141,7 +130,7 @@ static inline int bpf_list_add_device(struct lxc_conf *conf,
 }
 #endif /* !HAVE_STRUCT_BPF_CGROUP_DEV_CTX */
 
-#define __do_bpf_program_free \
-	__attribute__((__cleanup__(__auto_bpf_program_free__)))
+define_cleanup_function(struct bpf_program *, bpf_program_free);
+#define __do_bpf_program_free call_cleaner(bpf_program_free)
 
 #endif /* __LXC_CGROUP2_DEVICES_H */

src/lxc/log.c

@@ -46,7 +46,7 @@
 int lxc_log_fd = -EBADF;
 static bool wants_syslog = false;
 static int lxc_quiet_specified;
-int lxc_log_use_global_fd;
+bool lxc_log_use_global_fd = false;
 static int lxc_loglevel_specified;
 
 static char log_prefix[LXC_LOG_PREFIX_SIZE] = "lxc";
@@ -92,6 +92,20 @@ static const char *lxc_log_get_container_name(void)
 	return log_vmname;
 }
 
+int lxc_log_get_fd(void)
+{
+	int fd_log = -EBADF;
+
+#ifndef NO_LXC_CONF
+	if (current_config && !lxc_log_use_global_fd)
+		fd_log = current_config->logfd;
+#endif
+	if (fd_log < 0)
+		fd_log = lxc_log_fd;
+
+	return fd_log;
+}
+
 static char *lxc_log_get_va_msg(struct lxc_log_event *event)
 {
 	__do_free char *msg = NULL;
@@ -659,7 +673,7 @@ int lxc_log_init(struct lxc_log *log)
 		if (ret < 0)
 			return log_error_errno(-1, errno, "Failed to enable logfile");
 
-		lxc_log_use_global_fd = 1;
+		lxc_log_use_global_fd = true;
 	} else {
 		/* if no name was specified, there nothing to do */
 		if (!log->name)
@@ -762,9 +776,18 @@ int lxc_log_set_level(int *dest, int level)
 	return 0;
 }
 
-inline int lxc_log_get_level(void)
+int lxc_log_get_level(void)
 {
-	return lxc_log_category_lxc.priority;
+	int level = LXC_LOG_LEVEL_NOTSET;
+
+#ifndef NO_LXC_CONF
+	if (current_config)
+		level = current_config->loglevel;
+#endif
+	if (level == LXC_LOG_LEVEL_NOTSET)
+		level = lxc_log_category_lxc.priority;
+
+	return level;
 }
 
 bool lxc_log_has_valid_level(void)

src/lxc/log.h

@@ -85,7 +85,7 @@ struct lxc_log_category {
 };
 
 #ifndef NO_LXC_CONF
-extern int lxc_log_use_global_fd;
+extern bool lxc_log_use_global_fd;
 #endif
 
 /*
@@ -568,11 +568,16 @@ __hidden extern void lxc_log_syslog_enable(void);
 __hidden extern void lxc_log_syslog_disable(void);
 __hidden extern int lxc_log_set_level(int *dest, int level);
 __hidden extern int lxc_log_get_level(void);
+static inline bool lxc_log_trace(void)
+{
+	return lxc_log_get_level() <= LXC_LOG_LEVEL_TRACE;
+}
 __hidden extern bool lxc_log_has_valid_level(void);
 __hidden extern int lxc_log_set_file(int *fd, const char *fname);
 __hidden extern const char *lxc_log_get_file(void);
 __hidden extern void lxc_log_set_prefix(const char *prefix);
 __hidden extern const char *lxc_log_get_prefix(void);
 __hidden extern void lxc_log_options_no_override(void);
+__hidden extern int lxc_log_get_fd(void);
 
 #endif /* __LXC_LOG_H */

src/lxc/macro.h

@@ -602,7 +602,8 @@ enum {
 #define PTR_TO_PID(p) ((pid_t)((intptr_t)(p)))
 #define PID_TO_PTR(u) ((void *)((intptr_t)(u)))
 
-#define PTR_TO_UINT64(p) ((uint64_t)((intptr_t)(p)))
+#define PTR_TO_UINT64(p) ((uint64_t)((uintptr_t)(p)))
+#define PTR_TO_U64(p) ((__u64)((uintptr_t)(p)))
 
 #define UINT_TO_PTR(u) ((void *) ((uintptr_t) (u)))
 #define PTR_TO_USHORT(p) ((unsigned short)((uintptr_t)(p)))

src/lxc/seccomp.c

@@ -1260,16 +1260,18 @@ int lxc_seccomp_load(struct lxc_conf *conf)
 /* After load seccomp filter into the kernel successfully, export the current seccomp
  * filter to log file */
 #if HAVE_SCMP_FILTER_CTX
-	if ((lxc_log_get_level() <= LXC_LOG_LEVEL_TRACE ||
-	     conf->loglevel <= LXC_LOG_LEVEL_TRACE) &&
-	    lxc_log_fd >= 0) {
-		ret = seccomp_export_pfc(conf->seccomp.seccomp_ctx, lxc_log_fd);
-		/* Just give an warning when export error */
+	if (lxc_log_trace()) {
+		int fd_log;
+
+		fd_log = lxc_log_get_fd();
+		if (fd_log >= 0) {
+			ret = seccomp_export_pfc(conf->seccomp.seccomp_ctx, fd_log);
 			if (ret < 0) {
 				errno = -ret;
 				SYSWARN("Failed to export seccomp filter to log file");
 			}
 		}
+	}
 #endif
 
 #if HAVE_DECL_SECCOMP_NOTIFY_FD