Re: [PATCH] X.509: Partially revert patch to add validation against IMA MOK keyring

2016-01-06 Thread James Morris
> Partially revert commit 41c89b64d7184a780f12f2cccdabe65cb2408893:
> 
>   Author: Petko Manolov 
>   Date:   Wed Dec 2 17:47:55 2015 +0200
>   IMA: create machine owner and blacklist keyrings
> 

If you need this applied to a tree, please state which.

-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[GIT PULL] keys bugfix

2015-12-27 Thread James Morris
This is a resend of just the first (critical) fix.

Please pull.


The following changes since commit 8db7b3c54401d83a4dc370a59b8692854000ea03:

  Merge branch 'parisc-4.4-4' of 
git://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux (2015-12-25 
13:19:50 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
for-linus2

David Howells (1):
  KEYS: Fix race between read and revoke

 security/keys/keyctl.c |   18 +-
 1 files changed, 9 insertions(+), 9 deletions(-)

---

commit b4a1b4f5047e4f54e194681125c74c0aa64d637d
Author: David Howells 
Date:   Fri Dec 18 01:34:26 2015 +

KEYS: Fix race between read and revoke

This fixes CVE-2015-7550.

There's a race between keyctl_read() and keyctl_revoke().  If the revoke
happens between keyctl_read() checking the validity of a key and the key's
semaphore being taken, then the key type read method will see a revoked key.

This causes a problem for the user-defined key type because it assumes in
its read method that there will always be a payload in a non-revoked key
and doesn't check for a NULL pointer.

Fix this by making keyctl_read() check the validity of a key after taking
semaphore instead of before.

I think the bug was introduced with the original keyrings code.

This was discovered by a multithreaded test program generated by syzkaller
(http://github.com/google/syzkaller).  Here's a cleaned up version:

#include 
#include 
#include 
void *thr0(void *arg)
{
key_serial_t key = (unsigned long)arg;
keyctl_revoke(key);
return 0;
}
void *thr1(void *arg)
{
key_serial_t key = (unsigned long)arg;
char buffer[16];
keyctl_read(key, buffer, 16);
return 0;
}
int main()
{
key_serial_t key = add_key("user", "%", "foo", 3, 
KEY_SPEC_USER_KEYRING);
pthread_t th[5];
pthread_create(&th[0], 0, thr0, (void *)(unsigned long)key);
pthread_create(&th[1], 0, thr1, (void *)(unsigned long)key);
pthread_create(&th[2], 0, thr0, (void *)(unsigned long)key);
pthread_create(&th[3], 0, thr1, (void *)(unsigned long)key);
pthread_join(th[0], 0);
pthread_join(th[1], 0);
pthread_join(th[2], 0);
pthread_join(th[3], 0);
return 0;
}

Build as:

cc -o keyctl-race keyctl-race.c -lkeyutils -lpthread

Run as:

while keyctl-race; do :; done

as it may need several iterations to crash the kernel.  The crash can be
summarised as:

BUG: unable to handle kernel NULL pointer dereference at 
0010
IP: [] user_read+0x56/0xa3
...
Call Trace:
 [] keyctl_read_key+0xb6/0xd7
 [] SyS_keyctl+0x83/0xe0
 [] entry_SYSCALL_64_fastpath+0x12/0x6f

Reported-by: Dmitry Vyukov 
Signed-off-by: David Howells 
Tested-by: Dmitry Vyukov 
Cc: sta...@vger.kernel.org
Signed-off-by: James Morris 

diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index fb111ea..1c3872a 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -751,16 +751,16 @@ long keyctl_read_key(key_serial_t keyid, char __user 
*buffer, size_t buflen)
 
/* the key is probably readable - now try to read it */
 can_read_key:
-   ret = key_validate(key);
-   if (ret == 0) {
-   ret = -EOPNOTSUPP;
-   if (key->type->read) {
-   /* read the data with the semaphore held (since we
-* might sleep) */
-   down_read(&key->sem);
+   ret = -EOPNOTSUPP;
+   if (key->type->read) {
+   /* Read the data with the semaphore held (since we might sleep)
+* to protect against the key being updated or revoked.
+*/
+   down_read(&key->sem);
+   ret = key_validate(key);
+   if (ret == 0)
ret = key->type->read(key, buffer, buflen);
-       up_read(&key->sem);
-   }
+   up_read(&key->sem);
}
 
 error2:

-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] linux-integrity changes for 4.5

2015-12-26 Thread James Morris
On Mon, 21 Dec 2015, Mimi Zohar wrote:

> Hi James,
> 
> Lots of changes this time.  This pull request adds support, by Dmitry
> Kasatkin, for: making the EVM keyring a trusted keyring, such that only
> keys signed by a key on the system keyring can be loaded onto the EVM
> keyring, loading the EVM keys onto the EVM trusted keyring by the
> kernel, enabling EVM when either the x509 or symmetric keys are
> available and loading the EVM symmetric key from hardware.
> 
> As described by Mark Baushke and Petko Manalov at LSS 2015 in their talk
> "IMA/EVM: Real Applications for Embedded Networking Systems", this pull
> request includes support for two new IMA trusted keyrings named .ima_mok
> and .ima_blacklist.  Keys being loaded on either the EVM or IMA trusted
> keyrings can be validated against either the system trusted keyring or
> the intermediary .ima_mok keyring and prevented from being loaded if on
> the .ima_blacklist keyring.
> 
> Lastly, support for extending and displaying the IMA policy.
> 

Applied.

-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PULL] Smack - Changes for 4.5

2015-12-26 Thread James Morris
On Tue, 22 Dec 2015, Casey Schaufler wrote:

> The following changes since commit ebd68df3f24b318d391d15c458d6f43f340ba36a:
> 
>   Sync to Linus v4.4-rc2 for LSM developers. (2015-11-23 22:46:28 +1100)
> 
> are available in the git repository at:
> 
>   https://github.com/cschaufler/smack-next.git smack-for-4.5
> 
> for you to fetch changes up to 81bd0d56298f93af6ac233d8a7e8b29aa4b094b7:
> 
>   Smack: type confusion in smak sendmsg() handler (2015-12-17 10:21:56 -0800)
> 
> 
> Casey Schaufler (1):
>   Smack: File receive for sockets
> 
> Roman Kubiak (1):
>   Smack: type confusion in smak sendmsg() handler
> 
>  security/smack/smack_lsm.c | 24 +++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 

Applied.


-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] tpmdd updates for Linux 4.5

2015-12-26 Thread James Morris
On Mon, 21 Dec 2015, Jarkko Sakkinen wrote:

> Hi
> 
> Here are tpmdd updates for Linux 4.5. Sorry I didn't send this already
> last week but I had to hold until I get ack from Peter and Mimi before
> doing anything. Patches are quite well baked for a while now with the
> exception of small fix from Stefan to tpm_ibmvtpm, which I considered
> trivial enough to be included.
> 

Applied.

-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] SELinux patches for 4.5

2015-12-26 Thread James Morris
On Thu, 24 Dec 2015, Paul Moore wrote:

> Hi James,
> 
> Nine patches for v4.5; there are a handful of minor fixes (constify 
> parameters, warning rate-limits, etc.) but there are a couple of significant 
> patches that invalidate/revalidate inode labels (needed for gfs2) and make 
> validate_trans decisions visible via selinuxfs.  All the patches pass the 
> selinux-testsuite and have been included in the pcmoore/kernel-secnext Fedora 
> COPR repository[1] for some time now, all looks good.
> 
> As of about five minutes ago, selinux#upstream applied cleanly on top of 
> linux-security#next so I don't expect you should have any problems merging 
> the 
> code.

Applied.

-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] Keys fixes

2015-12-18 Thread James Morris
On Fri, 18 Dec 2015, Linus Torvalds wrote:

> So there is no way in hell I am pulling this.
> 

Sorry for the confusion.

Please pull the first patch from here:

The following changes since commit 73796d8bf27372e26c2b79881947304c14c2d353:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2015-12-17 
14:05:22 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
for-linus2

David Howells (1):
  KEYS: Fix race between read and revoke

---

 security/keys/keyctl.c |   18 +-
 1 files changed, 9 insertions(+), 9 deletions(-)
commit b4a1b4f5047e4f54e194681125c74c0aa64d637d
Author: David Howells 
Date:   Fri Dec 18 01:34:26 2015 +

KEYS: Fix race between read and revoke

This fixes CVE-2015-7550.

There's a race between keyctl_read() and keyctl_revoke().  If the revoke
happens between keyctl_read() checking the validity of a key and the key's
semaphore being taken, then the key type read method will see a revoked key.

This causes a problem for the user-defined key type because it assumes in
its read method that there will always be a payload in a non-revoked key
and doesn't check for a NULL pointer.

Fix this by making keyctl_read() check the validity of a key after taking
semaphore instead of before.

I think the bug was introduced with the original keyrings code.

This was discovered by a multithreaded test program generated by syzkaller
(http://github.com/google/syzkaller).  Here's a cleaned up version:

#include 
#include 
#include 
void *thr0(void *arg)
{
key_serial_t key = (unsigned long)arg;
keyctl_revoke(key);
return 0;
}
void *thr1(void *arg)
{
key_serial_t key = (unsigned long)arg;
char buffer[16];
keyctl_read(key, buffer, 16);
return 0;
}
int main()
{
key_serial_t key = add_key("user", "%", "foo", 3, 
KEY_SPEC_USER_KEYRING);
pthread_t th[5];
pthread_create(&th[0], 0, thr0, (void *)(unsigned long)key);
pthread_create(&th[1], 0, thr1, (void *)(unsigned long)key);
pthread_create(&th[2], 0, thr0, (void *)(unsigned long)key);
pthread_create(&th[3], 0, thr1, (void *)(unsigned long)key);
pthread_join(th[0], 0);
pthread_join(th[1], 0);
pthread_join(th[2], 0);
pthread_join(th[3], 0);
return 0;
}

Build as:

cc -o keyctl-race keyctl-race.c -lkeyutils -lpthread

Run as:

while keyctl-race; do :; done

as it may need several iterations to crash the kernel.  The crash can be
summarised as:

BUG: unable to handle kernel NULL pointer dereference at 
0010
IP: [] user_read+0x56/0xa3
...
Call Trace:
 [] keyctl_read_key+0xb6/0xd7
 [] SyS_keyctl+0x83/0xe0
 [] entry_SYSCALL_64_fastpath+0x12/0x6f

Reported-by: Dmitry Vyukov 
Signed-off-by: David Howells 
Tested-by: Dmitry Vyukov 
Cc: sta...@vger.kernel.org
Signed-off-by: James Morris 

diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index fb111ea..1c3872a 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -751,16 +751,16 @@ long keyctl_read_key(key_serial_t keyid, char __user 
*buffer, size_t buflen)
 
/* the key is probably readable - now try to read it */
 can_read_key:
-   ret = key_validate(key);
-   if (ret == 0) {
-   ret = -EOPNOTSUPP;
-   if (key->type->read) {
-   /* read the data with the semaphore held (since we
-* might sleep) */
-   down_read(&key->sem);
+   ret = -EOPNOTSUPP;
+   if (key->type->read) {
+   /* Read the data with the semaphore held (since we might sleep)
+* to protect against the key being updated or revoked.
+*/
+   down_read(&key->sem);
+   ret = key_validate(key);
+   if (ret == 0)
ret = key->type->read(key, buffer, buflen);
-   up_read(&key->sem);
-   }
+   up_read(&key->sem);
}
 
 error2:
--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[GIT PULL] Keys fixes

2015-12-17 Thread James Morris
Please pull these fixes for the keys subsystem, including a fix for 
CVE-2015-7550.


The following changes since commit 73796d8bf27372e26c2b79881947304c14c2d353:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2015-12-17 
14:05:22 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
for-linus

David Howells (7):
  KEYS: Fix race between read and revoke
  X.509: Fix determination of self-signedness
  X.509: Fix leap year handling again
  Handle leap seconds in mktime64()
  X.509: Support leap seconds
  Handle both ISO 8601 encodings of midnight in mktime64()
  X.509: Handle midnight alternative notation in GeneralizedTime

 crypto/asymmetric_keys/x509_cert_parser.c |   24 +---
 crypto/asymmetric_keys/x509_public_key.c  |   11 ---
 include/linux/time.h  |   13 ++---
 kernel/time/time.c|   19 +++
 security/keys/keyctl.c|   18 +-
 5 files changed, 55 insertions(+), 30 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] keys, trusted: seal with a policy

2015-12-07 Thread James Morris
On Mon, 7 Dec 2015, Jarkko Sakkinen wrote:

> On Fri, Nov 20, 2015 at 01:34:35PM +1100, James Morris wrote:
> > On Wed, 18 Nov 2015, Jarkko Sakkinen wrote:
> > 
> > > On Wed, Nov 18, 2015 at 11:21:01AM +1100, James Morris wrote:
> > > > On Tue, 17 Nov 2015, Jarkko Sakkinen wrote:
> > > > 
> > > > >   }
> > > > >   break;
> > > > > + case Opt_policydigest:
> > > > > + if (!tpm2 ||
> > > > > + strlen(args[0].from) != (2 * 
> > > > > opt->digest_len))
> > > > > + return -EINVAL;
> > > > > + kfree(opt->policydigest);
> > > > > + opt->policydigest = kzalloc(opt->digest_len,
> > > > > + GFP_KERNEL);
> > > > 
> > > > Is it correct to kfree opt->policydigest here before allocating it?
> > > 
> > > I think so. The same option might be encountered multiple times.
> > 
> > This would surely signify an error?
> 
> I'm following the semantics of other options. That's why I implemented
> it that way for example:
> 
> keyctl add trusted kmk "new 32 keyhandle=0x8000 keyhandle=0x8000"
> 
> is perfectly OK. I just thought that it'd be more odd if this option
> behaved in a different way...

