public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Andrew G. Morgan" <morgan@kernel.org>
To: Chris Wright <chrisw@sous-sol.org>
Cc: Dave Jones <davej@codemonkey.org.uk>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	bojan@rexursive.com, "Serge E. Hallyn" <serue@us.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux Security Modules List 
	<linux-security-module@vger.kernel.org>
Subject: Re: capget() overflows buffers.
Date: Fri, 23 May 2008 17:02:24 -0700	[thread overview]
Message-ID: <48375B10.10003@kernel.org> (raw)
In-Reply-To: <20080523182602.GO30402@sequoia.sous-sol.org>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Chris Wright wrote:
| * Andrew G. Morgan (morgan@kernel.org) wrote:
|> diff --git a/include/linux/capability.h b/include/linux/capability.h
|> index f4ea0dd..f88b4db 100644
|> --- a/include/linux/capability.h
|> +++ b/include/linux/capability.h
|> @@ -34,9 +34,6 @@ struct task_struct;
|>  #define _LINUX_CAPABILITY_VERSION_2  0x20071026
|>  #define _LINUX_CAPABILITY_U32S_2     2
|>
|> -#define _LINUX_CAPABILITY_VERSION    _LINUX_CAPABILITY_VERSION_2
|> -#define _LINUX_CAPABILITY_U32S       _LINUX_CAPABILITY_U32S_2
|> -
|>  typedef struct __user_cap_header_struct {
|>  	__u32 version;
|>  	int pid;
|> @@ -77,10 +74,23 @@ struct vfs_cap_data {
|>  	} data[VFS_CAP_U32];
|>  };
|>
|> -#ifdef __KERNEL__
|> +#ifndef __KERNEL__
|> +
|> +/*
|> + * Backwardly compatible definition for source code - trapped in a
|> + * 32-bit world. If you find you need this, please consider using
|> + * libcap to untrap yourself...
|> + */
|> +#define _LINUX_CAPABILITY_VERSION  _LINUX_CAPABILITY_VERSION_1
|> +#define _LINUX_CAPABILITY_U32S     _LINUX_CAPABILITY_U32S_1
|
| I'm sure you're painfully aware, but this will need some change
| to libcap as well (to let it handle 64bit caps again).

Not really.

