From 6fba89813ccf333d2bc4d5caea04cd5f3c39eb50 Mon Sep 17 00:00:00 2001 From: Casey Schaufler Date: Wed, 23 Oct 2024 14:21:54 -0700 Subject: lsm: ensure the correct LSM context releaser Add a new lsm_context data structure to hold all the information about a "security context", including the string, its size and which LSM allocated the string. The allocation information is necessary because LSMs have different policies regarding the lifecycle of these strings. SELinux allocates and destroys them on each use, whereas Smack provides a pointer to an entry in a list that never goes away. Update security_release_secctx() to use the lsm_context instead of a (char *, len) pair. Change its callers to do likewise. The LSMs supporting this hook have had comments added to remind the developer that there is more work to be done. The BPF security module provides all LSM hooks. While there has yet to be a known instance of a BPF configuration that uses security contexts, the possibility is real. In the existing implementation there is potential for multiple frees in that case. Cc: linux-integrity@vger.kernel.org Cc: netdev@vger.kernel.org Cc: audit@vger.kernel.org Cc: netfilter-devel@vger.kernel.org To: Pablo Neira Ayuso Cc: linux-nfs@vger.kernel.org Cc: Todd Kjos Signed-off-by: Casey Schaufler [PM: subject tweak] Signed-off-by: Paul Moore --- security/apparmor/include/secid.h | 2 +- security/apparmor/secid.c | 11 +++++++++-- security/security.c | 8 ++++---- security/selinux/hooks.c | 11 +++++++++-- 4 files changed, 23 insertions(+), 9 deletions(-) (limited to 'security') diff --git a/security/apparmor/include/secid.h b/security/apparmor/include/secid.h index f6a515640950..1bcde2f45f24 100644 --- a/security/apparmor/include/secid.h +++ b/security/apparmor/include/secid.h @@ -29,7 +29,7 @@ int apparmor_secid_to_secctx(u32 secid, char **secdata, u32 *seclen); int apparmor_lsmprop_to_secctx(struct lsm_prop *prop, char **secdata, u32 *seclen); int apparmor_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid); -void apparmor_release_secctx(char *secdata, u32 seclen); +void apparmor_release_secctx(struct lsm_context *cp); int aa_alloc_secid(struct aa_label *label, gfp_t gfp); diff --git a/security/apparmor/secid.c b/security/apparmor/secid.c index 47dc08fc583e..43e4dab7f6e6 100644 --- a/security/apparmor/secid.c +++ b/security/apparmor/secid.c @@ -106,9 +106,16 @@ int apparmor_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid) return 0; } -void apparmor_release_secctx(char *secdata, u32 seclen) +void apparmor_release_secctx(struct lsm_context *cp) { - kfree(secdata); + /* + * stacking scaffolding: + * When it is possible for more than one LSM to provide a + * release hook, do this check: + * if (cp->id == LSM_ID_APPARMOR || cp->id == LSM_ID_UNDEF) + */ + + kfree(cp->context); } /** diff --git a/security/security.c b/security/security.c index 09664e09fec9..b7f8ec93f6f0 100644 --- a/security/security.c +++ b/security/security.c @@ -4360,14 +4360,14 @@ EXPORT_SYMBOL(security_secctx_to_secid); /** * security_release_secctx() - Free a secctx buffer - * @secdata: secctx - * @seclen: length of secctx + * @cp: the security context * * Release the security context. */ -void security_release_secctx(char *secdata, u32 seclen) +void security_release_secctx(struct lsm_context *cp) { - call_void_hook(release_secctx, secdata, seclen); + call_void_hook(release_secctx, cp); + memset(cp, 0, sizeof(*cp)); } EXPORT_SYMBOL(security_release_secctx); diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index f5a08f94e094..aabc724f4d44 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6657,9 +6657,16 @@ static int selinux_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid) secid, GFP_KERNEL); } -static void selinux_release_secctx(char *secdata, u32 seclen) +static void selinux_release_secctx(struct lsm_context *cp) { - kfree(secdata); + /* + * stacking scaffolding: + * When it is possible for more than one LSM to provide a + * release hook, do this check: + * if (cp->id == LSM_ID_SELINUX || cp->id == LSM_ID_UNDEF) + */ + + kfree(cp->context); } static void selinux_inode_invalidate_secctx(struct inode *inode) -- cgit v1.2.3