It seems broken to me -- if you're messing up keyctl commands you might 
want to know about it, but we should remain consistent.


-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[GIT PULL] security: updated fixes for 4.4

2015-11-25 Thread James Morris
Please pull this updated set, which now includes a fix for SELinux policy 
processing (regression introduced by fa1aa143ac4a), as well as the 
previously posted fix for the user-triggerable oops in the Keys code.

---

The following changes since commit 6ffeba9607343f15303a399bc402a538800d89d9:

  Merge tag 'dm-4.4-fixes' of 
git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm 
(2015-11-24 12:53:11 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
for-linus2

David Howells (1):
  KEYS: Fix handling of stored error in a negatively instantiated user key

James Morris (1):
  Merge branch 'upstream' of git://git.infradead.org/users/pcmoore/selinux 
into for-linus2

Stephen Smalley (1):
  selinux: fix bug in conditional rules handling

 security/keys/encrypted-keys/encrypted.c |2 ++
 security/keys/trusted.c  |5 -
 security/keys/user_defined.c |5 -
 security/selinux/ss/conditional.c|4 ++--
 4 files changed, 12 insertions(+), 4 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[GIT PULL] security: KEYS: Fix handling of stored error in a negatively instantiated user key

2015-11-25 Thread James Morris
Please pull this fix for the keys subsystem, for 4.4, from David Howells.

Note: this oops is triggerable by non-privileged users.

The following changes since commit 6ffeba9607343f15303a399bc402a538800d89d9:

  Merge tag 'dm-4.4-fixes' of 
git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm 
(2015-11-24 12:53:11 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
for-linus

David Howells (1):
  KEYS: Fix handling of stored error in a negatively instantiated user key

 security/keys/encrypted-keys/encrypted.c |2 ++
 security/keys/trusted.c  |5 -
 security/keys/user_defined.c |5 -
 3 files changed, 10 insertions(+), 2 deletions(-)

---
commit 096fe9eaea40a17e125569f9e657e34cdb6d73bd
Author: David Howells 
Date:   Tue Nov 24 21:36:31 2015 +

KEYS: Fix handling of stored error in a negatively instantiated user key

If a user key gets negatively instantiated, an error code is cached in the
payload area.  A negatively instantiated key may be then be positively
instantiated by updating it with valid data.  However, the ->update key
type method must be aware that the error code may be there.

The following may be used to trigger the bug in the user key type:

keyctl request2 user user "" @u
keyctl add user user "a" @u

which manifests itself as:

BUG: unable to handle kernel paging request at ff8a
IP: [] __call_rcu.constprop.76+0x1f/0x280 
kernel/rcu/tree.c:3046
PGD 7cc30067 PUD 0
Oops: 0002 [#1] SMP
Modules linked in:
CPU: 3 PID: 2644 Comm: a.out Not tainted 4.3.0+ #49
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 
01/01/2011
task: 88003ddea700 ti: 88003dd88000 task.ti: 88003dd88000
RIP: 0010:[]  [] 
__call_rcu.constprop.76+0x1f/0x280
 [] __call_rcu.constprop.76+0x1f/0x280 
kernel/rcu/tree.c:3046
RSP: 0018:88003dd8bdb0  EFLAGS: 00010246
RAX: ff82 RBX:  RCX: 0001
RDX: 81e3fe40 RSI:  RDI: ff82
RBP: 88003dd8bde0 R08: 88007d2d2da0 R09: 
R10:  R11: 88003e8073c0 R12: ff82
R13: 88003dd8be68 R14: 88007d027600 R15: 88003ddea700
FS:  00b92880(0063) GS:88007fd0() 
knlGS:
CS:  0010 DS:  ES:  CR0: 8005003b
CR2: ff8a CR3: 7cc5f000 CR4: 06e0
Stack:
 88003dd8bdf0 81160a8a  ff82
 88003dd8be68 88007d027600 88003dd8bdf0 810a39e5
 88003dd8be20 812a31ab 88007d027600 88007d027620
Call Trace:
 [] kfree_call_rcu+0x15/0x20 kernel/rcu/tree.c:3136
 [] user_update+0x8b/0xb0 
security/keys/user_defined.c:129
 [< inline >] __key_update security/keys/key.c:730
 [] key_create_or_update+0x291/0x440 
security/keys/key.c:908
 [< inline >] SYSC_add_key security/keys/keyctl.c:125
 [] SyS_add_key+0x101/0x1e0 security/keys/keyctl.c:60
 [] entry_SYSCALL_64_fastpath+0x12/0x6a 
arch/x86/entry/entry_64.S:185

Note the error code (-ENOKEY) in EDX.

A similar bug can be tripped by:

keyctl request2 trusted user "" @u
keyctl add trusted user "a" @u

This should also affect encrypted keys - but that has to be correctly
parameterised or it will fail with EINVAL before getting to the bit that
will crashes.

Reported-by: Dmitry Vyukov 
Signed-off-by: David Howells 
Acked-by: Mimi Zohar 
Signed-off-by: James Morris 

diff --git a/security/keys/encrypted-keys/encrypted.c 
b/security/keys/encrypted-keys/encrypted.c
index 927db9f..696ccfa 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -845,6 +845,8 @@ static int encrypted_update(struct key *key, struct 
key_preparsed_payload *prep)
size_t datalen = prep->datalen;
int ret = 0;
 
+   if (test_bit(KEY_FLAG_NEGATIVE, &key->flags))
+   return -ENOKEY;
if (datalen <= 0 || datalen > 32767 || !prep->data)
return -EINVAL;
 
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index 903dace..16dec53 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -1007,13 +1007,16 @@ static void trusted_rcu_free(struct rcu_head *rcu)
  */
 static int trusted_update(struct key *key, struct key_preparsed_payload *prep)
 {
-   struct trusted_key_payload *p = key->payload.data[0];
+   struct trusted_key_payload *

Re: [PATCH] KEYS: Fix handling of stored error in a negatively instantiated user key

2015-11-24 Thread James Morris
On Tue, 24 Nov 2015, David Howells wrote:

> Hi James,
> 
> Can this be passed straight to Linus please?

Is this triggerable by normal users?


-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Security next tree synced to v4.4-rc2

2015-11-23 Thread James Morris
For LSM developers who might be waiting for a resync to Linus...



-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] keys, trusted: seal with a policy

2015-11-19 Thread James Morris
On Wed, 18 Nov 2015, Jarkko Sakkinen wrote:

> On Wed, Nov 18, 2015 at 11:21:01AM +1100, James Morris wrote:
> > On Tue, 17 Nov 2015, Jarkko Sakkinen wrote:
> > 
> > >   }
> > >   break;
> > > + case Opt_policydigest:
> > > + if (!tpm2 ||
> > > + strlen(args[0].from) != (2 * opt->digest_len))
> > > + return -EINVAL;
> > > + kfree(opt->policydigest);
> > > + opt->policydigest = kzalloc(opt->digest_len,
> > > + GFP_KERNEL);
> > 
> > Is it correct to kfree opt->policydigest here before allocating it?
> 
> I think so. The same option might be encountered multiple times.

This would surely signify an error?


-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 0/7] User namespace mount updates

2015-11-18 Thread James Morris
On Wed, 18 Nov 2015, Richard Weinberger wrote:

> On Wed, Nov 18, 2015 at 4:13 PM, Al Viro  wrote:
> > On Wed, Nov 18, 2015 at 09:05:12AM -0600, Seth Forshee wrote:
> >
> >> Yes, the host admin. I'm not talking about trusting the admin inside the
> >> container at all.
> >
> > Then why not have the same host admin just plain mount it when setting the
> > container up and be done with that?  From the host namespace, before 
> > spawning
> > the docker instance or whatever framework you are using.  IDGI...
> 
> Because hosting companies sell containers as "full virtual machines"
> and customers expect to be able mount stuff like disk images they upload.

I don't think this is a valid reason for merging functionality into the 
kernel.


-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] keys, trusted: seal with a policy

2015-11-17 Thread James Morris
On Tue, 17 Nov 2015, Jarkko Sakkinen wrote:

>   }
>   break;
> + case Opt_policydigest:
> + if (!tpm2 ||
> + strlen(args[0].from) != (2 * opt->digest_len))
> + return -EINVAL;
> + kfree(opt->policydigest);
> + opt->policydigest = kzalloc(opt->digest_len,
> + GFP_KERNEL);

Is it correct to kfree opt->policydigest here before allocating it?


> + if (!opt->policydigest)
> + return -ENOMEM;
> + res = hex2bin(opt->policydigest, args[0].from,
> +   opt->digest_len);
> + if (res < 0)
> +     return -EINVAL;

Do you need to kfree it here on error?

-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 7/7] Smack: Handle labels consistently in untrusted mounts

2015-11-17 Thread James Morris
On Tue, 17 Nov 2015, Seth Forshee wrote:

> + sbsp = inode->i_sb->s_security;
> + if ((sbsp->smk_flags & SMK_SB_UNTRUSTED) &&

Where is SMK_SB_UNTRUSTED defined?

I can't see it in this patch series, mainline or security next.


-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 6/7] userns: Replace in_userns with current_in_userns

2015-11-17 Thread James Morris
On Tue, 17 Nov 2015, Seth Forshee wrote:

> All current callers of in_userns pass current_user_ns as the
> first argument. Simplify by replacing in_userns with
> current_in_userns which checks whether current_user_ns is in the
> namespace supplied as an argument.
> 
> Signed-off-by: Seth Forshee 

Nice cleanup.


Acked-by: James Morris 

-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 5/7] selinux: Add support for unprivileged mounts from user namespaces

2015-11-17 Thread James Morris
On Tue, 17 Nov 2015, Seth Forshee wrote:

> Security labels from unprivileged mounts in user namespaces must
> be ignored. Force superblocks from user namespaces whose labeling
> behavior is to use xattrs to use mountpoint labeling instead.
> For the mountpoint label, default to converting the current task
> context into a form suitable for file objects, but also allow the
> policy writer to specify a different label through policy
> transition rules.
> 
> Pieced together from code snippets provided by Stephen Smalley.
> 
> Signed-off-by: Seth Forshee 
> Acked-by: Stephen Smalley 


Acked-by: James Morris 

-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 4/7] fs: Treat foreign mounts as nosuid

2015-11-17 Thread James Morris
On Tue, 17 Nov 2015, Seth Forshee wrote:

> From: Andy Lutomirski 
> 
> If a process gets access to a mount from a different user
> namespace, that process should not be able to take advantage of
> setuid files or selinux entrypoints from that filesystem.  Prevent
> this by treating mounts from other mount namespaces and those not
> owned by current_user_ns() or an ancestor as nosuid.
> 
> This will make it safer to allow more complex filesystems to be
> mounted in non-root user namespaces.
> 
> This does not remove the need for MNT_LOCK_NOSUID.  The setuid,
> setgid, and file capability bits can no longer be abused if code in
> a user namespace were to clear nosuid on an untrusted filesystem,
> but this patch, by itself, is insufficient to protect the system
> from abuse of files that, when execed, would increase MAC privilege.
> 
> As a more concrete explanation, any task that can manipulate a
> vfsmount associated with a given user namespace already has
> capabilities in that namespace and all of its descendents.  If they
> can cause a malicious setuid, setgid, or file-caps executable to
> appear in that mount, then that executable will only allow them to
> elevate privileges in exactly the set of namespaces in which they
> are already privileges.
> 
> On the other hand, if they can cause a malicious executable to
> appear with a dangerous MAC label, running it could change the
> caller's security context in a way that should not have been
> possible, even inside the namespace in which the task is confined.
> 
> As a hardening measure, this would have made CVE-2014-5207 much
> more difficult to exploit.
> 
> Signed-off-by: Andy Lutomirski 
> Signed-off-by: Seth Forshee 


Acked-by: James Morris 

-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] security/capability.h: cap_issubset/isclear can be boolean

2015-11-17 Thread James Morris
On Tue, 17 Nov 2015, Yaowei Bai wrote:

> This patch makes cap_issubset/isclear return bool due to these
> functions only using either one or zero as their return
> value.
> 
> No functional change.
> 
> Signed-off-by: Yaowei Bai 

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


-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] security: remove unused cap_is_fs_cap function

2015-11-17 Thread James Morris
On Tue, 17 Nov 2015, Yaowei Bai wrote:

> Since commit 3bc1fa8a ("LSM: remove BSD secure level security module")
> there is no user of cap_is_fs_cap any more, so remove it.
> 
> Signed-off-by: Yaowei Bai 

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

-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[GIT PULL] Security subsystem bugfixes for 4.4

2015-11-12 Thread James Morris
This includes several fixes for TPM, as well as a fix for the x.509 
certificate parser to address CVE-2015-5327.

Please pull.

---

The following changes since commit 5d50ac70fe98518dbf620bfba8184254663125eb:

  Merge tag 'xfs-for-linus-4.4' of 
git://git.kernel.org/pub/scm/linux/kernel/git/dgc/linux-xfs (2015-11-11 
20:18:48 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
for-linus

Christophe JAILLET (1):
  TPM: Avoid reference to potentially freed memory