libcap2 (the one with 64-bit capability support) continues to work fine
(it ships with its own copy of a compatible linux/capability.h, and when
I update that, I'll make the corresponding change) but the effect should
be transparent to its users:

http://www.kernel.org/pub/linux/libs/security/linux-privs/kernel-2.6/

| That's what I meant earlier by "And use another mechanism to

| signal the availability of 64bit caps."

There is and has always been a method:

~   head.version = 0;
~   getcap(&head, NULL);
~   switch (head.version) {
~   case _LINUX_...:
~      ...
~      break;
~   case _,,,,:
~      etc...
~      break;
~   default:
~      abort("no idea what to do");
~   }

| All looks good.  I think we need to issue some warnings, because
| at least Fedora 9 and openSUSE 11 are/will be 2.6.25 based.

Do any of the above answers help? (FWIW I attached the patch to the
redhat bug.)

Cheers

Andrew

|
| Something like this:
|
|
| --- a/include/linux/capability.h
| +++ b/include/linux/capability.h
| @@ -31,9 +31,13 @@ struct task_struct;
|  #define _LINUX_CAPABILITY_VERSION_1  0x19980330
|  #define _LINUX_CAPABILITY_U32S_1     1
|
| +/* v2 should be considered broken */
|  #define _LINUX_CAPABILITY_VERSION_2  0x20071026
|  #define _LINUX_CAPABILITY_U32S_2     2
|
| +#define _LINUX_CAPABILITY_VERSION_3  0x20080522
| +#define _LINUX_CAPABILITY_U32S_3     2
| +
|  typedef struct __user_cap_header_struct {
|  	__u32 version;
|  	int pid;
| @@ -86,8 +90,8 @@ struct vfs_cap_data {
|
|  #else
|
| -#define _KERNEL_CAPABILITY_VERSION _LINUX_CAPABILITY_VERSION_2
| -#define _KERNEL_CAPABILITY_U32S    _LINUX_CAPABILITY_U32S_2
| +#define _KERNEL_CAPABILITY_VERSION _LINUX_CAPABILITY_VERSION_3
| +#define _KERNEL_CAPABILITY_U32S    _LINUX_CAPABILITY_U32S_3
|
|  typedef struct kernel_cap_struct {
|  	__u32 cap[_KERNEL_CAPABILITY_U32S];
| --- a/kernel/capability.c
| +++ b/kernel/capability.c
| @@ -39,6 +39,19 @@ EXPORT_SYMBOL(__cap_init_eff_set);
|   *   http://www.kernel.org/pub/linux/libs/security/linux-privs/
|   */
|
| +static void warn_broken_capability_use(void)
| +{
| +	static int warned;
| +	if (!warned) {
| +		char name[sizeof(current->comm)];
| +
| +		printk(KERN_INFO "warning: `%s' uses a capabilities version"
| +		       " which could be broken if it is not using libcap.\n",
| +		       get_task_comm(name, current));
| +		warned = 1;
| +	}
| +}
| +
|  static void warn_legacy_capability_use(void)
|  {
|  	static int warned;
| @@ -85,8 +98,12 @@ asmlinkage long sys_capget(cap_user_head
|  		tocopy = _LINUX_CAPABILITY_U32S_1;
|  		break;
|  	case _LINUX_CAPABILITY_VERSION_2:
| +		warn_broken_capability_use();
|  		tocopy = _LINUX_CAPABILITY_U32S_2;
|  		break;
| +	case _LINUX_CAPABILITY_VERSION_3:
| +		tocopy = _LINUX_CAPABILITY_U32S_3;
| +		break;
|  	default:
|  		if (put_user(_KERNEL_CAPABILITY_VERSION, &header->version))
|  			return -EFAULT;
| @@ -257,8 +274,12 @@ asmlinkage long sys_capset(cap_user_head
|  		tocopy = _LINUX_CAPABILITY_U32S_1;
|  		break;
|  	case _LINUX_CAPABILITY_VERSION_2:
| +		warn_broken_capability_use();
|  		tocopy = _LINUX_CAPABILITY_U32S_2;
|  		break;
| +	case _LINUX_CAPABILITY_VERSION_3:
| +		tocopy = _LINUX_CAPABILITY_U32S_3;
| +		break;
|  	default:
|  		if (put_user(_KERNEL_CAPABILITY_VERSION, &header->version))
|  			return -EFAULT;
|
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFIN1sQ+bHCR3gb8jsRAgezAJ0ZRuPNENeHTM46u/LC7IU2++yIowCguMwR
GJhKzg7F2p6PcNrKAv9i8Jg=
=Mrri
-----END PGP SIGNATURE-----

  reply	other threads:[~2008-05-24  0:03 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-22 14:04 capget() overflows buffers Dave Jones
2008-05-22 17:58 ` Chris Wright
2008-05-22 20:53   ` Chris Wright
2008-05-22 22:52     ` Andrew G. Morgan
2008-05-22 23:37       ` Chris Wright
2008-05-23  7:09         ` Andrew G. Morgan
2008-05-23 15:57           ` Chris Wright
2008-05-24  6:25             ` Andrew G. Morgan
2008-05-24  8:07               ` Chris Wright
2008-05-27  1:17                 ` [PATCH] security: was "Re: capget() overflows buffers." Andrew G. Morgan
2008-05-27 21:42                   ` Chris Wright
2008-05-28  3:33                   ` Andrew Morton
2008-05-23 18:26           ` capget() overflows buffers Chris Wright
2008-05-24  0:02             ` Andrew G. Morgan [this message]
2008-05-24  1:09               ` Chris Wright
2008-05-24  4:40                 ` Andrew G. Morgan
2008-05-24  8:17                   ` Chris Wright
2008-05-23  1:20     ` Bojan Smojver
2008-05-23  2:06       ` Chris Wright
2008-05-23  4:01         ` Bojan Smojver
2008-05-22 21:20   ` Bojan Smojver

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48375B10.10003@kernel.org \
    --to=morgan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=bojan@rexursive.com \
    --cc=chrisw@sous-sol.org \
    --cc=davej@codemonkey.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=serue@us.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox