From c6d097ba60f7ae7bfbd6b704db07d60e269386f8 Mon Sep 17 00:00:00 2001 From: Kevin Day Date: Sun, 9 Jan 2022 21:30:55 -0600 Subject: [PATCH] Bugfix: Fix problems exposed by unit testing and perform cleanups. f_capability_ambient_set() should not be testing for (!value_flag). Te flags pointer test should be "flags" and not "*flags". Explicitly cast capability to void * for cap_free(). Use "value" rather than "code" for "f_capability_value_t". Expand out CAP_IS_SUPPORTED() and CAP_AMBIENT_SUPPORTED() to explicitly set F_true and F_false. This is better than relying on the direct result of those macros. Make sure f_type support "weak" compiler attributes. Rename the compiler attributes to be more than just for visibility. Correct the function ordering. --- level_0/f_capability/c/capability.c | 87 ++++++++++++++++++---------------- level_0/f_capability/c/capability.h | 57 +++++++++++----------- level_0/f_memory/tests/c/test-memory.h | 8 ++-- level_0/f_type/c/type.h | 15 ++++-- 4 files changed, 90 insertions(+), 77 deletions(-) diff --git a/level_0/f_capability/c/capability.c b/level_0/f_capability/c/capability.c index 6257d99..e56864a 100644 --- a/level_0/f_capability/c/capability.c +++ b/level_0/f_capability/c/capability.c @@ -52,6 +52,12 @@ extern "C" { } #endif // _di_f_capability_clear_flag_ + #ifndef _di_f_capability_compare_ + f_status_t f_capability_compare(const f_capability_t capability1, const f_capability_t capability2, int *flags) { + return F_status_set_error(F_implemented_not); + } + #endif // _di_f_capability_compare_ + #ifndef _di_f_capability_copy_ f_status_t f_capability_copy(const f_capability_t source, f_capability_t *destination) { #ifndef _di_level_0_parameter_checking_ @@ -62,16 +68,6 @@ extern "C" { } #endif // _di_f_capability_copy_ - #ifndef _di_f_capability_compare_ - f_status_t f_capability_compare(const f_capability_t capability1, const f_capability_t capability2, int *flags) { - #ifndef _di_level_0_parameter_checking_ - if (!flags) return F_status_set_error(F_parameter); - #endif // _di_level_0_parameter_checking_ - - return F_status_set_error(F_implemented_not); - } - #endif // _di_f_capability_compare_ - #ifndef _di_f_capability_copy_external_ f_status_t f_capability_copy_external(const f_capability_t capability, const ssize_t max, void *external, ssize_t *size) { #ifndef _di_level_0_parameter_checking_ @@ -409,6 +405,7 @@ extern "C" { f_status_t f_capability_ambient_reset(void) { if (cap_reset_ambient() == -1) { + // The documentation doesn't explicitly describe this for "reset" but it can be implicitly inferred because they say "..all of the setting functions..". if (errno == EINVAL) return F_status_set_error(F_parameter); if (errno == ENOMEM) return F_status_set_error(F_memory_not); @@ -423,9 +420,6 @@ extern "C" { #ifndef _di_f_capability_ambient_set_ f_status_t f_capability_ambient_set(const f_capability_value_t value, const f_capability_flag_value_t value_flag) { - #ifndef _di_level_0_parameter_checking_ - if (!value_flag) return F_status_set_error(F_parameter); - #endif // _di_level_0_parameter_checking_ if (cap_set_ambient(value, value_flag) == -1) { if (errno == EINVAL) return F_status_set_error(F_parameter); @@ -477,28 +471,10 @@ extern "C" { } #endif // _di_f_capability_clear_flag_ - #ifndef _di_f_capability_copy_ - f_status_t f_capability_copy(const f_capability_t source, f_capability_t *destination) { - #ifndef _di_level_0_parameter_checking_ - if (!destination) return F_status_set_error(F_parameter); - #endif // _di_level_0_parameter_checking_ - - *destination = cap_dup(source); - - if (*destination) { - return F_none; - } - - if (errno == EINVAL) return F_status_set_error(F_parameter); - if (errno == ENOMEM) return F_status_set_error(F_memory_not); - - return F_status_set_error(F_failure); - } - #endif // _di_f_capability_copy_ - #ifndef _di_f_capability_compare_ f_status_t f_capability_compare(const f_capability_t capability1, const f_capability_t capability2, int *flags) { - if (*flags) { + + if (flags) { *flags = 0; } @@ -511,7 +487,7 @@ extern "C" { } if (result) { - if (*flags) { + if (flags) { *flags = result; } @@ -522,6 +498,25 @@ extern "C" { } #endif // _di_f_capability_compare_ + #ifndef _di_f_capability_copy_ + f_status_t f_capability_copy(const f_capability_t source, f_capability_t *destination) { + #ifndef _di_level_0_parameter_checking_ + if (!destination) return F_status_set_error(F_parameter); + #endif // _di_level_0_parameter_checking_ + + *destination = cap_dup(source); + + if (*destination) { + return F_none; + } + + if (errno == EINVAL) return F_status_set_error(F_parameter); + if (errno == ENOMEM) return F_status_set_error(F_memory_not); + + return F_status_set_error(F_failure); + } + #endif // _di_f_capability_copy_ + #ifndef _di_f_capability_copy_external_ f_status_t f_capability_copy_external(const f_capability_t capability, const ssize_t max, void *external, ssize_t *size) { #ifndef _di_level_0_parameter_checking_ @@ -571,7 +566,7 @@ extern "C" { if (!capability) return F_status_set_error(F_parameter); #endif // _di_level_0_parameter_checking_ - if (cap_free(*capability) == -1) { + if (cap_free((void *) *capability) == -1) { if (errno == EINVAL) return F_status_set_error(F_parameter); if (errno == ENOMEM) return F_status_set_error(F_memory_not); @@ -869,12 +864,12 @@ extern "C" { #ifndef _di_libcap_ #ifndef _di_f_capability_process_bound_drop_ - f_status_t f_capability_process_bound_drop(f_capability_value_t code, int *bound) { + f_status_t f_capability_process_bound_drop(f_capability_value_t value, int *bound) { #ifndef _di_level_0_parameter_checking_ if (!bound) return F_status_set_error(F_parameter); #endif // _di_level_0_parameter_checking_ - *bound = cap_drop_bound(code); + *bound = cap_drop_bound(value); if (*bound == -1) { if (errno == EINVAL) return F_status_set_error(F_parameter); @@ -889,12 +884,12 @@ extern "C" { #endif // _di_f_capability_process_bound_drop_ #ifndef _di_f_capability_process_bound_get_ - f_status_t f_capability_process_bound_get(f_capability_value_t code, int *bound) { + f_status_t f_capability_process_bound_get(f_capability_value_t value, int *bound) { #ifndef _di_level_0_parameter_checking_ if (!bound) return F_status_set_error(F_parameter); #endif // _di_level_0_parameter_checking_ - *bound = cap_get_bound(code); + *bound = cap_get_bound(value); if (*bound == -1) { return F_status_set_error(F_known_not); @@ -1021,7 +1016,12 @@ extern "C" { #ifndef _di_f_capability_supported_ambient_ bool f_capability_supported_ambient(void) { - return CAP_AMBIENT_SUPPORTED(); + + if (CAP_AMBIENT_SUPPORTED()) { + return F_true; + } + + return F_false; } #endif // _di_f_capability_supported_ambient_ @@ -1031,7 +1031,12 @@ extern "C" { #ifndef _di_f_capability_supported_code_ bool f_capability_supported_code(const f_capability_value_t code) { - return CAP_IS_SUPPORTED(code); + + if (CAP_IS_SUPPORTED(code)) { + return F_true; + } + + return F_false; } #endif // _di_f_capability_supported_code_ diff --git a/level_0/f_capability/c/capability.h b/level_0/f_capability/c/capability.h index 663db0b..2be347b 100644 --- a/level_0/f_capability/c/capability.h +++ b/level_0/f_capability/c/capability.h @@ -158,30 +158,6 @@ extern "C" { #endif // _di_f_capability_clear_flag_ /** - * Copy the capability structure. - * - * @param source - * The capability to copy from. - * @param destination - * The capability to copy to. - * This must be freed via f_capability_delete() when finished with. - * - * @return - * F_none on success. - * - * F_implemented_not (with error bit) if this function is not available (due to not having libcap support compiled in). - * F_memory_not (with error bit) if a out of memory. - * F_parameter (with error bit) if a parameter is invalid. - * - * F_failure (with error bit) on any other failure. - * - * @see cap_dup() - */ -#ifndef _di_f_capability_copy_ - extern f_status_t f_capability_copy(const f_capability_t source, f_capability_t *destination); -#endif // _di_f_capability_copy_ - -/** * Compare two capability structures. * * @param capability1 @@ -208,6 +184,30 @@ extern "C" { #endif // _di_f_capability_compare_ /** + * Copy the capability structure. + * + * @param source + * The capability to copy from. + * @param destination + * The capability to copy to. + * This must be freed via f_capability_delete() when finished with. + * + * @return + * F_none on success. + * + * F_implemented_not (with error bit) if this function is not available (due to not having libcap support compiled in). + * F_memory_not (with error bit) if a out of memory. + * F_parameter (with error bit) if a parameter is invalid. + * + * F_failure (with error bit) on any other failure. + * + * @see cap_dup() + */ +#ifndef _di_f_capability_copy_ + extern f_status_t f_capability_copy(const f_capability_t source, f_capability_t *destination); +#endif // _di_f_capability_copy_ + +/** * Copy an internally represented capability into an externally represented capability. * * @param capability @@ -266,6 +266,7 @@ extern "C" { * * @param capability * The capability to delete. + * Pointer address is set to 0 on success. * * @return * F_none on success. @@ -685,7 +686,7 @@ extern "C" { * * This will lower the specified bounding set capability if appropriate permission exist (the prevailing effective capability set must have a raised CAP_SETPCAP). * - * @param code + * @param value * The capability code to get the bound for. * @param bound * The determined bound value. @@ -703,13 +704,13 @@ extern "C" { * @see cap_drop_bound() */ #ifndef _di_f_capability_process_bound_drop_ - extern f_status_t f_capability_process_bound_drop(f_capability_value_t code, int *bound); + extern f_status_t f_capability_process_bound_drop(f_capability_value_t value, int *bound); #endif // _di_f_capability_process_bound_drop_ /** * Get the bound for the process. * - * @param code + * @param value * The capability code to get the bound for. * @param bound * The determined bound value. @@ -726,7 +727,7 @@ extern "C" { * @see cap_get_bound() */ #ifndef _di_f_capability_process_bound_get_ - extern f_status_t f_capability_process_bound_get(f_capability_value_t code, int *bound); + extern f_status_t f_capability_process_bound_get(f_capability_value_t value, int *bound); #endif // _di_f_capability_process_bound_get_ /** diff --git a/level_0/f_memory/tests/c/test-memory.h b/level_0/f_memory/tests/c/test-memory.h index d0ff7c6..f9e27ba 100644 --- a/level_0/f_memory/tests/c/test-memory.h +++ b/level_0/f_memory/tests/c/test-memory.h @@ -10,19 +10,19 @@ #ifndef _TEST__F_memory_h #define _TEST__F_memory_h -// libc includes +// libc includes. #include #include #include #include -// cmocka includes +// cmocka includes. #include -// fll-0 includes +// fll-0 includes. #include -// test includes +// test includes. #include "test-memory-adjust.h" #include "test-memory-delete.h" #include "test-memory-destroy.h" diff --git a/level_0/f_type/c/type.h b/level_0/f_type/c/type.h index 933639b..c3fa197 100644 --- a/level_0/f_type/c/type.h +++ b/level_0/f_type/c/type.h @@ -26,18 +26,25 @@ extern "C" { #endif /** - * Compiler-specific attribute visibility features. + * Compiler-specific attribute features. * * Use these macros for visibility-specific tweaks so that if these are not supported by any given compiler, then they can be easily disabled. * - * F_attribute_visibility_internal_d provides a way to make some functions effectively private. + * F_attribute_*: + * - visibility_hidden: Visibility is hidden. + * - visibility_internal: Visibility is private. + * - visibility_protected: Visibility is protected. + * - visibility_public: Visibility is public. + * - weak: Designate symbol is weak rather than global. */ -#ifndef _di_f_attribute_visibility_ +#ifndef _di_compiler_attributes_ #define F_attribute_visibility_hidden_d __attribute__((visibility("hidden"))) #define F_attribute_visibility_internal_d __attribute__((visibility("internal"))) #define F_attribute_visibility_protected_d __attribute__((visibility("protected"))) #define F_attribute_visibility_public_d __attribute__((visibility("default"))) -#endif // _di_f_attribute_visibility_ + + #define F_attribute_weak_d __attribute__((weak)) +#endif // _di_compiler_attributes_ /** * A status intended to be used as the return value status of some function or operation. -- 1.8.3.1