David Howells (1):
  X.509: Fix the time validation [ver #2]

James Morris (1):
  Merge tag 'tpmdd-next-20151110' of 
https://github.com/jsakkine/linux-tpmdd into for-linus

Jarkko Sakkinen (3):
  TPM: revert the list handling logic fixed in 398a1e7
  tpm: fix missing migratable flag in sealing functionality for TPM2
  tpm: fix compat 'ppi' link handling in tpm_chip_register()

Martin Wilck (2):
  tpm_tis: free irq after probing
  tpm_tis: restore IRQ vector in IO memory after failed probing

 crypto/asymmetric_keys/x509_cert_parser.c |   12 +++-
 drivers/char/tpm/tpm-chip.c   |   20 +++-
 drivers/char/tpm/tpm2-cmd.c   |   15 ++-
 drivers/char/tpm/tpm_of.c |3 ++-
 drivers/char/tpm/tpm_tis.c|8 +++-
 5 files changed, 37 insertions(+), 21 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] X.509: Fix the time validation

2015-11-12 Thread James Morris
On Wed, 11 Nov 2015, David Howells wrote:

> This fixes CVE-2015-5327.  It affects kernels from 4.3-rc1 onwards.

This doesn't apply to current Linus, please fix and resend.

-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4] keys, trusted: select hash algorithm for TPM2 chips

2015-11-09 Thread James Morris
On Thu, 5 Nov 2015, Jarkko Sakkinen wrote:

> v4:
> 
> * Added missing select CRYPTO_HASH_INFO in drivers/char/tpm/Kconfig
> 
> Signed-off-by: Jarkko Sakkinen 


Reviewed-by: James Morris 


-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[GIT PULL] Security subsystem update for 4.4

2015-11-03 Thread James Morris
Please pull.

This is mostly maintenance updates across the subsystem, with a notable 
update for TPM 2.0, and addition of Jarkko Sakkinen as a maintainer of 
that.

The following changes since commit 5062ecdb662bf3aed6dc975019c53ffcd3b01d1c:

  Merge tag 'regmap-v4.4' of 
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap (2015-11-02 
16:16:24 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next

Arnd Bergmann (1):
  apparmor: clarify CRYPTO dependency

David Howells (3):
  KEYS: Provide a script to extract the sys cert list from a vmlinux file
  KEYS: Provide a script to extract a module signature
  KEYS: Merge the type-specific data with the payload data

Dmitry Kasatkin (1):
  integrity: prevent loading untrusted certificates on the IMA trusted 
keyring

Geert Uytterhoeven (1):
  tpm: Allow compile test of GPIO consumers if !GPIOLIB

Geliang Tang (3):
  smack: smk_ipv6_port_list should be static
  KEYS: use kvfree() in add_key
  selinux: ioctl_has_perm should be static

Hon Ching \(Vicky\) Lo (6):
  vTPM: fix memory allocation flag for rtce buffer at kernel boot
  vTPM: fix searching for the right vTPM node in device tree
  vTPM: reformat event log to be byte-aligned
  vTPM: get the buffer allocated for event log instead of the actual log
  vTPM: support little endian guests
  TPM: remove unnecessary little endian conversion

Insu Yun (1):
  keys: Be more consistent in selection of union members used

James Morris (4):
  Merge branch 'next' of git://git.kernel.org/.../zohar/linux-integrity 
into next
  Merge branch 'smack-for-4.4' of https://github.com/cschaufler/smack-next 
into next
  Merge branch 'upstream' of git://git.infradead.org/users/pcmoore/selinux 
into next
  Merge tag 'keys-next-20151021' of 
git://git.kernel.org/.../dhowells/linux-fs into next

Jarkko Sakkinen (10):
  tpm, tpm_crb: fix unaligned read of the command buffer address
  tpm, tpm_tis: fix tpm_tis ACPI detection issue with TPM 2.0
  sysfs: added __compat_only_sysfs_link_entry_to_kobj()
  tpm: move the PPI attributes to character device directory.
  tpm: update PPI documentation to address the location change.
  tpm: introduce tpm_buf
  keys, trusted: move struct trusted_key_options to trusted-type.h
  tpm: seal/unseal for TPM 2.0
  keys, trusted: seal/unseal with TPM 2.0 chips
  MAINTAINERS: add new maintainer for TPM DEVICE DRIVER

Jeff Vander Stoep (1):
  selinux: do not check open perm on ftruncate call

José Bollo (1):
  Smack: Minor initialisation improvement

Krzysztof Kozlowski (1):
  char: Drop owner assignment from i2c_driver

Lukasz Pawelczyk (1):
  Smack: fix a NULL dereference in wrong smack_import_entry() usage

Paul Gortmaker (1):
  certs: add .gitignore to stop git nagging about x509_certificate_list

Paul Moore (1):
  selinux: change CONFIG_SECURITY_SELINUX_CHECKREQPROT_VALUE default

Rasmus Villemoes (5):
  selinux: introduce security_context_str_to_sid
  selinux: remove pointless cast in selinux_inode_setsecurity()
  selinux: use kmemdup in security_sid_to_context_core()
  selinux: use kstrdup() in security_get_bools()
  selinux: use sprintf return value

Roman Kubiak (1):
  Smack: pipefs fix in smack_d_instantiate

Sangwoo (1):
  selinux: Use a kmem_cache for allocation struct file_security_struct

Zbigniew Jasinski (1):
  Smack: limited capability for changing process label

 Documentation/ABI/testing/sysfs-driver-ppi   |   19 +-
 Documentation/crypto/asymmetric-keys.txt |   27 ++--
 Documentation/security/Smack.txt |   10 +
 Documentation/security/keys.txt  |   41 +++--
 MAINTAINERS  |1 +
 arch/powerpc/kernel/prom_init.c  |   40 +++-
 certs/.gitignore |4 +
 crypto/asymmetric_keys/asymmetric_keys.h |5 -
 crypto/asymmetric_keys/asymmetric_type.c |   44 +++--
 crypto/asymmetric_keys/public_key.c  |4 +-
 crypto/asymmetric_keys/signature.c   |2 +-
 crypto/asymmetric_keys/x509_parser.h |1 +
 crypto/asymmetric_keys/x509_public_key.c |9 +-
 drivers/char/tpm/st33zp24/Kconfig|2 +-
 drivers/char/tpm/st33zp24/i2c.c  |1 -
 drivers/char/tpm/tpm-chip.c  |   24 ++-
 drivers/char/tpm/tpm-interface.c |   76 +++
 drivers/char/tpm/tpm.h   |  134 +++-
 drivers/char/tpm/tpm2-cmd.c  |  250 +-
 drivers/char/tpm/tpm_crb.c   |   39 ++--
 drivers/char/tpm/tpm_eventlog.c  |   78 +--
 drivers/char/tpm/tpm_eventlog.h  

Re: [PATCH v3 5/7] security: Add hook to invalidate inode security labels

2015-10-27 Thread James Morris
On Mon, 26 Oct 2015, Andreas Gruenbacher wrote:

> Add a hook to invalidate an inode's security label when the cached
> information becomes invalid.
> 
> Implement the new hook in selinux: set a flag when a security label becomes
> invalid.  When hitting a security label which has been marked as invalid in
> inode_has_perm, try reloading the label.
> 
> If an inode does not have any dentries attached, we cannot reload its
> security label because we cannot use the getxattr inode operation.  In that
> case, continue using the old, invalid label until a dentry becomes
> available.
> 
> Signed-off-by: Andreas Gruenbacher 


Reviewed-by: James Morris 

-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 5/7] security: Add hook to invalidate inode security labels

2015-10-27 Thread James Morris
On Mon, 26 Oct 2015, Andreas Gruenbacher wrote:

> Add a hook to invalidate an inode's security label when the cached
> information becomes invalid.
> 
> Implement the new hook in selinux: set a flag when a security label becomes
> invalid.  When hitting a security label which has been marked as invalid in
> inode_has_perm, try reloading the label.
> 
> If an inode does not have any dentries attached, we cannot reload its
> security label because we cannot use the getxattr inode operation.  In that
> case, continue using the old, invalid label until a dentry becomes
> available.
> 
> Signed-off-by: Andreas Gruenbacher 


Reviewed-by: James Morris 

-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] KEYS: Miscellaneous patches for next

2015-10-22 Thread James Morris
On Thu, 22 Oct 2015, David Howells wrote:

> Hi James,
> 
> Could you pull these changes into your next branch please?
> 
> There are three groups:
> 
>  (1) Miscellaneous cleanups.
> 
>  (2) Add scripts for extracting system cert list and module sigs.
> 
>  (3) Condense the type-specific data in the key struct into the payload
>  data as it doesn't really make any sense to keep them separate.
> 

Pulled.

Have these been in next yet?


-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PULL] SELinux patches for 4.4

2015-10-21 Thread James Morris
On Wed, 21 Oct 2015, Paul Moore wrote:

> Hi James,
> 
> Nine SELinux patches in total for v4.4, although six of those patches are 
> either trivial, minor cleanups, or both.  The remaining three patches aren't 
> too bad: one changes the CHECKREQPROT default to check the actual memory 
> protections, one stops us from checking file:open on ftruncate() calls, and 
> one converts the file_security_struct over to kmem_cache.
> 
> All pass the SELinux testsuite and should apply cleanly on top of your next 
> branch.
> 

Pulled, thanks.


-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] apparmor: clarify CRYPTO dependency

2015-10-21 Thread James Morris
On Wed, 21 Oct 2015, Arnd Bergmann wrote:

> The crypto framework can be built as a loadable module, but the
> apparmor hash code can only be built-in, which then causes a
> link error:
> 
> security/built-in.o: In function `aa_calc_profile_hash':
> integrity_audit.c:(.text+0x21610): undefined reference to 
> `crypto_shash_update'
> security/built-in.o: In function `init_profile_hash':
> integrity_audit.c:(.init.text+0xb4c): undefined reference to 
> `crypto_alloc_shash'
> 
> This changes Apparmor to use 'select CRYPTO' like a lot of other
> subsystems do.
> 
> Signed-off-by: Arnd Bergmann 

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


-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 0/3] RFC: Secure Memory Allocation Framework

2015-10-21 Thread James Morris
On Wed, 21 Oct 2015, Benjamin Gaignard wrote:

> 
> The outcome of the previous RFC about how do secure data path was the need
> of a secure memory allocator (https://lkml.org/lkml/2015/5/5/551)
> 

Have you addressed all the questions raised by Alan here:

https://lkml.org/lkml/2015/5/8/629

Also, is there any application of this beyond DRM?


- James
-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 1/3] create SMAF module

2015-10-21 Thread James Morris
On Wed, 21 Oct 2015, Benjamin Gaignard wrote:

> Secure Memory Allocation Framework goal is to be able
> to allocate memory that can be securing.
> There is so much ways to allocate and securing memory that SMAF
> doesn't do it by itself but need help of additional modules.
> To be sure to use the correct allocation method SMAF implement
> deferred allocation (i.e. allocate memory when only really needed)
> 
> Allocation modules (smaf-alloctor.h):
> SMAF could manage with multiple allocation modules at same time.
> To select the good one SMAF call match() to be sure that a module
> can allocate memory for a given list of devices. It is to the module
> to check if the devices are compatible or not with it allocation
> method.
> 
> Securing module (smaf-secure.h):
> The way of how securing memory it is done is platform specific.
> Secure module is responsible of grant/revoke memory access.
> 

This documentation is highly inadequate.

What does "allocate memory that can be securing" mean?


-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PULL] Smack - Changes for 4.4

2015-10-20 Thread James Morris
On Mon, 19 Oct 2015, Casey Schaufler wrote:

> The following changes since commit 049e6dde7e57f0054fdc49102e7ef4830c698b46:
> 
>   Linux 4.3-rc4 (2015-10-04 16:57:17 +0100)
> 
> are available in the git repository at:
> 
>   https://github.com/cschaufler/smack-next.git smack-for-4.4
> 

Pulled, thanks.

-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[GIT PULL] Keys bugfixes

2015-10-19 Thread James Morris
Please pull these key susbystem fixes for 4.3, per the message from David 
Howells:

"Here are two patches, the first of which at least should go upstream
immediately:

 (1) Prevent a user-triggerable crash in the keyrings destructor when a
 negatively instantiated keyring is garbage collected.  I have also 
 seen this triggered for user type keys.

 (2) Prevent the user from using requesting that a keyring be created and
 instantiated through an upcall.  Doing so is probably safe since the 
 keyring type ignores the arguments to its instantiation function - 
 but we probably shouldn't let keyrings be created in this manner."

---

The following changes since commit 1099f86044111e9a7807f09523e42d4c9d0fb781:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2015-10-19 
09:55:40 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git 
for-linus

David Howells (2):
  KEYS: Fix crash when attempt to garbage collect an uninstantiated keyring
  KEYS: Don't permit request_key() to construct a new keyring

 security/keys/gc.c  |6 --
 security/keys/request_key.c |3 +++
 2 files changed, 7 insertions(+), 2 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PULL REQUEST] IMA changes for 4.4

2015-10-19 Thread James Morris
On Mon, 19 Oct 2015, Mimi Zohar wrote:

> Hi James,
> 
> This pull request is for a single bug fix from Dimtry to properly load
> only signed certificates onto the trusted IMA keyring from the kernel.
> (This patch has been in the linux-next tree).
> 
> thanks,
> 
> Mimi
> 
> The following changes since commit
> 049e6dde7e57f0054fdc49102e7ef4830c698b46:
> 
>   Linux 4.3-rc4 (2015-10-04 16:57:17 +0100)
> 
> are available in the git repository at:
> 
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
> next
> 

Pulled, thanks.

-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PULL REQUEST] Fixes and Updates of TPM Subsystem

2015-10-19 Thread James Morris
On Mon, 19 Oct 2015, Peter Hüwe wrote:

