Re: [PATCH] bugfix: was Re: [ linus-git ] prctl(PR_SET_KEEPCAPS, ...) is broken for some configs, e.g. CONFIG_SECURITY_SELINUX

["Serge E. Hallyn" <serue@us.ibm.com>]

Quoting Andrew G. Morgan (morgan@kernel.org):
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> I agree. Short term, here is a patch to add dummy support for KEEPCAPS.
>
> Cheers
>
> Andrew
>
> Serge E. Hallyn wrote:
> |>> I fear that nothing will happen, and we'll end up wasting a lot of
> |> peoples' time sending hey-why-did-my-dhcp-break reports.
> |
> | If we decide to get rid of dummy long-term, then it's far less
> | distasteful to have it lie and claim the keepcaps worked in the
> | meantime.
> |
> | So for 2.6.26 we could have dummy lie, then plan to make capabilities
> | the default for 2.6.27?
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.2.6 (GNU/Linux)
>
> iD8DBQFITgKA+bHCR3gb8jsRAiQYAJ47VnlBq2GSvLQv40tymjybLhNAtQCgya8G
> YZQN/5w1uq+X2MYv1x4T4D4=
> =NhwX
> -----END PGP SIGNATURE-----

> From be19a4716c97c5aaf4c9721eeccfab2d44897ce2 Mon Sep 17 00:00:00 2001
> From: Andrew G. Morgan <morgan@kernel.org>
> Date: Mon, 9 Jun 2008 21:22:18 -0700
> Subject: [PATCH] Add (back) dummy support for KEEPCAPS.
> 
> See: http://bugzilla.kernel.org/show_bug.cgi?id=10748
> 
> Signed-off-by: Andrew G. Morgan <morgan@kernel.org>

Thanks, Andrew.  Just one question inline.  Nevertheless,

Acked-by: Serge Hallyn <serue@us.ibm.com>

Dmitry, does this fix the problem for you?

(Not sure why I'm feeling queasy about this given that
find . -name "*.c" -exec "grep" "-Hn" "issecure" "{}" \;
returns only hits in security/commoncap.c...)

> ---
>  security/dummy.c |   24 +++++++++++++++++++++++-
>  1 files changed, 23 insertions(+), 1 deletions(-)
> 
> diff --git a/security/dummy.c b/security/dummy.c
> index f50c6c3..b891688 100644
> --- a/security/dummy.c
> +++ b/security/dummy.c
> @@ -27,6 +27,8 @@
>  #include <linux/hugetlb.h>
>  #include <linux/ptrace.h>
>  #include <linux/file.h>
> +#include <linux/prctl.h>
> +#include <linux/securebits.h>
>  
>  static int dummy_ptrace (struct task_struct *parent, struct task_struct *child)
>  {
> @@ -607,7 +609,27 @@ static int dummy_task_kill (struct task_struct *p, struct siginfo *info,
>  static int dummy_task_prctl (int option, unsigned long arg2, unsigned long arg3,
>  			     unsigned long arg4, unsigned long arg5, long *rc_p)
>  {
> -	return 0;
> +	switch (option) {
> +	case PR_CAPBSET_READ:
> +		*rc_p = (cap_valid(arg2) ? 1 : -EINVAL);
> +		break;
> +	case PR_GET_KEEPCAPS:
> +		*rc_p = issecure(SECURE_KEEP_CAPS);
> +		break;
> +	case PR_SET_KEEPCAPS:
> +		if (arg2 > 1)
> +			*rc_p = -EINVAL;
> +		else if (arg2)
> +			current->securebits |= issecure_mask(SECURE_KEEP_CAPS);
> +		else
> +			current->securebits &=
> +				~issecure_mask(SECURE_KEEP_CAPS);

In these last two conditions, don't you need to set *rc_p?

Oh, or my kernel tree may be out of date, as I seem to recall a recent
patch initializing error to 0 in sys_prctl(), so this wouldn't
technically be a problem?  Still would seem correct...

> +		break;
> +	default:
> +		return 0;
> +	}
> +
> +	return 1;
>  }
>  
>  static void dummy_task_reparent_to_init (struct task_struct *p)
> -- 
> 1.5.3.7
>