From 5ce70e5bcd62e89b52485961c3699312ee4a7e0e Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 31 Dec 2013 22:35:54 -0500 Subject: [PATCH] Introduce cleanup functions for cap_free Unfortunately a different cleanup function is necessary per type, because cap_t** and char** are incompatible with void**. --- src/core/dbus-execute.c | 11 ++---- src/core/execute.c | 69 +++++++++++++++-------------------- src/libsystemd-bus/bus-dump.c | 7 ++-- src/shared/capability.c | 24 +++--------- src/shared/capability.h | 12 ++++++ 5 files changed, 55 insertions(+), 68 deletions(-) diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index b79a45645..4e9529708 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -29,6 +29,7 @@ #include "fileio.h" #include "execute.h" #include "dbus-execute.h" +#include "capability.h" BUS_DEFINE_PROPERTY_GET_ENUM(bus_property_get_exec_output, exec_output, ExecOutput); @@ -319,9 +320,8 @@ static int property_get_capabilities( sd_bus_error *error) { ExecContext *c = userdata; - char *t = NULL; + _cleanup_cap_free_charp_ char *t = NULL; const char *s; - int r; assert(bus); assert(reply); @@ -335,12 +335,7 @@ static int property_get_capabilities( if (!s) return -ENOMEM; - r = sd_bus_message_append(reply, "s", s); - - if (t) - cap_free(t); - - return r; + return sd_bus_message_append(reply, "s", s); } static int property_get_syscall_filter( diff --git a/src/core/execute.c b/src/core/execute.c index 39c0fed7a..4317afad8 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -650,14 +650,13 @@ static int enforce_groups(const ExecContext *context, const char *username, gid_ } static int enforce_user(const ExecContext *context, uid_t uid) { - int r; assert(context); /* Sets (but doesn't lookup) the uid and make sure we keep the * capabilities while doing so. */ if (context->capabilities) { - cap_t d; + _cleanup_cap_free_ cap_t d = NULL; static const cap_value_t bits[] = { CAP_SETUID, /* Necessary so that we can run setresuid() below */ CAP_SETPCAP /* Necessary so that we can set PR_SET_SECUREBITS later on */ @@ -677,23 +676,16 @@ static int enforce_user(const ExecContext *context, uid_t uid) { /* Second step: set the capabilities. This will reduce * the capabilities to the minimum we need. */ - if (!(d = cap_dup(context->capabilities))) + d = cap_dup(context->capabilities); + if (!d) return -errno; if (cap_set_flag(d, CAP_EFFECTIVE, ELEMENTSOF(bits), bits, CAP_SET) < 0 || - cap_set_flag(d, CAP_PERMITTED, ELEMENTSOF(bits), bits, CAP_SET) < 0) { - r = -errno; - cap_free(d); - return r; - } - - if (cap_set_proc(d) < 0) { - r = -errno; - cap_free(d); - return r; - } + cap_set_flag(d, CAP_PERMITTED, ELEMENTSOF(bits), bits, CAP_SET) < 0) + return -errno; - cap_free(d); + if (cap_set_proc(d) < 0) + return -errno; } /* Third step: actually set the uids */ @@ -2000,37 +1992,37 @@ void exec_context_dump(ExecContext *c, FILE* f, const char *prefix) { prefix, yes_no(c->tty_vhangup), prefix, yes_no(c->tty_vt_disallocate)); - if (c->std_output == EXEC_OUTPUT_SYSLOG || c->std_output == EXEC_OUTPUT_KMSG || c->std_output == EXEC_OUTPUT_JOURNAL || - c->std_output == EXEC_OUTPUT_SYSLOG_AND_CONSOLE || c->std_output == EXEC_OUTPUT_KMSG_AND_CONSOLE || c->std_output == EXEC_OUTPUT_JOURNAL_AND_CONSOLE || - c->std_error == EXEC_OUTPUT_SYSLOG || c->std_error == EXEC_OUTPUT_KMSG || c->std_error == EXEC_OUTPUT_JOURNAL || - c->std_error == EXEC_OUTPUT_SYSLOG_AND_CONSOLE || c->std_error == EXEC_OUTPUT_KMSG_AND_CONSOLE || c->std_error == EXEC_OUTPUT_JOURNAL_AND_CONSOLE) { - char *fac_str, *lvl_str; - int r; + if (c->std_output == EXEC_OUTPUT_SYSLOG || + c->std_output == EXEC_OUTPUT_KMSG || + c->std_output == EXEC_OUTPUT_JOURNAL || + c->std_output == EXEC_OUTPUT_SYSLOG_AND_CONSOLE || + c->std_output == EXEC_OUTPUT_KMSG_AND_CONSOLE || + c->std_output == EXEC_OUTPUT_JOURNAL_AND_CONSOLE || + c->std_error == EXEC_OUTPUT_SYSLOG || + c->std_error == EXEC_OUTPUT_KMSG || + c->std_error == EXEC_OUTPUT_JOURNAL || + c->std_error == EXEC_OUTPUT_SYSLOG_AND_CONSOLE || + c->std_error == EXEC_OUTPUT_KMSG_AND_CONSOLE || + c->std_error == EXEC_OUTPUT_JOURNAL_AND_CONSOLE) { - r = log_facility_unshifted_to_string_alloc(c->syslog_priority >> 3, &fac_str); - if (r < 0) - fac_str = NULL; + _cleanup_free_ char *fac_str = NULL, *lvl_str = NULL; - r = log_level_to_string_alloc(LOG_PRI(c->syslog_priority), &lvl_str); - if (r < 0) - lvl_str = NULL; + log_facility_unshifted_to_string_alloc(c->syslog_priority >> 3, &fac_str); + log_level_to_string_alloc(LOG_PRI(c->syslog_priority), &lvl_str); fprintf(f, "%sSyslogFacility: %s\n" "%sSyslogLevel: %s\n", prefix, strna(fac_str), prefix, strna(lvl_str)); - free(lvl_str); - free(fac_str); } if (c->capabilities) { - char *t; - if ((t = cap_to_text(c->capabilities, NULL))) { - fprintf(f, "%sCapabilities: %s\n", - prefix, t); - cap_free(t); - } + _cleanup_cap_free_charp_ char *t; + + t = cap_to_text(c->capabilities, NULL); + if (t) + fprintf(f, "%sCapabilities: %s\n", prefix, t); } if (c->secure_bits) @@ -2049,12 +2041,11 @@ void exec_context_dump(ExecContext *c, FILE* f, const char *prefix) { for (l = 0; l <= cap_last_cap(); l++) if (!(c->capability_bounding_set_drop & ((uint64_t) 1ULL << (uint64_t) l))) { - char *t; + _cleanup_cap_free_charp_ char *t; - if ((t = cap_to_name(l))) { + t = cap_to_name(l); + if (t) fprintf(f, " %s", t); - cap_free(t); - } } fputs("\n", f); diff --git a/src/libsystemd-bus/bus-dump.c b/src/libsystemd-bus/bus-dump.c index df7cf6893..78e7597ed 100644 --- a/src/libsystemd-bus/bus-dump.c +++ b/src/libsystemd-bus/bus-dump.c @@ -19,8 +19,6 @@ along with systemd; If not, see . ***/ -#include - #include "util.h" #include "capability.h" #include "strv.h" @@ -281,12 +279,15 @@ static void dump_capabilities( for (;;) { if (r > 0) { + _cleanup_cap_free_charp_ char *t; + if (n > 0) fputc(' ', f); if (n % 4 == 3) fputs("\n ", f); - fputs(cap_to_name(i), f); + t = cap_to_name(i); + fprintf(f, "%s", t); n++; } diff --git a/src/shared/capability.c b/src/shared/capability.c index 72c671cf8..f8ee1c683 100644 --- a/src/shared/capability.c +++ b/src/shared/capability.c @@ -37,21 +37,17 @@ #include "fileio.h" int have_effective_cap(int value) { - cap_t cap; + _cleanup_cap_free_ cap_t cap; cap_flag_value_t fv; - int r; cap = cap_get_proc(); if (!cap) return -errno; if (cap_get_flag(cap, value, CAP_EFFECTIVE, &fv) < 0) - r = -errno; + return -errno; else - r = fv == CAP_SET; - - cap_free(cap); - return r; + return fv == CAP_SET; } unsigned long cap_last_cap(void) { @@ -89,7 +85,7 @@ unsigned long cap_last_cap(void) { int capability_bounding_set_drop(uint64_t drop, bool right_now) { unsigned long i; - cap_t after_cap = NULL, temp_cap = NULL; + _cleanup_cap_free_ cap_t after_cap = NULL, temp_cap = NULL; cap_flag_value_t fv; int r; @@ -102,10 +98,8 @@ int capability_bounding_set_drop(uint64_t drop, bool right_now) { if (!after_cap) return -errno; - if (cap_get_flag(after_cap, CAP_SETPCAP, CAP_EFFECTIVE, &fv) < 0) { - cap_free(after_cap); + if (cap_get_flag(after_cap, CAP_SETPCAP, CAP_EFFECTIVE, &fv) < 0) return -errno; - } if (fv != CAP_SET) { static const cap_value_t v = CAP_SETPCAP; @@ -162,13 +156,7 @@ int capability_bounding_set_drop(uint64_t drop, bool right_now) { r = 0; finish: - if (temp_cap) - cap_free(temp_cap); - - if (after_cap) { - cap_set_proc(after_cap); - cap_free(after_cap); - } + cap_set_proc(after_cap); return r; } diff --git a/src/shared/capability.h b/src/shared/capability.h index 59469e56c..64f86410a 100644 --- a/src/shared/capability.h +++ b/src/shared/capability.h @@ -23,8 +23,20 @@ #include #include +#include + +#include "util.h" unsigned long cap_last_cap(void); int have_effective_cap(int value); int capability_bounding_set_drop(uint64_t drop, bool right_now); int capability_bounding_set_drop_usermode(uint64_t drop); + +DEFINE_TRIVIAL_CLEANUP_FUNC(cap_t, cap_free); +#define _cleanup_cap_free_ _cleanup_(cap_freep) + +static inline void cap_free_charpp(char **p) { + if (*p) + cap_free(*p); +} +#define _cleanup_cap_free_charp_ _cleanup_(cap_free_charpp) -- 2.30.2