> Hi James,
> 
> I know it's superduper late, but maybe I am lucky enough.
> Can you please pull the following Fixes and Updates at the earliest 
> convenience? 
> If possible even for 4.4?
> 
> Most of the patches have been lying around for a long time due to my fault -
> but at least they are mature enough now.
> 

Pulled, thanks.


-- 
James Morris



Re: Will you be updating security#next for 4.4?

2015-10-06 Thread James Morris
On Mon, 5 Oct 2015, Casey Schaufler wrote:

> Hi James. I'm starting my patch processing for 4.4 and wondered
> if you're planning to update security#next any time soon.
> 

Now merged to -rc4.

-- 
James Morris


--
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: NFS/LSM: allow NFS to control all of its own mount options

2008-02-19 Thread James Morris
On Tue, 19 Feb 2008, Christoph Hellwig wrote:

> Please don't introduce a special case for just nfs.  All filesystems
> should control their mount options, so please provide some library
> helpers for context= handling and move it into all filesystems that
> can support selinux.

It's not so much a special case for NFS, just that NFS happens to use 
binary mount options.  So, I guess it could be put into a library for 
other potential filesystems with binary mount options.

To clarify:

The SELinux options are indeed filesystem independent, and the FS should 
really not need to be concerned at all with them.  For everything except 
NFS, we parse text options looking for context=, then use that value from 
within SELinux as the label for all files in the mount.

Previously, as Eric mentions, we were using a method initially approved by 
the NFS folk, where, for NFS, SELinux was peeking around inside the binary 
options.  We were then asked to change that so that NFS (or other 
binary-option FS) would obtain the values itself and call into LSM with 
them.  This is what Eric's latest patch enables (a previous patch 
installed the infrastructure for it).

While this code could be put into a library if desired, there is no need 
to make any changes for filesystems with text options (i.e. the general 
case).



- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/37] Security: Add a kernel_service object class to SELinux

2008-02-11 Thread James Morris
On Fri, 8 Feb 2008, David Howells wrote:

> +++ b/security/selinux/include/flask.h
> @@ -51,6 +51,7 @@
>  #define SECCLASS_DCCP_SOCKET 60
>  #define SECCLASS_MEMPROTECT  61
>  #define SECCLASS_PEER68
> +#define SECCLASS_KERNEL_SERVICE  69

I just pushed a patch to Linus from Stephen which uses this class number; 
you'll likely need to bump it to 70.


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 07/37] Security: De-embed task security record from task and use refcounting

2008-02-11 Thread James Morris
On Fri, 8 Feb 2008, David Howells wrote:

> Remove the temporarily embedded task security record from task_struct.  
> Instead
> it is made to dangle from the task_struct::sec and task_struct::act_as 
> pointers
> with references counted for each.

...

These patches are kind of huge.

> +static int selinux_task_dup_security(struct task_security *sec)
> +{
> + struct task_security_struct *tsec1, *tsec2;
> +
> + tsec1 = sec->security;
> +
> + tsec2 = kmemdup(tsec1, sizeof(*tsec1), GFP_KERNEL);
> + if (!tsec2)
> + return -ENOMEM;
> +
> + tsec2->osid = tsec1->osid;
> + tsec2->sid = tsec1->sid;
> +
> + tsec2->exec_sid = tsec1->exec_sid;
> + tsec2->create_sid = tsec1->create_sid;
> + tsec2->keycreate_sid = tsec1->keycreate_sid;
> + tsec2->sockcreate_sid = tsec1->sockcreate_sid;
> + tsec2->ptrace_sid = SECINITSID_UNLABELED;
> + sec->security = tsec2;
> +
> + return 0;
>  }

Why manually copy these fields after a kmemdup?

What about the task backpointer? (i.e. tsec2->task)


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 06/37] Security: Separate task security context from task_struct

2008-02-11 Thread James Morris
On Fri, 8 Feb 2008, David Howells wrote:

> Separate the task security context from task_struct.  At this point, the
> security data is temporarily embedded in the task_struct with two pointers
> pointing to it.
> 
> Alpha needs further alteration as it refers to UID & GID in entry.S via asm
> offsets.
> 
> Sparc needs further alteration as it refers to UID & GID in sclow.S via asm
> offsets.
> 
> Signed-off-by: David Howells <[EMAIL PROTECTED]>

Reviewed-by: James Morris <[EMAIL PROTECTED]>

(SELinux stuff mostly).

-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 05/37] Security: Change current->fs[ug]id to current_fs[ug]id()

2008-02-11 Thread James Morris
On Fri, 8 Feb 2008, David Howells wrote:

> Change current->fs[ug]id to current_fs[ug]id() so that fsgid and fsuid can be
> separated from the task_struct.
> 
> Signed-off-by: David Howells <[EMAIL PROTECTED]>

Reviewed-by: James Morris <[EMAIL PROTECTED]>


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] per-process securebits

2008-02-01 Thread James Morris
On Fri, 1 Feb 2008, Andrew Morton wrote:

> Really?  I'd feel a lot more comfortable if yesterday's version 1 had led
> to a stream of comments from suitably-knowledgeable kernel developers which
> indicated that those developers had scrutinised this code from every
> conceivable angle and had declared themselves 100% happy with it.

FWIW, I've reviewed the patch in detail a couple of times, and don't see 
any issues with it that haven't already been raised by Serge.

You can add my reviewed-by.

I think it does need more eyes, and some time baking in -mm.  

File capabilities are at least marked experimental, although it's not 
clear what the criteria would be to unmark them.


- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] security: add iptables "security" table for MAC rules

2008-01-29 Thread James Morris
On Tue, 29 Jan 2008, Paul Moore wrote:

> That seems reasonable.  By the way, this isn't really related, but is it 
> possible to change the NF_IP_PRI_SELINUX_* constants to NF_IP_PRI_SECURITY_* 
> for the sake of consistency or are those values already visible to userspace? 
>  

They are visible to userspace, and included in glibc headers, but I don't 
see any userland use of them via google codesearch or know of a possible 
valid use.

> I suppose we could always rename them anyway and just add a #define for 
> compatibility ...

Yep, if you want to.


- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC] security: add iptables "security" table for MAC rules

2008-01-29 Thread James Morris
On Tue, 29 Jan 2008, Paul Moore wrote:

> I'm not sure if returning false and failing here is the best thing to do in 
> terms of backwards compatibility.  I think we need some grace period where we 
> print a warning message and still allow the operation; after some period of 
> time we can then remove the ability completely and force users to use the 
> new "security" table.

Currently, the patch allows the use of the mangle table, so it is 
backwards compatible.

-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC] security: add iptables "security" table for MAC rules

2008-01-29 Thread James Morris
The following patch implements a new "security" table for iptables, so 
that MAC (SELinux etc.) networking rules can be managed separately to 
standard DAC rules.

This is to help with distro integration of the new secmark-based network 
controls, per various previous discussions.

The need for a separate table arises from the fact that existing tools and 
usage of iptables will likely clash with centralized MAC policy 
management.

The SECMARK and CONNSECMARK targets will still be valid in the mangle 
table, to prevent breakage of existing users, although I suspect that 
these targets are not in significant use and we could probably make them 
valid only in the security table without major issues.   (Comments?)

I've set the table priority to just after NF_IP_FILTER, to allow DAC 
rules to take effect before MAC rules.

There is also not (yet) any LSM hooking for modifying the MAC rules, as it 
will be more invasive, and we have coarse coverage via CAP_NET_ADMIN.  
(Comments?)

IPv6 is not done yet as this is just at RFC stage.

Please review and let me know if this looks like a viable approach for 
distro integration.  If it is, we will then need to run it past the 
Netfilter & netdev folk.

---

Add a "security" table to iptables for managing Mandatory Access
Control (MAC) rules.  This is intended for use with the SECMARK
and CONNSECMARK targets.

Signed-off-by: James Morris <[EMAIL PROTECTED]>
---
 include/linux/netfilter_ipv4.h|1 +
 net/ipv4/netfilter/Kconfig|   10 ++
 net/ipv4/netfilter/Makefile   |1 +
 net/ipv4/netfilter/iptable_security.c |  181 +
 net/netfilter/xt_CONNSECMARK.c|   10 ++-
 net/netfilter/xt_SECMARK.c|   10 ++-
 6 files changed, 207 insertions(+), 6 deletions(-)
 create mode 100644 net/ipv4/netfilter/iptable_security.c

diff --git a/include/linux/netfilter_ipv4.h b/include/linux/netfilter_ipv4.h
index 1a63adf..aec8961 100644
--- a/include/linux/netfilter_ipv4.h
+++ b/include/linux/netfilter_ipv4.h
@@ -60,6 +60,7 @@ enum nf_ip_hook_priorities {
NF_IP_PRI_MANGLE = -150,
NF_IP_PRI_NAT_DST = -100,
NF_IP_PRI_FILTER = 0,
+   NF_IP_PRI_SECURITY = 50,
NF_IP_PRI_NAT_SRC = 100,
NF_IP_PRI_SELINUX_LAST = 225,
NF_IP_PRI_CONNTRACK_HELPER = INT_MAX - 2,
diff --git a/net/ipv4/netfilter/Kconfig b/net/ipv4/netfilter/Kconfig
index 9aca9c5..84d0d31 100644
--- a/net/ipv4/netfilter/Kconfig
+++ b/net/ipv4/netfilter/Kconfig
@@ -373,6 +373,16 @@ config IP_NF_RAW
  If you want to compile it as a module, say M here and read
  .  If unsure, say `N'.
 
+# security table for MAC policy
+config IP_NF_SECURITY
+   tristate "Security table"
+   depends on IP_NF_IPTABLES
+   help
+ This option adds a `security' table to iptables, for use
+ with Mandatory Access Control (MAC) policy.
+ 
+ If unsure, say N.
+
 # ARP tables
 config IP_NF_ARPTABLES
tristate "ARP tables support"
diff --git a/net/ipv4/netfilter/Makefile b/net/ipv4/netfilter/Makefile
index 7456833..90fcdd6 100644
--- a/net/ipv4/netfilter/Makefile
+++ b/net/ipv4/netfilter/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_IP_NF_FILTER) += iptable_filter.o
 obj-$(CONFIG_IP_NF_MANGLE) += iptable_mangle.o
 obj-$(CONFIG_NF_NAT) += iptable_nat.o
 obj-$(CONFIG_IP_NF_RAW) += iptable_raw.o
+obj-$(CONFIG_IP_NF_SECURITY) += iptable_security.o
 
 # matches
 obj-$(CONFIG_IP_NF_MATCH_ADDRTYPE) += ipt_addrtype.o
