From 5f00c5684f96c93a22840f7241ee444b9a632b1e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 10 Oct 2018 11:07:54 +0200 Subject: [PATCH 1/3] capability: introduce CAP_TO_MASK_CORRECTED() macro replacing CAP_TO_MASK() linux/capability.h's CAP_TO_MASK potentially shifts a signed int "1" (i.e. 32bit wide) left by 31 which means it becomes negative. That's just weird, and ubsan complains about it. Let's introduce our own macro CAP_TO_MASK_CORRECTED which doesn't fall into this trap, and make use of it. Fixes: #10347 --- src/basic/capability-util.h | 4 ++++ src/libsystemd/sd-bus/bus-creds.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/basic/capability-util.h b/src/basic/capability-util.h index 4a4a86093a..59591d4b52 100644 --- a/src/basic/capability-util.h +++ b/src/basic/capability-util.h @@ -39,3 +39,7 @@ static inline bool cap_test_all(uint64_t caps) { } bool ambient_capabilities_supported(void); + +/* Identical to linux/capability.h's CAP_TO_MASK(), but uses an unsigned 1U instead of a signed 1 for shifting left, in + * order to avoid complaints about shifting a signed int left by 31 bits, which would make it negative. */ +#define CAP_TO_MASK_CORRECTED(x) (1U << ((x) & 31U)) diff --git a/src/libsystemd/sd-bus/bus-creds.c b/src/libsystemd/sd-bus/bus-creds.c index 738c922ce0..b454270d3f 100644 --- a/src/libsystemd/sd-bus/bus-creds.c +++ b/src/libsystemd/sd-bus/bus-creds.c @@ -661,7 +661,7 @@ static int has_cap(sd_bus_creds *c, unsigned offset, int capability) { sz = DIV_ROUND_UP(cap_last_cap(), 32U); - return !!(c->capability[offset * sz + CAP_TO_INDEX(capability)] & CAP_TO_MASK(capability)); + return !!(c->capability[offset * sz + CAP_TO_INDEX((uint32_t) capability)] & CAP_TO_MASK_CORRECTED((uint32_t) capability)); } _public_ int sd_bus_creds_has_effective_cap(sd_bus_creds *c, int capability) { From 3cae6c21e732fd46ff024d6625243d88ef6377ed Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 10 Oct 2018 11:12:22 +0200 Subject: [PATCH 2/3] sd-bus: use size_t when dealing with memory offsets --- src/libsystemd/sd-bus/bus-creds.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd/sd-bus/bus-creds.c b/src/libsystemd/sd-bus/bus-creds.c index b454270d3f..722366a6fd 100644 --- a/src/libsystemd/sd-bus/bus-creds.c +++ b/src/libsystemd/sd-bus/bus-creds.c @@ -649,7 +649,7 @@ _public_ int sd_bus_creds_get_description(sd_bus_creds *c, const char **ret) { return 0; } -static int has_cap(sd_bus_creds *c, unsigned offset, int capability) { +static int has_cap(sd_bus_creds *c, size_t offset, int capability) { size_t sz; assert(c); From 92a40e20bf970c3ded8a50fbeeae882a7b970c9a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 10 Oct 2018 11:12:54 +0200 Subject: [PATCH 3/3] sd-bus: call cap_last_cap() only once in has_cap() Also, use the same type everywhere for dealing with it. --- src/libsystemd/sd-bus/bus-creds.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-creds.c b/src/libsystemd/sd-bus/bus-creds.c index 722366a6fd..a6dda16876 100644 --- a/src/libsystemd/sd-bus/bus-creds.c +++ b/src/libsystemd/sd-bus/bus-creds.c @@ -650,16 +650,19 @@ _public_ int sd_bus_creds_get_description(sd_bus_creds *c, const char **ret) { } static int has_cap(sd_bus_creds *c, size_t offset, int capability) { + unsigned long lc; size_t sz; assert(c); assert(capability >= 0); assert(c->capability); - if ((unsigned) capability > cap_last_cap()) + lc = cap_last_cap(); + + if ((unsigned long) capability > lc) return 0; - sz = DIV_ROUND_UP(cap_last_cap(), 32U); + sz = DIV_ROUND_UP(lc, 32LU); return !!(c->capability[offset * sz + CAP_TO_INDEX((uint32_t) capability)] & CAP_TO_MASK_CORRECTED((uint32_t) capability)); }