selinux,xfrm: fix dangling refcount on deferred skb free

Message ID	20241106155509.1706965-1-omosnace@redhat.com (mailing list archive)
State	Not Applicable
Delegated to:	Netdev Maintainers
Headers	show Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8B8B71DED78 for <netdev@vger.kernel.org>; Wed, 6 Nov 2024 15:55:16 +0000 (UTC) From: Ondrej Mosnacek <omosnace@redhat.com> To: Paul Moore <paul@paul-moore.com>, Steffen Klassert <steffen.klassert@secunet.com>, Herbert Xu <herbert@gondor.apana.org.au>, "David S. Miller" <davem@davemloft.net> Cc: Stephen Smalley <stephen.smalley.work@gmail.com>, Eric Dumazet <edumazet@google.com>, selinux@vger.kernel.org, linux-security-module@vger.kernel.org, netdev@vger.kernel.org Subject: [PATCH] selinux,xfrm: fix dangling refcount on deferred skb free Date: Wed, 6 Nov 2024 16:55:09 +0100 Message-ID: <20241106155509.1706965-1-omosnace@redhat.com> Precedence: bulk MIME-Version: 1.0 Content-Transfer-Encoding: 8bit
Series	selinux,xfrm: fix dangling refcount on deferred skb free \| expand selinux,xfrm: fix dangling refcount on deferred skb free

