From: John Johansen Date: Sat, 8 Nov 2025 11:00:56 +0000 (-0800) Subject: apparmor: refactor/cleanup cred helper fns. X-Git-Tag: v7.0-rc1~35^2~15 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=acf2a94ac4734962742398bed0cde156baf48244;p=thirdparty%2Flinux.git apparmor: refactor/cleanup cred helper fns. aa_cred_raw_label() and cred_label() now do the same things so consolidate to cred_label() Document the crit section use and constraints better and refactor __begin_current_label_crit_section() into a base fn __begin_cred_crit_section() and a wrapper that calls the base with current cred. Reviewed-by: Georgia Garcia Signed-off-by: John Johansen --- diff --git a/security/apparmor/include/cred.h b/security/apparmor/include/cred.h index b028e4c13b6f..2b6098149b15 100644 --- a/security/apparmor/include/cred.h +++ b/security/apparmor/include/cred.h @@ -36,22 +36,6 @@ static inline void set_cred_label(const struct cred *cred, *blob = label; } -/** - * aa_cred_raw_label - obtain cred's label - * @cred: cred to obtain label from (NOT NULL) - * - * Returns: confining label - * - * does NOT increment reference count - */ -static inline struct aa_label *aa_cred_raw_label(const struct cred *cred) -{ - struct aa_label *label = cred_label(cred); - - AA_BUG(!label); - return label; -} - /** * aa_get_newest_cred_label - obtain the newest label on a cred * @cred: cred to obtain label from (NOT NULL) @@ -60,13 +44,13 @@ static inline struct aa_label *aa_cred_raw_label(const struct cred *cred) */ static inline struct aa_label *aa_get_newest_cred_label(const struct cred *cred) { - return aa_get_newest_label(aa_cred_raw_label(cred)); + return aa_get_newest_label(cred_label(cred)); } static inline struct aa_label *aa_get_newest_cred_label_condref(const struct cred *cred, bool *needput) { - struct aa_label *l = aa_cred_raw_label(cred); + struct aa_label *l = cred_label(cred); if (unlikely(label_is_stale(l))) { *needput = true; @@ -93,7 +77,7 @@ static inline void aa_put_label_condref(struct aa_label *l, bool needput) */ static inline struct aa_label *aa_current_raw_label(void) { - return aa_cred_raw_label(current_cred()); + return cred_label(current_cred()); } /** @@ -114,18 +98,80 @@ static inline struct aa_label *aa_get_current_label(void) return aa_get_label(l); } +/** + * __end_cred_crit_section - end crit section begun with __begin_... + * @label: label obtained from __begin_cred_crit_section + * @needput: output: bool set by __begin_cred_crit_section + * + * While the cred passed to __begin is guaranteed to not change + * and the cred and label could be passed here instead of needput + * using needput with a local var makes it easier for the compiler + * and processor to optimize and speculatively execute the comparison + * than chasing a pointer in the cred struct. + */ +static inline void __end_cred_crit_section(struct aa_label *label, + bool needput) +{ + if (unlikely(needput)) + aa_put_label(label); +} + +/** + * __begin_cred_crit_section - @cred's confining label + * @cred: current's cred to start a crit section on its label + * @needput: store whether the label needs to be put when ending crit section + * + * Returns: up to date confining label or the ns unconfined label (NOT NULL) + * + * safe to call inside locks + * + * The returned reference must be put with __end_cred_crit_section() + * This must NOT be used if the task cred could be updated within the + * critical section between + * __begin_cred_crit_section() .. __end_cred_crit_section() + * + * The crit section is an optimization to avoid having to get and put + * the newest version of the label. While the cred won't change and + * hence the label it contains won't change, the newest version of the + * label can. During the crit section the newest versions of the label + * will be used until the end of the crit section. + * + * If the label has not been updated at the start of the crit section + * no refcount is taken, the cred's refcount is enough to hold the + * label for the duration of the crit section. + * + * If the label has been updated then a refcount will be taken and the + * newest version of the label will be returned. While the cred label + * and the returned label could be compared at the end of the crit + * section, needput is used because it allows better optimization by + * the compiler and the processor's speculative execution. + */ +static inline struct aa_label *__begin_cred_crit_section(const struct cred *cred, + bool *needput) +{ + struct aa_label *label = cred_label(cred); + + if (label_is_stale(label)) { + *needput = true; + return aa_get_newest_label(label); + } + + *needput = false; + return label; +} + /** * __end_current_label_crit_section - end crit section begun with __begin_... * @label: label obtained from __begin_current_label_crit_section * @needput: output: bool set by __begin_current_label_crit_section * - * Returns: label to use for this crit section + * wrapper around __end_cred_crit_section() to pair nicely with + * __begin_current_label_crit_section() */ static inline void __end_current_label_crit_section(struct aa_label *label, bool needput) { - if (unlikely(needput)) - aa_put_label(label); + __end_cred_crit_section(label, needput); } /** @@ -157,15 +203,7 @@ static inline void end_current_label_crit_section(struct aa_label *label) */ static inline struct aa_label *__begin_current_label_crit_section(bool *needput) { - struct aa_label *label = aa_current_raw_label(); - - if (label_is_stale(label)) { - *needput = true; - return aa_get_newest_label(label); - } - - *needput = false; - return label; + return __begin_cred_crit_section(current_cred(), needput); } /**