diff --git a/net/ipv4/netfilter/iptable_security.c 
b/net/ipv4/netfilter/iptable_security.c
new file mode 100644
index 000..640f200
--- /dev/null
+++ b/net/ipv4/netfilter/iptable_security.c
@@ -0,0 +1,181 @@
+/*
+ * "security" table
+ *
+ * This is for use by Mandatory Access Control (MAC) security models,
+ * which need to be able to manage security policy in separate context
+ * to DAC.
+ *
+ * Based on iptable_mangle.c
+ *
+ * Copyright (C) 1999 Paul `Rusty' Russell & Michael J. Neuling
+ * Copyright (C) 2000-2004 Netfilter Core Team <[EMAIL PROTECTED]>
+ * Copyright (C) 2008 Red Hat, Inc., James Morris <[EMAIL PROTECTED]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("James Morris <[EMAIL PROTECTED]>");
+MODULE_DESCRIPTION("iptables security table, for MAC rules");
+
+#define SECURITY_VALID_HOOKS   (1 << NF_IP_LOCAL_IN) | \
+   (1 << NF_IP_FORWARD) | \
+   (1 << NF_IP_LOCAL_OUT)
+
+static struct
+{
+   struct ipt_replace repl;
+   struct ipt_standard entries

Re: "Default Linux Capabilities" default in 2.6.24

2008-01-28 Thread James Morris
On Mon, 28 Jan 2008, Matt LaPlante wrote:

> On Thu, 24 Jan 2008 19:12:01 -0600
> Matt LaPlante <[EMAIL PROTECTED]> wrote:
> 
> > 
> > I'm doing a make oldconfig with the new 2.6.24 kernel.  I came to the 
> > prompt for "Default Linux Capabilities" which defaults to No:
> > 
> > ---
> >  Default Linux Capabilities (SECURITY_CAPABILITIES) [N/y/?] (NEW) ?
> > ---
> > 
> > However the help text recommends saying Yes.
> > 
> > ---
> >  This enables the "default" Linux capabilities functionality.
> >  If you are unsure how to answer this question, answer Y.
> > ---
> > 
> > Does this seem incongruous?  Also, what's the "question"? :)
> > 
> > Thanks, 
> > Matt LaPlante
> 
> Anyone?

I think this should be default y.


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Security: constify function pointer tables and fields

2008-01-23 Thread James Morris
On Wed, 23 Jan 2008, Jan Engelhardt wrote:

> Signed-off-by: Jan Engelhardt <[EMAIL PROTECTED]>
> ---
>  include/linux/security.h |2 +-
>  security/keys/proc.c |4 ++--
>  security/selinux/selinuxfs.c |2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)

Applied to 
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/selinux-2.6.git#for-akpm


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/28] SECURITY: Allow kernel services to override LSM settings for task actions [try #2]

2008-01-23 Thread James Morris
On Wed, 23 Jan 2008, David Howells wrote:

> Stephen Smalley <[EMAIL PROTECTED]> wrote:
> 
> > Make sure that you or Dan submits a policy patch to register these
> > classes and permissions in the policy when the kernel patch is queued
> > for merge.
> 
> Do I just send the attached patch to <[EMAIL PROTECTED]>?  

That should be enough (and that list is on the cc).

>Or do I need to
> make a diff from a point in the tree nearer the root?  Is there anything else
> I need to alter whilst I'm at it?
> 
> David
> ---
> Index: policy/flask/security_classes
> ===
> --- policy/flask/security_classes (revision 2573)
> +++ policy/flask/security_classes (working copy)
> @@ -109,4 +109,7 @@
>  # network peer labels
>  class peer
>  
> +# kernel services that need to override task security
> +class kernel_service
> +
>  # FLASK
> Index: policy/flask/access_vectors
> ===
> --- policy/flask/access_vectors   (revision 2573)
> +++ policy/flask/access_vectors   (working copy)
> @@ -736,3 +736,10 @@
>  {
>   recv
>  }
> +
> +# kernel services that need to override task security
> +class kernel_service
> +{
> + use_as_override
> + create_files_as
> +}
> -
> To unsubscribe from this list: send the line "unsubscribe 
> linux-security-module" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/26] Add a secctx_to_secid() LSM hook to go along with the existing

2008-01-16 Thread James Morris
On Wed, 16 Jan 2008, Paul Moore wrote:

> On Tuesday 15 January 2008 8:05:27 pm James Morris wrote:
> > On Tue, 15 Jan 2008, David Howells wrote:
> > > secid_to_secctx() LSM hook.  This patch also includes the SELinux
> > > implementation for this hook.
> > >
> > > Signed-off-by: Paul Moore <[EMAIL PROTECTED]>
> > > Acked-by: Stephen Smalley <[EMAIL PROTECTED]>
> >
> > This is useful in its own right, and I would like to push it upstream for
> > 2.6.24 unless there are any objections.
> 
> Isn't it a bit late in 2.6.24 to add new functionality, especially when there 
> isn't an in-tree user for it in 2.6.24?

Actually, I meant to go into 2.6.24 after the merge window opens.

> 
> You are right, there are several users of this function currently under 
> development but I'm pretty sure all of them are targeting 2.6.25 or greater.  
> With that in mind, I think the prudent thing to is to wait and push this 
> upstream for 2.6.25.


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 00/26] Permit filesystem local caching

2008-01-15 Thread James Morris
On Tue, 15 Jan 2008, David Howells wrote:

> 
>   (*) 04-keys-get-label.diff
> 
>   A patch to allow the security label of a key to be retrieved.
>   Included because of patches 05-08.
> 
>   (*) 05-security-current-fsugid.diff
>   (*) 06-security-separate-task-bits.diff
>   (*) 07-security-subjective.diff
>   (*) 08-security-secctx2secid.diff
>   (*) 09-security-additional-classes.diff
>   (*) 10-security-kernel_service-class.diff
>   (*) 11-security-kernel-service.diff

All of the security patches look ok to me.  From the SELinux pov, this 
will need to go in after Paul Moore's labeled networking update (hopefully 
very soon after the next merge window opens).


- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 08/26] Add a secctx_to_secid() LSM hook to go along with the existing

2008-01-15 Thread James Morris
id);
> +}
> +
>  static void selinux_release_secctx(char *secdata, u32 seclen)
>  {
>   kfree(secdata);
> @@ -4937,6 +4942,7 @@ static struct security_operations selinux_ops = {
>   .setprocattr =  selinux_setprocattr,
>  
>   .secid_to_secctx =  selinux_secid_to_secctx,
> + .secctx_to_secid =      selinux_secctx_to_secid,
>   .release_secctx =   selinux_release_secctx,
>  
>  .unix_stream_connect =   
> selinux_socket_unix_stream_connect,
> 
> -
> To unsubscribe from this list: send the line "unsubscribe 
> linux-security-module" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 00/26] Permit filesystem local caching

2008-01-15 Thread James Morris
On Tue, 15 Jan 2008, David Howells wrote:

> The SELinux base code will also need updating to have the security class, lest
> the following error appear in dmesg:
> 
>   context_struct_compute_av:  unrecognized class 69

Alternately, what's needed is a newer policy which supports unknown 
permission classes.  Any recent distro policy should have this.


- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH][RFC] security: call security_file_permission from rw_verify_area

2008-01-12 Thread James Morris
Please review.  

Tested with SELinux in enforcing mode.

---

All instances of rw_verify_area() are followed by a call to
security_file_permission(), so just call the latter from the former.

Signed-off-by: James Morris <[EMAIL PROTECTED]>
---
 fs/compat.c |4 ---
 fs/read_write.c |   63 +--
 fs/splice.c |8 ---
 3 files changed, 24 insertions(+), 51 deletions(-)

diff --git a/fs/compat.c b/fs/compat.c
index 15078ce..5216c3f 100644
--- a/fs/compat.c
+++ b/fs/compat.c
@@ -1104,10 +1104,6 @@ static ssize_t compat_do_readv_writev(int type, struct 
file *file,
if (ret < 0)
goto out;
 
-   ret = security_file_permission(file, type == READ ? MAY_READ:MAY_WRITE);
-   if (ret)
-   goto out;
-
fnv = NULL;
if (type == READ) {
fn = file->f_op->read;
diff --git a/fs/read_write.c b/fs/read_write.c
index ea1f94c..c4d3d17 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -197,25 +197,27 @@ int rw_verify_area(int read_write, struct file *file, 
loff_t *ppos, size_t count
 {
struct inode *inode;
loff_t pos;
+   int retval = -EINVAL;
 
inode = file->f_path.dentry->d_inode;
if (unlikely((ssize_t) count < 0))
-   goto Einval;
+   return retval;
pos = *ppos;
if (unlikely((pos < 0) || (loff_t) (pos + count) < 0))
-   goto Einval;
+   return retval;
 
if (unlikely(inode->i_flock && mandatory_lock(inode))) {
-   int retval = locks_mandatory_area(
+   retval = locks_mandatory_area(
read_write == READ ? FLOCK_VERIFY_READ : 
FLOCK_VERIFY_WRITE,
inode, file, pos, count);
if (retval < 0)
return retval;
}
+   retval = security_file_permission(file,
+   read_write == READ ? MAY_READ : MAY_WRITE);
+   if (retval)
+   return retval;
return count > MAX_RW_COUNT ? MAX_RW_COUNT : count;
-
-Einval:
-   return -EINVAL;
 }
 
 static void wait_on_retry_sync_kiocb(struct kiocb *iocb)
@@ -267,18 +269,15 @@ ssize_t vfs_read(struct file *file, char __user *buf, 
size_t count, loff_t *pos)
ret = rw_verify_area(READ, file, pos, count);
if (ret >= 0) {
count = ret;
-   ret = security_file_permission (file, MAY_READ);
-   if (!ret) {
-   if (file->f_op->read)
-   ret = file->f_op->read(file, buf, count, pos);
-   else
-   ret = do_sync_read(file, buf, count, pos);
-   if (ret > 0) {
-   fsnotify_access(file->f_path.dentry);
-   add_rchar(current, ret);
-   }
-   inc_syscr(current);
+   if (file->f_op->read)
+   ret = file->f_op->read(file, buf, count, pos);
+   else
+   ret = do_sync_read(file, buf, count, pos);
+   if (ret > 0) {
+   fsnotify_access(file->f_path.dentry);
+   add_rchar(current, ret);
}
+   inc_syscr(current);
}
 
return ret;
@@ -325,18 +324,15 @@ ssize_t vfs_write(struct file *file, const char __user 
*buf, size_t count, loff_
ret = rw_verify_area(WRITE, file, pos, count);
if (ret >= 0) {
count = ret;
-   ret = security_file_permission (file, MAY_WRITE);
-   if (!ret) {
-   if (file->f_op->write)
-   ret = file->f_op->write(file, buf, count, pos);
-   else
-   ret = do_sync_write(file, buf, count, pos);
-   if (ret > 0) {
-   fsnotify_modify(file->f_path.dentry);
-   add_wchar(current, ret);
-   }
-   inc_syscw(current);
+   if (file->f_op->write)
+   ret = file->f_op->write(file, buf, count, pos);
+   else
+   ret = do_sync_write(file, buf, count, pos);
+   if (ret > 0) {
+   fsnotify_modify(file->f_path.dentry);
+   add_wchar(current, ret);
}
+   inc_syscw(current);
}
 
return ret;
@@ -603,9 +599,6 @@ static ssize_t do_readv_writev(int type, struct file *file,
ret = rw_verify_area(type, file, pos, tot_len);
if (ret < 0)
goto out;
-   ret = security_file_permission(file, type == READ ? MAY_READ : 
MAY_

Re: [TOMOYO #6 retry 08/21] Utility functions and policy manipulationinterface.

2008-01-11 Thread James Morris
On Sat, 12 Jan 2008, Tetsuo Handa wrote:

> Hello.
> 
> James Morris wrote:
> > > > TOMOYO Linux uses /sys/kernel/security/tomoyo interface for 
> > > > configuration.
> > > 
> > > Why aren't you using securityfs for this?  (It was designed for LSMs).
> > 
> > Doh, it is using securityfs, don't worry.
> > 
> I got a mm-commits mail titled
> "+ sysfs-make-sysfs_deprecated-depend-on-sysfs.patch added to -mm tree" .
> 
> Is sysfs going to be deprecated?

Unlikely -- what's in the patches?


- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH] Adding prctl override support for LSMs

2008-01-10 Thread James Morris
On Wed, 9 Jan 2008, Andrew Morgan wrote:

> So far the objection (thanks Stephen for the historical context!) seems
> to be "potential for abuse":
> 
> ~  <[EMAIL PROTECTED]>
>   [PATCH] remove sys_security
> 
>   I've been auditing the LSM stuff a bit more..
> 
>   They have registered an implemented a syscall, sys_security
>   that does nothing but switch into the individual modules
>   based on the first argument, i.e. it's ioctl() switching
>   on the security module instead of device node.  Yuck.
> 
>   Patch below removes it (no intree users), maybe selinux/etc
>   folks should send their actual syscall for review instead..
> 
> Since SELinux is now 'in-tree', is this class of objection now moot?

Class of objection to a sys_security, or to the prctl override?

If the former, I think it would still be considered a poor option, as 
multiplexor syscalls are generally seen as such for several reasons.  I 
don't think SELinux would need to use it now if it came back.

As Stephen mentioned, the prctl override might also be seen as a means to 
revector/hijack the syscall.

I should mention that I'm still not clear on why you need to have a 
permissive version of this hook.


- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [TOMOYO #6 retry 08/21] Utility functions and policy manipulation interface.

2008-01-08 Thread James Morris
On Wed, 9 Jan 2008, James Morris wrote:

> On Wed, 9 Jan 2008, Kentaro Takeda wrote:
> 
> > Common functions for TOMOYO Linux.
> > 
> > TOMOYO Linux uses /sys/kernel/security/tomoyo interface for configuration.
> 
> Why aren't you using securityfs for this?  (It was designed for LSMs).

Doh, it is using securityfs, don't worry.


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [TOMOYO #6 retry 08/21] Utility functions and policy manipulation interface.

2008-01-08 Thread James Morris
On Wed, 9 Jan 2008, Kentaro Takeda wrote:

> Common functions for TOMOYO Linux.
> 
> TOMOYO Linux uses /sys/kernel/security/tomoyo interface for configuration.

Why aren't you using securityfs for this?  (It was designed for LSMs).


- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Exporting capability code/name pairs

2008-01-02 Thread James Morris
On Wed, 2 Jan 2008, KaiGai Kohei wrote:

> > Another issue is that securityfs depends on CONFIG_SECURITY, which might be
> > undesirable, given that capabilities are a standard feature.
> 
> We can implement this feature on another pseudo filesystems.
> Do you think what filesystem is the best candidate?
> I prefer procfs or sysfs instead.

Sysfs makes more sense, as this information is system-wide and does not 
relate to specific processes.


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] security: remove security_sb_post_mountroot hook

2007-12-29 Thread James Morris
On Sat, 29 Dec 2007, H. Peter Anvin wrote:

> The security_sb_post_mountroot() hook is long-since obsolete, and is
> fundamentally broken: it is never invoked if someone uses initramfs.
> This is particularly damaging, because the existence of this hook has
> been used as motivation for not using initramfs.
> 
> Stephen Smalley confirmed on 2007-07-19 that this hook was originally
> used by SELinux but can now be safely removed:
> 
>  http://marc.info/?l=linux-kernel&m=118485683612916&w=2

Thanks.

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/selinux-2.6.git#for-akpm


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Exporting capability code/name pairs

2007-12-28 Thread James Morris
On Fri, 28 Dec 2007, KaiGai Kohei wrote:

> Remaining issues:
> - We have to mount securityfs explicitly, or use /etc/fstab.
>   It can cause a matter when we want to use this feature on
>   very early phase on boot. (like /sbin/init)

Why can't early userspace itself mount securityfs?

I'm not even sure this is a good idea at all.  Existing capabilities will 
never disappear, and, as with syscalls, it's probably up to userland to 
handle new ones not existing.

In any case, some more technical issues:

> kernel/cap_names.sh generates the body of cap_entries[] array,

This needs to be in the scripts directory.

The generated header should be made idempotent (#ifdef wrapping), and also 
include a warning that it is automatically generated (identifying the 
script which does so), and that is should not be edited.

> + d_caps = securityfs_create_dir("capability", NULL);
> + if (!d_caps)

Wrong way to check for error -- the function returns an ERR_PTR().

> + f_caps[i] = securityfs_create_file(cap_entries[i].name, 0444,
> +d_caps, &cap_entries[i],
> +&cap_entry_fops);
> + if (!f_caps[i])

Ditto.

Another issue is that securityfs depends on CONFIG_SECURITY, which might 
be undesirable, given that capabilities are a standard feature.


- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Exporting capability code/name pairs

2007-12-27 Thread James Morris
On Fri, 28 Dec 2007, KaiGai Kohei wrote:

> + snprintf(tmp, sizeof(tmp),
> +  cap_entry == &cap_entries[0] ? "0x%08x" :  "%u",
> +  cap_entry->code);
> + len = strlen(tmp);

You don't need to call strlen(), just use scnprintf() and grab the return 
value.


- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Exporting capability code/name pairs

2007-12-27 Thread James Morris
On Thu, 27 Dec 2007, KaiGai Kohei wrote:


(Please put the patch above the .sig separator).

+   len = strlen(tmp);
+
+   if (ofs >= len)
+   return 0;
+
+   if (len - ofs < count)
+   count = len - ofs;
+
+   rc = copy_to_user(buffer, tmp + ofs, count);
+   if (rc)
+   return rc;
+
+   *ppos += count;

Use simple_read_from_buffer().



- James
-- 
James Morris
<[EMAIL PROTECTED]>

-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch, rfc] mm.h, security.h, key.h and preventing namespace poisoning

2007-12-25 Thread James Morris
On Tue, 25 Dec 2007, Andrew Morton wrote:

> On Thu, 20 Dec 2007 15:11:40 +1100 (EST) James Morris <[EMAIL PROTECTED]> 
> wrote:
> 
> > > > +#ifdef CONFIG_SECURITY
> > > > +extern unsigned long mmap_min_addr;
> > > > +#endif
> > > > +
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > 
> > > Fine by me.
> > 
> > I'll queue it for -mm & 2.6.25.
> 
> I don't think we need the ifdef.  If someone incorrectly refers to this
> then they'll get a link-time error rather than a compile-time error (bad). 
> But we lose an ifdef (good).

Done, & rebased the git branch.


- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch, rfc] mm.h, security.h, key.h and preventing namespace poisoning

2007-12-19 Thread James Morris
On Thu, 20 Dec 2007, David Chinner wrote:

> > I'm not sure I understand your namespace pollution issue, either.
> 
> doing this globally:
> 
> #ifdef CONFIG_SOMETHING
> extern intsome_common_name(int a, int b, int c);
> #else
> #define some_common_name(a,b,c)   0
> #endif

I suspect it may be useful ensure all global identifiers for the key 
subsystem are prefixed with key_, as 'copy_keys' does seem a little 
generic.

> > +#ifdef CONFIG_SECURITY
> > +extern unsigned long mmap_min_addr;
> > +#endif
> > +
> >  #include 
> >  #include 
> >  #include 
> 
> Fine by me.

I'll queue it for -mm & 2.6.25.


- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch, rfc] mm.h, security.h, key.h and preventing namespace poisoning

2007-12-19 Thread James Morris
On Wed, 19 Dec 2007, David Chinner wrote:

> Folks,
> 
> I just updated a git tree and started getting errors on a
> "copy_keys" macro warning.
> 
> The code I've been working on uses a ->copy_keys() method for
> copying the keys in a btree block from one place to another. I've
> been working on this code for a while
> (http://oss.sgi.com/archives/xfs/2007-11/msg00046.html) and keep the
> tree I'm working in reletively up to date (lags linus by a couple of
> weeks at most). The update I did this afternoon gave a conflict
> warning with the macro in include/linux/key.h.
> 
> Given that I'm not directly including key.h anywhere in the XFS
> code, I'm getting the namespace polluted indirectly from some other
> include that is necessary.
> 
> As it turns out, this commit from 13 days ago:
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=7cd94146cd504016315608e297219f9fb7b1413b
> 
> included security.h in mm.h and that is how I'm seeing the namespace
> poisoning coming from key.h when !CONFIG_KEY.
> 
> Including security.h in mm.h means much wider includes for pretty
> much the entire kernel, and it opens up namespace issues like this
> that never previously existed.
> 
> The patch below (only tested for !CONFIG_KEYS && !CONFIG_SECURITY)
> moves security.h into the mmap.c and nommu.c files that need it so
> it doesn't end up with kernel wide scope.
> 
> Comments?

The idea with this placement was to keep memory management code with other 
similar code, rather than pushing it into security.h, where it does not 
functionally belong.

Something to not also is that you can't "depend" on security.h not being 
included all over the place, as LSM does touch a lot of the kernel.  
Unecessarily including it is bad, of course.

I'm not sure I understand your namespace pollution issue, either.

In any case, I think the right solution is not to include security.h at 
all in mm.h, as it is only being done to get a declaration for 
mmap_min_addr.

How about this, instead ?

Signed-off-by: James Morris <[EMAIL PROTECTED]>
---

 mm.h |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 1b7b95c..02fbac7 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -12,7 +12,6 @@
 #include 
 #include 
 #include 
-#include 
 
 struct mempolicy;
 struct anon_vma;
@@ -34,6 +33,10 @@ extern int sysctl_legacy_va_layout;
 #define sysctl_legacy_va_layout 0
 #endif
 
+#ifdef CONFIG_SECURITY
+extern unsigned long mmap_min_addr;
+#endif
+
 #include 
 #include 
 #include 


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH v8 05/18] LSM: Add secctx_to_secid() LSM hook

2007-12-18 Thread James Morris
On Fri, 14 Dec 2007, Paul Moore wrote:

> Add a secctx_to_secid() LSM hook to go along with the existing
> secid_to_secctx() LSM hook.  This patch also includes the SELinux
> implementation for this hook.

Please sign off your patches (especially this one asap, so I can push it 
upstream).


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH v8 10/18] SELinux: Add a network node caching mechanism similar to the sel_netif_*() functions

2007-12-18 Thread James Morris
On Mon, 17 Dec 2007, Paul Moore wrote:

> On Monday 17 December 2007 3:35:28 pm Stephen Smalley wrote:
> > On Fri, 2007-12-14 at 16:50 -0500, Paul Moore wrote:
> > > This patch adds a SELinux IP address/node SID caching mechanism similar
> > > to the sel_netif_*() functions.  The node SID queries in the SELinux
> > > hooks files are also modified to take advantage of this new
> > > functionality.  In addition, remove the address length information from
> > > the sk_buff parsing routines as it is redundant since we already have the
> > > address family.
> >
> > This is very nice - we also need the same kind of cache for port SIDs.
> 
> Thanks.  Any problem if we wait until 2.6.26 for a port SID cache? 

Nope.



-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] VM/Security: add security hook to do_brk

2007-12-05 Thread James Morris
On Tue, 4 Dec 2007, Alan Cox wrote:

> On Tue, Dec 04, 2007 at 11:06:55AM -0500, Eric Paris wrote:
> > Given a specifically crafted binary do_brk() can be used to get low
> > pages available in userspace virtually memory and can thus be used to
> > circumvent the mmap_min_addr low memory protection.  Add security checks
> > in do_brk().
> > 
> > Signed-off-by: Eric Paris <[EMAIL PROTECTED]>
> 
> ACK

Applied to
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/selinux-2.6.git#for-akpm

-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Out of tree module using LSM

2007-11-30 Thread James Morris
On Fri, 30 Nov 2007, Crispin Cowan wrote:

> > The only case of this so far has been Multiadm, although there seems to be 
> > no reason for it to stay out of tree.
> >   
> Dazuko. It has the same yucky code issues as Talpa, but AFAIK is pure
> GPL2 and thus is clean on the license issues.
>
> That these modules are valid modules that users want to use, are GPL
> clean, and are *not* something LKML wants to upstream because of code
> issues, is precisely why the LSM interface makes sense.

I think the idea is to try and fix code quality issues through 
participation in the upstream process, rather than have upstream maintain 
stable kernel APIs which are naturally mismatched to the unknown 
requirements of out of tree users.

- James
-- 
James Morris 
<[EMAIL PROTECTED]>

-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Out of tree module using LSM

2007-11-30 Thread James Morris
On Fri, 30 Nov 2007, Crispin Cowan wrote:

> restored faces a lot of challenges, but I hope that some kind of
> solution can be found, because the alternative is to effectively force
> vendors like Sophos to do it the "dirty" way by fishing in memory for
> the syscall table.

I don't think this is quite correct.

The alternative is to engage with the kernel community to become part of 
the development process, to ensure that appropriate APIs are implemented, 
and to get code upstream before shipping it.

In any case, a patch to revert the dynamic aspect of LSM has been posted 
by Arjan (and acked by myself) for the case of valid out of tree users.  
The only case of this so far has been Multiadm, although there seems to be 
no reason for it to stay out of tree.



- James
-- 
James Morris 
<[EMAIL PROTECTED]>

-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3 -v2] mmap: round mmap hint address above mmap_min_addr

2007-11-26 Thread James Morris
On Mon, 26 Nov 2007, Eric Paris wrote:

> If mmap_min_addr is set and a process attempts to mmap (not fixed) with
> a non-null hint address less than mmap_min_addr the mapping will fail
> the security checks.  Since this is just a hint address this patch will
> round such a hint address above mmap_min_addr.
> 
> gcj was found to try to be very frugal with vm usage and give hint
> addresses in the 8k-32k range.  Without this patch all such programs
> failed and with the patch they happily get a higher address.
> 
> This patch is wrappad in CONFIG_SECURITY since mmap_min_addr doesn't
> exist without it and there would be no security check possible no matter
> what.  So we should not bother compiling in this rounding if it is just a
> waste of time.
> 
> Signed-off-by: Eric Paris <[EMAIL PROTECTED]>

Applied to 
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/selinux-2.6.git#for-akpm


> 
> ---
> 
>  include/linux/mm.h |   16 
>  mm/mmap.c  |3 +++
>  mm/nommu.c |3 +++
>  3 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 520238c..1b7b95c 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -12,6 +12,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  struct mempolicy;
>  struct anon_vma;
> @@ -513,6 +514,21 @@ static inline void set_page_links(struct page *page, 
> enum zone_type zone,
>  }
>  
>  /*
> + * If a hint addr is less than mmap_min_addr change hint to be as
> + * low as possible but still greater than mmap_min_addr
> + */
> +static inline unsigned long round_hint_to_min(unsigned long hint)
> +{
> +#ifdef CONFIG_SECURITY
> + hint &= PAGE_MASK;
> + if (((void *)hint != NULL) &&
> + (hint < mmap_min_addr))
> + return PAGE_ALIGN(mmap_min_addr);
> +#endif
> + return hint;
> +}
> +
> +/*
>   * Some inline functions in vmstat.h depend on page_zone()
>   */
>  #include 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 938313c..f4cfc6a 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -912,6 +912,9 @@ unsigned long do_mmap_pgoff(struct file * file, unsigned 
> long addr,
>   if (!len)
>   return -EINVAL;
>  
> + if (!(flags & MAP_FIXED))
> + addr = round_hint_to_min(addr);
> +
>   error = arch_mmap_check(addr, len, flags);
>   if (error)
>   return error;
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 35622c5..b989cb9 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -829,6 +829,9 @@ unsigned long do_mmap_pgoff(struct file *file,
>   void *result;
>   int ret;
>  
> + if (!(flags & MAP_FIXED))
> + addr = round_hint_to_min(addr);
> +
>   /* decide whether we should attempt the mapping, and if so what sort of
>* mapping */
>   ret = validate_mmap_request(file, addr, len, prot, flags, pgoff,
> 
> 

-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] mmap: protect from stack expantion into low vm addresses

2007-11-26 Thread James Morris
On Mon, 26 Nov 2007, Eric Paris wrote:

> Add security checks to make sure we are not attempting to expand the
> stack into memory protected by mmap_min_addr
> 
> Signed-off-by: Eric Paris <[EMAIL PROTECTED]>

Please include the LSMs list in the CC line (added again) for posts 
relating to security.

Applied to 
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/selinux-2.6.git#for-akpm

> ---
> 
> ** Be very careful applying/rediffing this patch.  Standard 3 lines of
> context from git diff will misapply the first hunk to expand_upwards
> instead of properly in expand_downwards.  This patch was generated using
> -U 4 to make sure it applies in the correct place.  **

Seems to have applied correctly for me.


> 
>  mm/mmap.c |8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> --- kernel-1/mm/mmap.c
> +++ kernel-2/mm/mmap.c
> @@ -1614,17 +1614,21 @@ static inline int expand_downwards(struc
>* so that the anon_vma locking is not a noop.
>*/
>   if (unlikely(anon_vma_prepare(vma)))
>   return -ENOMEM;
> +
> + address &= PAGE_MASK;
> + error = security_file_mmap(0, 0, 0, 0, address, 1);
> + if (error)
> + return error;
> +
>   anon_vma_lock(vma);
>  
>   /*
>* vma->vm_start/vm_end cannot change under us because the caller
>* is required to hold the mmap_sem in read mode.  We need the
>* anon_vma lock to serialize against concurrent expand_stacks.
>*/
> - address &= PAGE_MASK;
> - error = 0;
>  
>   /* Somebody else might have raced and expanded it already */
>   if (address < vma->vm_start) {
>   unsigned long size, grow;
> 
> 

-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH -mm 2/2] do_wait: cleanup delay_group_leader() usage

2007-11-26 Thread James Morris
[added LSM list]

On Fri, 23 Nov 2007, Oleg Nesterov wrote:

> eligible_child() == 2 means delay_group_leader(). With the previous patch this
> only matters for EXIT_ZOMBIE task, we can move that special check to the only
> place it is really needed.
> 
> Also, with this patch we don't skip security_task_wait() for the group leaders
> in a non-empty thread group. I don't really understand the exact semantics of
> security_task_wait(), but imho this change is a bugfix.

The semantics in terms of SELinux are that calls to wait() type calls need 
to be mediated by policy, as information can be transfered via the exit 
status.

It looks like a correct bugfix to me, but I'd be interested to see what 
others have to say.



> Also rearrange the code a bit to kill an ugly "check_continued" backdoor.
> 
> Signed-off-by: Oleg Nesterov <[EMAIL PROTECTED]>
> 
> --- PT/kernel/exit.c~6_delay_leader   2007-11-23 20:31:21.0 +0300
> +++ PT/kernel/exit.c  2007-11-23 21:29:44.0 +0300
> @@ -1137,12 +1137,6 @@ static int eligible_child(pid_t pid, int
>   if (((p->exit_signal != SIGCHLD) ^ ((options & __WCLONE) != 0))
>   && !(options & __WALL))
>   return 0;
> - /*
> -  * Do not consider thread group leaders that are
> -  * in a non-empty thread group:
> -  */
> - if (delay_group_leader(p))
> - return 2;
>  
>   err = security_task_wait(p);
>   if (err)
> @@ -1494,10 +1488,9 @@ repeat:
>   tsk = current;
>   do {
>   struct task_struct *p;
> - int ret;
>  
>   list_for_each_entry(p, &tsk->children, sibling) {
> - ret = eligible_child(pid, options, p);
> + int ret = eligible_child(pid, options, p);
>   if (!ret)
>   continue;
>  
> @@ -1521,19 +1514,17 @@ repeat:
>   retval = wait_task_stopped(p,
>   (options & WNOWAIT), infop,
>   stat_addr, ru);
> - } else if (p->exit_state == EXIT_ZOMBIE) {
> + } else if (p->exit_state == EXIT_ZOMBIE &&
> + !delay_group_leader(p)) {
>   /*
> -  * Eligible but we cannot release it yet:
> +  * We don't reap group leaders with subthreads.
>*/
> - if (ret == 2)
> - goto check_continued;
>   if (!likely(options & WEXITED))
>   continue;
>   retval = wait_task_zombie(p,
>   (options & WNOWAIT), infop,
>   stat_addr, ru);
>   } else if (p->exit_state != EXIT_DEAD) {
> -check_continued:
>   /*
>* It's running now, so it might later
>* exit, stop, or stop and then continue.
> 

-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH -mm 2/2] do_wait: cleanup delay_group_leader() usage (fwd)

2007-11-26 Thread James Morris
Any thoughts on this?

-- Forwarded message --
Date: Fri, 23 Nov 2007 22:59:42 +0300
From: Oleg Nesterov <[EMAIL PROTECTED]>
To: Andrew Morton <[EMAIL PROTECTED]>
Cc: Eric Paris <[EMAIL PROTECTED]>, James Morris <[EMAIL PROTECTED]>,
Roland McGrath <[EMAIL PROTECTED]>, [EMAIL PROTECTED]
Subject: [PATCH -mm 2/2] do_wait: cleanup delay_group_leader() usage

eligible_child() == 2 means delay_group_leader(). With the previous patch this
only matters for EXIT_ZOMBIE task, we can move that special check to the only
place it is really needed.

Also, with this patch we don't skip security_task_wait() for the group leaders
in a non-empty thread group. I don't really understand the exact semantics of
security_task_wait(), but imho this change is a bugfix.

Also rearrange the code a bit to kill an ugly "check_continued" backdoor.

Signed-off-by: Oleg Nesterov <[EMAIL PROTECTED]>

--- PT/kernel/exit.c~6_delay_leader 2007-11-23 20:31:21.0 +0300
+++ PT/kernel/exit.c2007-11-23 21:29:44.0 +0300
@@ -1137,12 +1137,6 @@ static int eligible_child(pid_t pid, int
if (((p->exit_signal != SIGCHLD) ^ ((options & __WCLONE) != 0))
&& !(options & __WALL))
return 0;
-   /*
-* Do not consider thread group leaders that are
-* in a non-empty thread group:
-*/
-   if (delay_group_leader(p))
-   return 2;
 
err = security_task_wait(p);
if (err)
@@ -1494,10 +1488,9 @@ repeat:
tsk = current;
do {
struct task_struct *p;
-   int ret;
 
list_for_each_entry(p, &tsk->children, sibling) {
-   ret = eligible_child(pid, options, p);
+   int ret = eligible_child(pid, options, p);
if (!ret)
continue;
 
@@ -1521,19 +1514,17 @@ repeat:
retval = wait_task_stopped(p,
(options & WNOWAIT), infop,
stat_addr, ru);
-   } else if (p->exit_state == EXIT_ZOMBIE) {
+   } else if (p->exit_state == EXIT_ZOMBIE &&
+   !delay_group_leader(p)) {
/*
-* Eligible but we cannot release it yet:
+* We don't reap group leaders with subthreads.
 */
-   if (ret == 2)
-   goto check_continued;
if (!likely(options & WEXITED))
continue;
retval = wait_task_zombie(p,
(options & WNOWAIT), infop,
stat_addr, ru);
} else if (p->exit_state != EXIT_DEAD) {
-check_continued:
/*
 * It's running now, so it might later
 * exit, stop, or stop and then continue.
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH net-2.6.25] Add packet filtering based on process's security context.

2007-11-22 Thread James Morris
On Thu, 22 Nov 2007, Tetsuo Handa wrote:

> This patch allows LSM modules filter incoming connections/datagrams
> based on the process's security context who is attempting to pick up.
> 
> There are already hooks to filter incoming connections/datagrams
> based on the socket's security context, but these hooks are not
> applicable when one wants to do TCP Wrapper-like filtering
> (e.g. App1 is permitted to accept TCP connections from 192.168.0.0/16).

This functionality looks like it could be useful in that we currently have 
no direct security mapping from incoming packet to user process, but only 
to the receiving socket, as you mention.  For SELinux, it may help us 
simplify/clarify policy.

It's also been long-desired for netfilter/iptables, to allow ipt_owner to 
work cleanly for incoming packets.

So, this probably needs to be implemented in a way which works for both LSM 
and netfilter.  There have been several discussions on the issue from the 
netfilter side, although I don't know what the latest status of that is 
(I've expanded the cc list to hopefully get some more feedback).

>From memory, one approach under discussion was to add netfilter hooks to 
the transport layer, which could be invoked correctly by each type of 
protocol when the target process is selected.

If this is done for netfilter, then an LSM hook is probably not needed at 
all, as security modules can utilize netfilter hooks directly.

> Precautions: This approach has a side effect which unlikely occurs.
> 
> If a socket is shared by multiple processes with differnt policy,
> the process who should be able to accept this connection
> will not be able to accept this connection
> because socket_post_accept() aborts this connection.
> But if socket_post_accept() doesn't abort this connection,
> the process who must not be able to accept this connection
> will repeat accept() forever, which is a worse side effect.
> 
> Similarly, if a socket is shared by multiple processes with differnt policy,
> the process who should be able to pick up this datagram
> will not be able to pick up this datagram
> because socket_post_recv_datagram() discards this datagram.
> But if socket_post_recv_datagram() doesn't discard this datagram,
> the process who must not be able to pick up this datagram
> will repeat recvmsg() forever, which is a worse side effect.
> 
> So, don't give different permissions between processes who share one socket.
> Otherwise, some connections/datagrams cannot be delivered to intended process.

These semantics changes are concerning, and lead me to wonder if there are 
any more.  Needs more review by networking folk.



- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [TOMOYO #5 18/18] LSM expansion for TOMOYO Linux.

2007-11-19 Thread James Morris
On Tue, 20 Nov 2007, Tetsuo Handa wrote:

> Hello.
> 
> Paul Moore wrote:
> > My apologies, I mistakenly read the following if statement in your patch:
> > 
> >  +   if (skb == skb_peek(&sk->sk_receive_queue)) {
> >  +   __skb_unlink(skb, &sk->sk_receive_queue);
> >  +   atomic_dec(&skb->users);
> >  +   }
> > 
> > I read the conditional as the following:
> > 
> >  +   if (skb = skb_peek(&sk->sk_receive_queue)) {
> > 
> > ... which would have caused the problems I was describing.  I'm sorry for 
> > all 
> > of the confusion/frustration, you patient explanations are correct; I was 
> > wrong in this particular case.
> No problem.
> 
> 
> 
> To everyone:
> 
>   Are there any remaining worries with 
> skb_recv_datagram()/socket_post_accept()?
> 
>   If nobody has objection, I'd like to cut these 
> skb_recv_datagram()/socket_post_accept() changes
>   and submit to -mm tree.

You should send anything which touches core networking to netdev, too, and 
get an ack from one of the core developers there.


-- 
James Morris <[EMAIL PROTECTED]>

-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH v6 09/13] SELinux: Better integration between peer labeling subsystems

2007-11-11 Thread James Morris
On Fri, 9 Nov 2007, Paul Moore wrote:

> + /* Between selinux_compat_net and selinux_policycap_netpeer this is
> +  * starting to get a bit messy - we need to setup a timetable for
> +  * deprecating some of this old/obsolete functionality so we can
> +  * reclaim some level of sanity in this function. */

I don't think we can do anything which could potentially break userspace 
now.

So, this one really needs to be right :-)


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH v6 08/13] SELinux: Add new peer permissions to the Flask definitions

2007-11-11 Thread James Morris
On Fri, 9 Nov 2007, Paul Moore wrote:

> Add additional Flask definitions to support the new "peer" object class.

Should this be dependent on dynamic class/permission support?

Or, will these checks only be invoked if labled networking is configured?


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] NetLabel: Introduce a new kernel configuration API for NetLabel - Version 11 (2.6.24-rc2) Smack: Simplified Mandatory Access Control Kernel

2007-11-09 Thread James Morris
On Fri, 9 Nov 2007, Andrew Morton wrote:

> On Thu, 08 Nov 2007 20:48:35 -0800
> Casey Schaufler <[EMAIL PROTECTED]> wrote:
> 
> > From: Paul Moore <[EMAIL PROTECTED]>
> > 
> > Add a new set of configuration functions to the NetLabel/LSM API so that
> > LSMs can perform their own configuration of the NetLabel subsystem without
> > relying on assistance from userspace.
> > 
> > Signed-off-by: Paul Moore <[EMAIL PROTECTED]>
> 
> You sent it, so this patch needs a Signed-off-by:you, please.

Also add

Reviewed-by: James Morris <[EMAIL PROTECTED]>


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] VFS/Security: Rework inode_getsecurity and callers to return resulting buffer

2007-11-01 Thread James Morris
On Thu, 1 Nov 2007, David P. Quigley wrote:

> This patch modifies the interface to inode_getsecurity to have the function
> return a buffer containing the security blob and its length via parameters
> instead of relying on the calling function to give it an appropriately sized
> buffer. Security blobs obtained with this function should be freed using the
> release_secctx LSM hook. This alleviates the problem of the caller having to
> guess a length and preallocate a buffer for this function allowing it to be
> used elsewhere for Labeled NFS. The patch also removed the unused err
> parameter. The conversion is similar to the one performed by Al Viro for the
> security_getprocattr hook.
> 
> Signed-off-by: David P. Quigley <[EMAIL PROTECTED]>

Acked-by: James Morris <[EMAIL PROTECTED]>

-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] VFS: Reorder vfs_getxattr to avoid unnecessary calls to the LSM

2007-11-01 Thread James Morris
On Thu, 1 Nov 2007, David P. Quigley wrote:

> Originally vfs_getxattr would pull the security xattr variable using
> the inode getxattr handle and then proceed to clobber it with a subsequent 
> call
> to the LSM. This patch reorders the two operations such that when the xattr
> requested is in the security namespace it first attempts to grab the value 
> from
> the LSM directly. If it fails to obtain the value because there is no module
> present or the module does not support the operation it will fall back to 
> using
> the inode getxattr operation. In the event that both are inaccessible it
> returns EOPNOTSUPP.
> 
> Signed-off-by: David P. Quigley <[EMAIL PROTECTED]>

Acked-by: James Morris <[EMAIL PROTECTED]>


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] VFS/Security: Rework inode_getsecurity and callers to return resulting buffer

2007-10-31 Thread James Morris
On Wed, 31 Oct 2007, David P. Quigley wrote:

> On Sat, 2007-10-27 at 08:14 +1000, James Morris wrote:
> > On Fri, 26 Oct 2007, Serge E. Hallyn wrote:
> > 
> > > > It wouldn't be much effort to rebase this patch against Linus's latest
> > > > tree. I am assuming that the static lsm patch is in there based on the
> > > > recent discussion on LKML?
> > > 
> > > Oh, sorry for the two emails.
> > > 
> > > Yeah it's in 2.6.24.  So a rebase will be necessary anyway. 
> > 
> > That code may still change -- Arjan's update, for example.
> > 
> 
> What do you think should be done with this patch set? We have already
> gone past the 2.6.24 merge window so it seems as if it will have to be
> put into 2.6.25. Should I wait until 2.6.24 is released and repost it
> then? Is it possible for them to get into 2.6.24 since its early in the
> rc cycle?

Not as far as I'm aware.


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] VFS/Security: Rework inode_getsecurity and callers to return resulting buffer

2007-10-26 Thread James Morris
On Fri, 26 Oct 2007, Serge E. Hallyn wrote:

> > It wouldn't be much effort to rebase this patch against Linus's latest
> > tree. I am assuming that the static lsm patch is in there based on the
> > recent discussion on LKML?
> 
> Oh, sorry for the two emails.
> 
> Yeah it's in 2.6.24.  So a rebase will be necessary anyway. 

That code may still change -- Arjan's update, for example.

-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [AppArmor 34/45] Factor out sysctl pathname code

2007-10-26 Thread James Morris
On Thu, 25 Oct 2007, [EMAIL PROTECTED] wrote:

> Convert the selinux sysctl pathname computation code into a standalone
> function.
> 
> Signed-off-by: Andreas Gruenbacher <[EMAIL PROTECTED]>
> Signed-off-by: John Johansen <[EMAIL PROTECTED]>

Reviewed-by: James Morris <[EMAIL PROTECTED]>



-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] VFS: Reorder vfs_getxattr to avoid unnecessary calls to the LSM

2007-10-23 Thread James Morris
On Mon, 22 Oct 2007, David P. Quigley wrote:

> Originally vfs_getxattr would pull the security xattr variable using
> the inode getxattr handle and then proceed to clobber it with a subsequent 
> call
> to the LSM. This patch reorders the two operations such that when the xattr
> requested is in the security namespace it first attempts to grab the value 
> from
> the LSM directly. If it fails to obtain the value because there is no module
> present or the module does not support the operation it will fall back to 
> using
> the inode getxattr operation. In the event that both are inaccessible it
> returns EOPNOTSUPP.
> 
> Signed-off-by: David P. Quigley <[EMAIL PROTECTED]>

Acked-by: James Morris <[EMAIL PROTECTED]>


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] VFS/Security: Rework inode_getsecurity and callers to return resulting buffer

2007-10-23 Thread James Morris
On Mon, 22 Oct 2007, David P. Quigley wrote:

> +static inline int security_inode_getsecurity(const struct inode *inode,
> + const char *name,
> + void **buffer)

It's better to keep function declarations on one line if possible (the 
80-col rule can be broken for this).

But in any case, it looks ok to me.


Acked-by: James Morris <[EMAIL PROTECTED]>


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2 -mm] capabilities: introduce per-process capability bounding set (v4)

2007-10-03 Thread James Morris
On Wed, 3 Oct 2007, Serge E. Hallyn wrote:


> 
> Signed-off-by: Serge Hallyn <[EMAIL PROTECTED]>

Acked-by: James Morris <[EMAIL PROTECTED]>

-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2 -mm] capabilities: define CONFIG_COMMONCAP

2007-10-03 Thread James Morris
On Wed, 3 Oct 2007, Serge E. Hallyn wrote:

> >From 54c70ca7671750fe8986451fae91d42107d0ca90 Mon Sep 17 00:00:00 2001
> From: Serge E. Hallyn <[EMAIL PROTECTED]>
> Date: Fri, 28 Sep 2007 10:33:33 -0500
> Subject: [PATCH 1/2 -mm] capabilities: define CONFIG_COMMONCAP
> 
> currently the compilation of commoncap.c is determined
> through Makefile logic.  So there is no single CONFIG
> variable which can be relied upon to know whether it
> will be compiled.
> 
> Define CONFIG_COMMONCAP to be true when lsm is not
> compiled in, or when the capability or rootplug modules
> are compiled.  These are the cases when commoncap is
> currently compiled.  Use this variable in security/Makefile
> to determine commoncap.c's compilation.
> 
> Apart from being a logic cleanup, this is needed by the
> upcoming cap_bset patch so that prctl can know whether
> PR_SET_BSET should be allowed.
> 
> Signed-off-by: Serge E. Hallyn <[EMAIL PROTECTED]>

Acked-by: James Morris <[EMAIL PROTECTED]>


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [TOMOYO 05/15](repost) Domain transition handler functions.

2007-10-03 Thread James Morris
On Wed, 3 Oct 2007, YOSHIFUJI Hideaki / µÈÆ£±ÑÌÀ wrote:

> In article <[EMAIL PROTECTED]> (at Wed, 3 Oct 2007 23:26:57 +0900), Tetsuo 
> Handa <[EMAIL PROTECTED]> says:
> 
> > Peter Zijlstra wrote:
> > > Also, how do you avoid referencing dead data with your sll? I haven't
> > > actually looked at your patches, but the simple scheme you outlined
> > > didn't handle the iteration + concurrent removal scenario:
> > Regarding my singly-linked list, no entries are removed from the list. It's 
> > append only (like CD-R media).
> > I set is_deleted flag of a entry instead of removing the entry from the 
> > list.
> 
> It is not a good practice.  Please free such objects.
> BTW, how many objects do you have in the list?

Doesn't matter.  No list should be able to grow without bounds in the 
kernel.

-- 
James Morris
<[EMAIL PROTECTED]>

Re: [TOMOYO 05/15](repost) Domain transition handler functions.

2007-10-03 Thread James Morris
On Wed, 3 Oct 2007, YOSHIFUJI Hideaki / µÈÆ£±ÑÌÀ wrote:

> In article <[EMAIL PROTECTED]> (at Wed, 3 Oct 2007 20:24:52 +0900), Tetsuo 
> Handa <[EMAIL PROTECTED]> says:
> 
> > It seems that standard kernel list API does not have singly-linked list 
> > manipulation.
> > I'm considering the following list manipulation API.
> 
> Tstsuo, please name it "slist", which is well-known.

I'm pretty sure that the singly linked list idea has been rejected a few 
times.  Just use the existing API.

-- 
James Morris
<[EMAIL PROTECTED]>

Re: [TOMOYO 03/15](repost) Memory and pathname management functions.

2007-10-03 Thread James Morris
On Tue, 2 Oct 2007, Kentaro Takeda wrote:

> +/**
> + * tmy_alloc - allocate memory for temporary purpose.
> + * @size: requested size in bytes.
> + *
> + * Returns '\0'-initialized memory region on success.
> + * Returns NULL on failure.
> + *
> + * This function allocates memory for keeping ACL entries.
> + * The caller has to call tmy_free() the returned pointer
> + * when memory is no longer needed.
> + */

Would you please explain why you need another level of memory allocation?

What does it do apart from let you check for memory leaks?

- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [TOMOYO 14/15](repost) LSM expansion for TOMOYO Linux.

2007-10-02 Thread James Morris
On Tue, 2 Oct 2007, Kentaro Takeda wrote:


> --- linux-2.6.orig/net/core/datagram.c2007-10-02 11:11:51.0 
> +0900
> +++ linux-2.6/net/core/datagram.c 2007-10-02 11:26:23.0 +0900
> @@ -55,6 +55,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  /*
>   *   Is a socket 'connection oriented' ?
> @@ -178,6 +179,27 @@ struct sk_buff *skb_recv_datagram(struct
>   } else
>   skb = skb_dequeue(&sk->sk_receive_queue);
>  
> + error = security_post_recv_datagram(sk, skb, flags);
> + if (error) {
> + unsigned long cpu_flags;
> +
> + if (!(flags & MSG_PEEK))
> + goto no_peek;
> +
> + spin_lock_irqsave(&sk->sk_receive_queue.lock,
> +   cpu_flags);
> + if (skb == skb_peek(&sk->sk_receive_queue)) {
> + __skb_unlink(skb,
> +  &sk->sk_receive_queue);
> + atomic_dec(&skb->users);
> + }
> + spin_unlock_irqrestore(&sk->sk_receive_queue.lock,
> +cpu_flags);
> +no_peek:
> + skb_free_datagram(sk, skb);
> + goto no_packet;
> + }
> +
>   if (skb)
>   return skb;

I'm guessing you need this to determine the receiving process, rather than 
the socket (which is available via security_sock_rcv_skb()).

If so, is this to interactively determine from the user or admin whether 
the packet should be accepted/denied for the receiving process?


- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [TOMOYO 05/15](repost) Domain transition handler functions.

2007-10-02 Thread James Morris
On Tue, 2 Oct 2007, Tetsuo Handa wrote:

> Hello.
> 
> Thank you for your comment.
> 
> James Morris wrote:
> > Why do you need to avoid spinlocks for these manipulations?
> 
> I don't need to use spinlocks here.
> What I need to do here is avoid read/write reordering,
> so mb() will be appropriate here.
> 
> struct something {
>   struct something *next;
>   void *data;
> };
> 
> struct something *prev_ptr = ...;
> struct something *new_ptr = kmalloc(sizeof(*new_ptr), GFP_KERNEL);
> new_ptr->next = NULL;
> new_ptr->data = some_value;
> mb();
> prev_ptr->next = new_ptr;

You're introducing a custom API, which is open-coded repeatedly throughout 
your module.

All linked lists (at least, new ones) must use the standard kernel list
API.

> Performance is not critical because this mb() is called
> only when appending new entry to the singly-linked list.

Most of these uses appear to be slow path or nested inside other locks, 
while overall, performance is likely to be dominated by your string 
matching and permission checking.  Use of mb(), which is typically 
considered less understandable, in this case does not appear to be 
justified vs. normal locking, and you also need to use the standard list 
API.  If performance really is an issue, then consider RCU.


- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [TOMOYO 14/15](repost) LSM expansion for TOMOYO Linux.

2007-10-02 Thread James Morris
On Tue, 2 Oct 2007, Kentaro Takeda wrote:

> LSM expansion for TOMOYO Linux.
> 
> LSM hooks for sending signal:
>* task_kill_unlocked is added in sys_kill
>* task_tkill_unlocked is added in sys_tkill
>* task_tgkill_unlocked is added in sys_tgkill

Why do you need racy unlocked versions, in addition to the existing 
security_task_kill() hook which is called safely via 
check_kill_permission() ?


- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [TOMOYO 05/15](repost) Domain transition handler functions.

2007-10-02 Thread James Morris
On Tue, 2 Oct 2007, Kentaro Takeda wrote:

> +
> + mb(); /* Instead of using spinlock. */
> + ptr = domain_initializer_list;
> + if (ptr) {
> + while (ptr->next)
> + ptr = ptr->next;
> + ptr->next = new_entry;
> + } else
> + domain_initializer_list = new_entry;
> +

Please use standard kernel list handling, per include/linux/list.h

Why do you need to avoid spinlocks for these manipulations?



- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [TOMOYO 00/15](repost) TOMOYO Linux - MAC based on process invocation history.

2007-10-02 Thread James Morris
It seems that patches 1-3 are missing.

I'd also suggest making all of the patches a reply to the first email, so 
they can be threaded.



- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] capabilities: per-process capbset

2007-10-01 Thread James Morris
On Mon, 1 Oct 2007, Serge E. Hallyn wrote:

> Here is a new per-process capability bounding set patchset
> which I expect to send to linux-kernel soon.  It makes
> the capbset per-process.  A process can only permanently
> remove bits from it's bounding set, not add them.  To
> remove bits, CAP_SYS_ADMIN is currently needed.  Maybe
> that's not the best choice, but some privilege should
> probably be required.

I'm not clear on why privilege would required for a process to remove 
capability bits from its set.  (Sure, if running setuid).

Doesn't that just make it more difficult to write safe applications ?


-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Version 3 (2.6.23-rc8) Smack: Simplified Mandatory Access Control Kernel

2007-10-01 Thread James Morris
On Sun, 30 Sep 2007, Andrew Morton wrote:

> So with the information which I presently have available to me, I'm
> thinking that this should go into 2.6.24.

I think the decision to merge Smack is something that needs to be 
considered in the wider context of overall security architecture.

Smack itself looks fine.  It seems like clean code with interesting ideas, 
and appears to be based upon sound principles.

Merging Smack, however, would lock the kernel into the LSM API.  
Presently, as SELinux is the only in-tree user, LSM can still be removed.

LSM's weak semantics and pervasive deep hooking of the kernel means that 
we'll have to continue dealing with several unpleasant issues, such as the 
abuse of the API by out of tree vendors, with a proliferation of 
binary/proprietary modules which typically maladapt the API for arbitrary 
purposes and/or use dangerous techniques.  We will continue to waste 
resources maintaining this capability for them.

On a broader scale, we'll miss the potential of Linux having a coherent, 
semantically strong security architecture. I have written about this in 
some detail before: http://lwn.net/Articles/240019/

Briefly, SELinux is a security architecture.  It provides an extremely 
high degree of flexibility, in terms of both the types of security models 
implemented, and security policy for those models.  It allows controlled 
composition of different security models, with a common policy language 
framework, allowing the entire system to be analyzed.  The same ideas and 
even code can be reused beyond the kernel as post-DAC security is extended 
into databases, the desktop, distributed applications etc.  It is a 
framework which provides a structured, coherent view of the security of 
the OS (and ultimately, the entire distributed environment).

If LSM remains, security will never be a first class citizen of the 
kernel.  Application developers will see multiple security schemes, and 
either burn themselves trying to support them, or more likely, ignore 
them.
 
Core kernel developers will continue to not have enough information to 
understand what the LSM hooks in their code are supposed to be doing, 
leading to long term maintenance issues and potential security problems.

LSM will remain a magnet for bad ideas.  There's a reason we don't have 
pluggable network stacks, and we are lucky to have had a unified 
networking framework maintained by people who know to say no to things 
like STREAMS and TOE.  I would question whether this quality of 
maintainership would exist if the networking core was simply a bunch of 
deep hooks into which people dumped arbitrary custom stacks.

LSM will prevent us from making systemic improvements to security, as 
there will be no common security architecture.  Things like Netfilter 
would not have been viable with a kernel which simply provided a bunch of 
hooks for networking stacks and an assortment of implementations.

Much of this we have learned from the experience of LSM, and I think it 
has been valuable for that, but I think we need to consider now whether we 
are going to continue down this track in a permanent manner.


- James
-- 
James Morris
<[EMAIL PROTECTED]>
-
To unsubscribe from this list: send the line "unsubscribe 
linux-security-module" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


  1   2   >