Context	Check	Description
netdev/tree_selection	success	Not a local patch

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 9eca013aa5e1f..c1f58a74a64ba 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -387,12 +387,14 @@ LSM_HOOK(int, 0, xfrm_policy_clone_security, struct xfrm_sec_ctx *old_ctx, LSM_HOOK(void, LSM_RET_VOID, xfrm_policy_free_security, struct xfrm_sec_ctx *ctx) LSM_HOOK(int, 0, xfrm_policy_delete_security, struct xfrm_sec_ctx *ctx) +LSM_HOOK(void, LSM_RET_VOID, xfrm_policy_deleted, struct xfrm_sec_ctx *ctx) LSM_HOOK(int, 0, xfrm_state_alloc, struct xfrm_state *x, struct xfrm_user_sec_ctx *sec_ctx) LSM_HOOK(int, 0, xfrm_state_alloc_acquire, struct xfrm_state *x, struct xfrm_sec_ctx *polsec, u32 secid) LSM_HOOK(void, LSM_RET_VOID, xfrm_state_free_security, struct xfrm_state *x) LSM_HOOK(int, 0, xfrm_state_delete_security, struct xfrm_state *x) +LSM_HOOK(void, LSM_RET_VOID, xfrm_state_deleted, struct xfrm_state *x) LSM_HOOK(int, 0, xfrm_policy_lookup, struct xfrm_sec_ctx *ctx, u32 fl_secid) LSM_HOOK(int, 1, xfrm_state_pol_flow_match, struct xfrm_state *x, struct xfrm_policy *xp, const struct flowi_common *flic) diff --git a/include/linux/security.h b/include/linux/security.h index b86ec2afc6910..ac1f85d0f1110 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -1873,10 +1873,12 @@ int security_xfrm_policy_alloc(struct xfrm_sec_ctx **ctxp, int security_xfrm_policy_clone(struct xfrm_sec_ctx *old_ctx, struct xfrm_sec_ctx **new_ctxp); void security_xfrm_policy_free(struct xfrm_sec_ctx *ctx); int security_xfrm_policy_delete(struct xfrm_sec_ctx *ctx); +void security_xfrm_policy_deleted(struct xfrm_sec_ctx *ctx); int security_xfrm_state_alloc(struct xfrm_state *x, struct xfrm_user_sec_ctx *sec_ctx); int security_xfrm_state_alloc_acquire(struct xfrm_state *x, struct xfrm_sec_ctx *polsec, u32 secid); int security_xfrm_state_delete(struct xfrm_state *x); +void security_xfrm_state_deleted(struct xfrm_state *x); void security_xfrm_state_free(struct xfrm_state *x); int security_xfrm_policy_lookup(struct xfrm_sec_ctx *ctx, u32 fl_secid); int security_xfrm_state_pol_flow_match(struct xfrm_state *x, @@ -1908,6 +1910,10 @@ static inline int security_xfrm_policy_delete(struct xfrm_sec_ctx *ctx) return 0; } +static inline void security_xfrm_policy_deleted(struct xfrm_sec_ctx *ctx) +{ +} + static inline int security_xfrm_state_alloc(struct xfrm_state *x, struct xfrm_user_sec_ctx *sec_ctx) { @@ -1929,6 +1935,10 @@ static inline int security_xfrm_state_delete(struct xfrm_state *x) return 0; } +static inline void security_xfrm_state_deleted(struct xfrm_state *x) +{ +} + static inline int security_xfrm_policy_lookup(struct xfrm_sec_ctx *ctx, u32 fl_secid) { return 0; diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index 914bac03b52ad..1433520c62c94 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -2313,6 +2313,7 @@ static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol, list_del_init(&pol->walk.all); net->xfrm.policy_count[dir]--; + security_xfrm_policy_deleted(pol->security); return pol; } diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index 37478d36a8dff..80f5006bc414e 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c @@ -760,6 +760,8 @@ int __xfrm_state_delete(struct xfrm_state *x) xfrm_dev_state_delete(x); + security_xfrm_state_deleted(x); + /* All xfrm_state objects are created by xfrm_state_alloc. * The xfrm_state_alloc call gives a reference, and that * is what we are dropping here. diff --git a/security/security.c b/security/security.c index 6875eb4a59fcc..f6a985417f6f8 100644 --- a/security/security.c +++ b/security/security.c @@ -5295,6 +5295,19 @@ int security_xfrm_policy_delete(struct xfrm_sec_ctx *ctx) return call_int_hook(xfrm_policy_delete_security, ctx); } +/** + * security_xfrm_policy_deleted() - Handle deletion of xfrm policy + * @ctx: xfrm security context + * + * Handle deletion of xfrm policy. This is called on the actual deletion + * of the policy from the xfrm system. References to the policy may be + * still held elsewhere, so resources should not be freed yet. + */ +void security_xfrm_policy_deleted(struct xfrm_sec_ctx *ctx) +{ + call_void_hook(xfrm_policy_deleted, ctx); +} + /** * security_xfrm_state_alloc() - Allocate a xfrm state LSM blob * @x: xfrm state being added to the SAD @@ -5345,6 +5358,19 @@ int security_xfrm_state_delete(struct xfrm_state *x) } EXPORT_SYMBOL(security_xfrm_state_delete); +/** + * security_xfrm_state_deleted() - Handle deletion of xfrm state + * @x: xfrm state + * + * Handle deletion of xfrm state. This is called on the actual deletion + * of the state from the xfrm system. References to the state may be + * still held elsewhere, so resources should not be freed yet. + */ +void security_xfrm_state_deleted(struct xfrm_state *x) +{ + call_void_hook(xfrm_state_deleted, x); +} + /** * security_xfrm_state_free() - Free a xfrm state * @x: xfrm state diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index ad3abd48eed1d..d3ade56c09e8f 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -7315,8 +7315,10 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = { #ifdef CONFIG_SECURITY_NETWORK_XFRM LSM_HOOK_INIT(xfrm_policy_free_security, selinux_xfrm_policy_free), LSM_HOOK_INIT(xfrm_policy_delete_security, selinux_xfrm_policy_delete), + LSM_HOOK_INIT(xfrm_policy_deleted, selinux_xfrm_policy_deleted), LSM_HOOK_INIT(xfrm_state_free_security, selinux_xfrm_state_free), LSM_HOOK_INIT(xfrm_state_delete_security, selinux_xfrm_state_delete), + LSM_HOOK_INIT(xfrm_state_deleted, selinux_xfrm_state_deleted), LSM_HOOK_INIT(xfrm_policy_lookup, selinux_xfrm_policy_lookup), LSM_HOOK_INIT(xfrm_state_pol_flow_match, selinux_xfrm_state_pol_flow_match), diff --git a/security/selinux/include/xfrm.h b/security/selinux/include/xfrm.h index de485556ae29c..bde5a9e2ccf95 100644 --- a/security/selinux/include/xfrm.h +++ b/security/selinux/include/xfrm.h @@ -19,12 +19,14 @@ int selinux_xfrm_policy_clone(struct xfrm_sec_ctx *old_ctx, struct xfrm_sec_ctx **new_ctxp); void selinux_xfrm_policy_free(struct xfrm_sec_ctx *ctx); int selinux_xfrm_policy_delete(struct xfrm_sec_ctx *ctx); +void selinux_xfrm_policy_deleted(struct xfrm_sec_ctx *ctx); int selinux_xfrm_state_alloc(struct xfrm_state *x, struct xfrm_user_sec_ctx *uctx); int selinux_xfrm_state_alloc_acquire(struct xfrm_state *x, struct xfrm_sec_ctx *polsec, u32 secid); void selinux_xfrm_state_free(struct xfrm_state *x); int selinux_xfrm_state_delete(struct xfrm_state *x); +void selinux_xfrm_state_deleted(struct xfrm_state *x); int selinux_xfrm_policy_lookup(struct xfrm_sec_ctx *ctx, u32 fl_secid); int selinux_xfrm_state_pol_flow_match(struct xfrm_state *x, struct xfrm_policy *xp, diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c index 90ec4ef1b082f..35372bdba7279 100644 --- a/security/selinux/xfrm.c +++ b/security/selinux/xfrm.c @@ -124,7 +124,6 @@ static void selinux_xfrm_free(struct xfrm_sec_ctx *ctx) if (!ctx) return; - atomic_dec(&selinux_xfrm_refcount); kfree(ctx); } @@ -321,6 +320,14 @@ int selinux_xfrm_policy_delete(struct xfrm_sec_ctx *ctx) return selinux_xfrm_delete(ctx); } +/* + * LSM hook implementation that handles deletion of labeled SAs. + */ +void selinux_xfrm_policy_deleted(struct xfrm_sec_ctx *ctx) +{ + atomic_dec(&selinux_xfrm_refcount); +} + /* * LSM hook implementation that allocates a xfrm_sec_state, populates it using * the supplied security context, and assigns it to the xfrm_state. @@ -389,6 +396,14 @@ int selinux_xfrm_state_delete(struct xfrm_state *x) return selinux_xfrm_delete(x->security); } +/* + * LSM hook implementation that handles deletion of labeled SAs. + */ +void selinux_xfrm_state_deleted(struct xfrm_state *x) +{ + atomic_dec(&selinux_xfrm_refcount); +} + /* * LSM hook that controls access to unlabelled packets. If * a xfrm_state is authorizable (defined by macro) then it was

selinux,xfrm: fix dangling refcount on deferred skb free

Checks

Commit Message

Comments

Patch