All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] selinux: fix kernel BUG on prlimit(..., NULL, NULL)
@ 2017-02-28 14:35 Stephen Smalley
  2017-02-28 21:39 ` Paul Moore
  2017-03-01  2:11 ` James Morris
  0 siblings, 2 replies; 4+ messages in thread
From: Stephen Smalley @ 2017-02-28 14:35 UTC (permalink / raw)
  To: james.l.morris
  Cc: linux-security-module, paul, jslaby, selinux, xiaolong.ye,
	Stephen Smalley

commit 79bcf325e6b32b3c ("prlimit,security,selinux: add a security hook
for prlimit") introduced a security hook for prlimit() and implemented it
for SELinux.  However, if prlimit() is called with NULL arguments for both
the new limit and the old limit, then the hook is called with 0 for the
read/write flags, since the prlimit() will neither read nor write the
process' limits.  This would in turn lead to calling avc_has_perm() with 0
for the requested permissions, which triggers a BUG_ON() in
avc_has_perm_noaudit() since the kernel should never be invoking
avc_has_perm() with no permissions.  Fix this in the SELinux hook by
returning immediately if the flags are 0.  Arguably prlimit64() itself
ought to return immediately if both old_rlim and new_rlim are NULL since
it is effectively a no-op in that case.

Reported by the lkp-robot based on trinity testing.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
 security/selinux/hooks.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 4a80bd8..af1ff15 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3922,6 +3922,8 @@ int selinux_task_prlimit(const struct cred *cred, const struct cred *tcred,
 {
 	u32 av = 0;
 
+	if (!flags)
+		return 0;
 	if (flags & LSM_PRLIMIT_WRITE)
 		av |= PROCESS__SETRLIMIT;
 	if (flags & LSM_PRLIMIT_READ)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] selinux: fix kernel BUG on prlimit(..., NULL, NULL)
  2017-02-28 14:35 [PATCH] selinux: fix kernel BUG on prlimit(..., NULL, NULL) Stephen Smalley
@ 2017-02-28 21:39 ` Paul Moore
  2017-03-01  2:09   ` James Morris
  2017-03-01  2:11 ` James Morris
  1 sibling, 1 reply; 4+ messages in thread
From: Paul Moore @ 2017-02-28 21:39 UTC (permalink / raw)
  To: Stephen Smalley, James Morris
  Cc: linux-security-module, jslaby, selinux, xiaolong.ye

On Tue, Feb 28, 2017 at 9:35 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> commit 79bcf325e6b32b3c ("prlimit,security,selinux: add a security hook
> for prlimit") introduced a security hook for prlimit() and implemented it
> for SELinux.  However, if prlimit() is called with NULL arguments for both
> the new limit and the old limit, then the hook is called with 0 for the
> read/write flags, since the prlimit() will neither read nor write the
> process' limits.  This would in turn lead to calling avc_has_perm() with 0
> for the requested permissions, which triggers a BUG_ON() in
> avc_has_perm_noaudit() since the kernel should never be invoking
> avc_has_perm() with no permissions.  Fix this in the SELinux hook by
> returning immediately if the flags are 0.  Arguably prlimit64() itself
> ought to return immediately if both old_rlim and new_rlim are NULL since
> it is effectively a no-op in that case.
>
> Reported by the lkp-robot based on trinity testing.
>
> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---
>  security/selinux/hooks.c | 2 ++
>  1 file changed, 2 insertions(+)

James,

I was going to merge this patch but upon closer inspection it appears
that you never sent 79bcf325e6b32b3c to Linux, it is sitting in your
next-queue branch.  What is the plan for your next-queue branch, are
you going to merge it into your next branch after the merge window
closes?  If so, feel free to add my ack.

Acked-by: Paul Moore <paul@paul-moore.com>

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 4a80bd8..af1ff15 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3922,6 +3922,8 @@ int selinux_task_prlimit(const struct cred *cred, const struct cred *tcred,
>  {
>         u32 av = 0;
>
> +       if (!flags)
> +               return 0;
>         if (flags & LSM_PRLIMIT_WRITE)
>                 av |= PROCESS__SETRLIMIT;
>         if (flags & LSM_PRLIMIT_READ)
> --
> 2.7.4
>

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] selinux: fix kernel BUG on prlimit(..., NULL, NULL)
  2017-02-28 21:39 ` Paul Moore
@ 2017-03-01  2:09   ` James Morris
  0 siblings, 0 replies; 4+ messages in thread
From: James Morris @ 2017-03-01  2:09 UTC (permalink / raw)
  To: Paul Moore
  Cc: Stephen Smalley, James Morris, linux-security-module, jslaby,
	xiaolong.ye, selinux

On Tue, 28 Feb 2017, Paul Moore wrote:

> I was going to merge this patch but upon closer inspection it appears
> that you never sent 79bcf325e6b32b3c to Linux, it is sitting in your
> next-queue branch.  What is the plan for your next-queue branch, are
> you going to merge it into your next branch after the merge window
> closes?  If so, feel free to add my ack.

Yes, at -rc1.

> 
> Acked-by: Paul Moore <paul@paul-moore.com>

Thanks.


-- 
James Morris
<jmorris@namei.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] selinux: fix kernel BUG on prlimit(..., NULL, NULL)
  2017-02-28 14:35 [PATCH] selinux: fix kernel BUG on prlimit(..., NULL, NULL) Stephen Smalley
  2017-02-28 21:39 ` Paul Moore
@ 2017-03-01  2:11 ` James Morris
  1 sibling, 0 replies; 4+ messages in thread
From: James Morris @ 2017-03-01  2:11 UTC (permalink / raw)
  To: Stephen Smalley
  Cc: james.l.morris, linux-security-module, paul, jslaby, selinux,
	xiaolong.ye

On Tue, 28 Feb 2017, Stephen Smalley wrote:

> 
> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
> ---

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git#next-queue


-- 
James Morris
<jmorris@namei.org>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-03-01  2:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-28 14:35 [PATCH] selinux: fix kernel BUG on prlimit(..., NULL, NULL) Stephen Smalley
2017-02-28 21:39 ` Paul Moore
2017-03-01  2:09   ` James Morris
2017-03-01  2:11 ` James Morris

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.