Re: [PATCH 2/2] selinux: add gfp argument to security_xfrm_policy_alloc and fix callers
From: Paul Moore <paul@paul-moore.com>
Date: 2014-03-07 03:22:07
Also in:
selinux
On Tuesday, March 04, 2014 01:26:24 PM Nikolay Aleksandrov wrote:
security_xfrm_policy_alloc can be called in atomic context so the allocation should be done with GFP_ATOMIC. Add an argument to let the callers choose the appropriate way. In order to do so a gfp argument needs to be added to the method xfrm_policy_alloc_security in struct security_operations and to the internal function selinux_xfrm_alloc_user. After that switch to GFP_ATOMIC in the atomic callers and leave GFP_KERNEL as before for the rest. The path that needed the gfp argument addition is: security_xfrm_policy_alloc -> security_ops.xfrm_policy_alloc_security -> all users of xfrm_policy_alloc_security (e.g. selinux_xfrm_policy_alloc) -> selinux_xfrm_alloc_user (here the allocation used to be GFP_KERNEL only) CC: Paul Moore <paul@paul-moore.com> CC: Dave Jones <redacted> CC: Steffen Klassert <steffen.klassert@secunet.com> CC: Fan Du <redacted> CC: David S. Miller <davem@davemloft.net> CC: LSM list <redacted> Signed-off-by: Nikolay Aleksandrov <redacted>
[NOTE: added the SELinux list to the CC list above] In general, the patch is pretty simple with the obvious necessary changes, just one gotcha, see below.
quoted hunk ↗ jump to hunk
diff --git a/security/selinux/xfrm.c b/security/selinux/xfrm.c index 0462cb3ff0a7..7ae773f4fe38 100644 --- a/security/selinux/xfrm.c +++ b/security/selinux/xfrm.c@@ -78,7 +78,8 @@ static inline int selinux_authorizable_xfrm(structxfrm_state *x) * xfrm_user_sec_ctx context. */ static int selinux_xfrm_alloc_user(struct xfrm_sec_ctx **ctxp, - struct xfrm_user_sec_ctx *uctx) + struct xfrm_user_sec_ctx *uctx, + gfp_t gfp) { int rc; const struct task_security_struct *tsec = current_security();@@ -94,7 +95,7 @@ static int selinux_xfrm_alloc_user(struct xfrm_sec_ctx**ctxp, if (str_len >= PAGE_SIZE) return -ENOMEM; - ctx = kmalloc(sizeof(*ctx) + str_len + 1, GFP_KERNEL); + ctx = kmalloc(sizeof(*ctx) + str_len + 1, gfp); if (!ctx) return -ENOMEM;
Also located in selinux_xfrm_alloc_user() is a call to security_context_to_sid() which calls security_context_to_sid_core() which in some cases does allocate memory. The good news is that to_sid_core() does accept a gfp_t flag, the bad news is that to_sid() always passes GFP_KERNEL. It looks like we need to extend this patch a bit, or add another. Sorry about that. If you're getting tired of playing with the LSM/SELinux code let me know :)
quoted hunk ↗ jump to hunk
@@ -282,9 +283,10 @@ int selinux_xfrm_skb_sid(struct sk_buff *skb, u32 *sid)* LSM hook implementation that allocs and transfers uctx spec to xfrm_policy. */ int selinux_xfrm_policy_alloc(struct xfrm_sec_ctx **ctxp, - struct xfrm_user_sec_ctx *uctx) + struct xfrm_user_sec_ctx *uctx, + gfp_t gfp) { - return selinux_xfrm_alloc_user(ctxp, uctx); + return selinux_xfrm_alloc_user(ctxp, uctx, gfp); } /*@@ -332,7 +334,7 @@ int selinux_xfrm_policy_delete(struct xfrm_sec_ctx *ctx)int selinux_xfrm_state_alloc(struct xfrm_state *x, struct xfrm_user_sec_ctx *uctx) { - return selinux_xfrm_alloc_user(&x->security, uctx); + return selinux_xfrm_alloc_user(&x->security, uctx, GFP_KERNEL); } /*
-- paul moore www.paul-moore.com