* [PATCH 0/9] Open loaders and interpreters with new creds during exec
@ 2011-04-21 14:30 David Howells
2011-04-21 14:30 ` [PATCH 1/9] TOMOYO: Fix tomoyo_dentry_open() to use the right creds David Howells
` (12 more replies)
0 siblings, 13 replies; 21+ messages in thread
From: David Howells @ 2011-04-21 14:30 UTC (permalink / raw)
To: serge.hallyn, eparis; +Cc: linux-security-module, selinux
Currently, the caller must be able to open the script interpreter and/or the
binary loader that an executable wants to make use of, even if the executable
will be transitioned to a different context that can make use of that
interpreter or loader when the caller's context does not permit it.
Override credentials in open_exec() and kernel_read() with the currently
constructed new credentials so that if the executable file or its interpreter
specifies a transition to a different security context (DAC or MAC), then the
caller only has to provide access to the file to be executed, and not to the
interpreter (e.g. perl) or binary loader (e.g. ld.so) for the executable or
interpreter.
This means that if the caller does not have access to a script interpreter or
binary loader, it can still use scripts and executables that transition to a
security context that do.
Tetsuo Handa pointed out that TOMOYO had a problem because the loader image
(such as ld-config.so) was checked with the wrong credentials (open_exec() is
using the caller's credentials rather than credentials-to-be of the exec'd
process.
I note that this also applies to scripts and their interpreters. A script may
end up getting run in a context that allows access to its interpreter, but
that's no good if the caller of execve() doesn't permit the script interpreter
to be opened.
Here's a set of patches to deal with this. The last patch is the main
ingredient.
I have a couple of comments/questions on the above:
(1) Consider a SUID binary. If the loader for that binary is executable by
the uid to which the binary changes its uid on execution, but not by the
uid of the caller, should execution succeed?
For example, if, as root, I do this:
cp -v /bin/ls /tmp/ls
perl -p -i -e s/ld-linux/ld-linuQ/ /tmp/ls
cp -v /lib64/ld-linux-x86-64.so.2 /lib64/ld-linuQ-x86-64.so.2
chmod -v 0700 /lib64/ld-linuQ-x86-64.so.2
then as myself do this:
/tmp/ls
this currently fails with permission denied; but with patch (5) above
applied, it works. Obviously, this is a contrived example, but it also
may applies to script interpreters if an LSM provides a security ID
transition.
(2) Conversely, if the loader is not executable by the uid to which an SUID
executable switches, should that execution succeed?
So if I alter the ownership and permissions on the above binaries:
chown -v dhowells /lib64/ld-linuQ-x86-64.so.2
chown -v bin /tmp/ls
chmod u+s /tmp/ls
then as myself do:
/tmp/ls
this works with patches removed and gives permission denied with it
applied.
(3) The SUID/SGID changes from patch 5 that I've noted in comments (1) and (2)
should, I think, have no impact on the execution environment because:
(*) loaders and interpreters are normally accessible to everyone in
their DAC permissions (UID, GID, umask), and
(*) recursing through prepare_binprm() multiple times only takes
account of the SUID/SGID settings of the final level (which means
that whilst you can have a SUID/SGID interpreter, you can't have a
SUID/SGID script).
An exception to this are binfmt_misc cases that have
MISC_FMT_CREDENTIALS flagged - in those the credentials remain
unchanged for the iteration, even if the next executable is
SUID/SGID.
(4) SELinux's testsuite needs the following additional rules:
[policy/test_ptrace.te]
allow test_ptrace_traced_t bin_t:file { execute read open };
[policy/test_file.te]
allow fileop_t fileop_exec_t:file { execute read open };
to permit the ptrace test to access its script interpreter (perl) and the
SIGIO file test's wait_io program to access its own executable.
---
David Howells (8):
LSM: Use derived creds for accessing an executable's interpreter and binary loader
LSM: Pass linux_binprm pointer to open_exec() so creds are available
LSM: Allow an LSM to indicate that it wants bprm->file reopening with new creds
LSM: Pass linux_binprm pointer to kernel_read() so creds are available
LSM: Install the new credentials earlier in the exec procedure
LSM: Make the linux_binfmt creds pointer const and pass creds to bprm_set_creds
TOMOYO: Derive the new domain for an exec'd process in tomoyo_bprm_set_creds()
TOMOYO: Fix tomoyo_dentry_open() to use the right creds
Eric Paris (1):
LSM: Permit commit_creds() to take a const pointer
arch/alpha/kernel/binfmt_loader.c | 2
arch/x86/ia32/ia32_aout.c | 2
fs/binfmt_aout.c | 2
fs/binfmt_elf.c | 28 +++---
fs/binfmt_elf_fdpic.c | 22 +++--
fs/binfmt_em86.c | 2
fs/binfmt_flat.c | 17 +---
fs/binfmt_misc.c | 4 -
fs/binfmt_script.c | 2
fs/binfmt_som.c | 4 -
fs/coda/dir.c | 2
fs/compat.c | 2
fs/exec.c | 154 +++++++++++++++++++++++++++++++----
include/linux/binfmts.h | 12 +++
include/linux/cred.h | 3 -
include/linux/fs.h | 3 -
include/linux/security.h | 17 +++-
kernel/cred.c | 40 ++++++---
net/9p/trans_fd.c | 2
security/apparmor/domain.c | 10 ++
security/apparmor/include/domain.h | 3 -
security/commoncap.c | 24 +++--
security/integrity/ima/ima_crypto.c | 2
security/security.c | 5 +
security/selinux/hooks.c | 17 +++-
security/smack/smack_lsm.c | 11 ++-
security/tomoyo/common.h | 3 -
security/tomoyo/domain.c | 9 ++
security/tomoyo/tomoyo.c | 35 +++-----
29 files changed, 300 insertions(+), 139 deletions(-)
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/9] TOMOYO: Fix tomoyo_dentry_open() to use the right creds
2011-04-21 14:30 [PATCH 0/9] Open loaders and interpreters with new creds during exec David Howells
@ 2011-04-21 14:30 ` David Howells
2011-04-21 14:30 ` [PATCH 2/9] TOMOYO: Derive the new domain for an exec'd process in tomoyo_bprm_set_creds() David Howells
` (11 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: David Howells @ 2011-04-21 14:30 UTC (permalink / raw)
To: serge.hallyn, eparis; +Cc: linux-security-module, selinux, David Howells
tomoyo_dentry_open() must use the credentials it is given, not current's
credentials.
Signed-off-by: David Howells <dhowells@redhat.com>
---
security/tomoyo/tomoyo.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index 95d3f95..9d3a828 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -187,7 +187,7 @@ static int tomoyo_dentry_open(struct file *f, const struct cred *cred)
/* Don't check read permission here if called from do_execve(). */
if (current->in_execve)
return 0;
- return tomoyo_check_open_permission(tomoyo_domain(), &f->f_path, flags);
+ return tomoyo_check_open_permission(cred->security, &f->f_path, flags);
}
static int tomoyo_file_ioctl(struct file *file, unsigned int cmd,
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/9] TOMOYO: Derive the new domain for an exec'd process in tomoyo_bprm_set_creds()
2011-04-21 14:30 [PATCH 0/9] Open loaders and interpreters with new creds during exec David Howells
2011-04-21 14:30 ` [PATCH 1/9] TOMOYO: Fix tomoyo_dentry_open() to use the right creds David Howells
@ 2011-04-21 14:30 ` David Howells
2011-04-21 14:30 ` [PATCH 3/9] LSM: Permit commit_creds() to take a const pointer David Howells
` (10 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: David Howells @ 2011-04-21 14:30 UTC (permalink / raw)
To: serge.hallyn, eparis; +Cc: linux-security-module, selinux, David Howells
Derive the new domain for an exec'd process in tomoyo_bprm_set_creds() rather
than in tomoyo_bprm_check_security(). bprm_set_creds() is called for each new
recurse through search_binary_handler() by prepare_binprm() being called first.
Signed-off-by: David Howells <dhowells@redhat.com>
---
security/tomoyo/tomoyo.c | 25 +++++++++----------------
1 files changed, 9 insertions(+), 16 deletions(-)
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index 9d3a828..5a72868 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -39,7 +39,7 @@ static void tomoyo_cred_free(struct cred *cred)
static int tomoyo_bprm_set_creds(struct linux_binprm *bprm)
{
- int rc;
+ int rc, idx, err;
rc = cap_bprm_set_creds(bprm);
if (rc)
@@ -65,12 +65,15 @@ static int tomoyo_bprm_set_creds(struct linux_binprm *bprm)
*/
atomic_dec(&((struct tomoyo_domain_info *)
bprm->cred->security)->users);
- /*
- * Tell tomoyo_bprm_check_security() is called for the first time of an
- * execve operation.
+
+ /* Check that the caller has execute permission on the program they
+ * actually asked to run and install the new domain into the
+ * credentials being constructed.
*/
- bprm->cred->security = NULL;
- return 0;
+ idx = tomoyo_read_lock();
+ err = tomoyo_find_next_domain(bprm);
+ tomoyo_read_unlock(idx);
+ return err;
}
static int tomoyo_bprm_check_security(struct linux_binprm *bprm)
@@ -78,16 +81,6 @@ static int tomoyo_bprm_check_security(struct linux_binprm *bprm)
struct tomoyo_domain_info *domain = bprm->cred->security;
/*
- * Execute permission is checked against pathname passed to do_execve()
- * using current domain.
- */
- if (!domain) {
- const int idx = tomoyo_read_lock();
- const int err = tomoyo_find_next_domain(bprm);
- tomoyo_read_unlock(idx);
- return err;
- }
- /*
* Read permission is checked against interpreters using next domain.
*/
return tomoyo_check_open_permission(domain, &bprm->file->f_path, O_RDONLY);
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/9] LSM: Permit commit_creds() to take a const pointer
2011-04-21 14:30 [PATCH 0/9] Open loaders and interpreters with new creds during exec David Howells
2011-04-21 14:30 ` [PATCH 1/9] TOMOYO: Fix tomoyo_dentry_open() to use the right creds David Howells
2011-04-21 14:30 ` [PATCH 2/9] TOMOYO: Derive the new domain for an exec'd process in tomoyo_bprm_set_creds() David Howells
@ 2011-04-21 14:30 ` David Howells
2011-04-21 14:30 ` [PATCH 4/9] LSM: Make the linux_binfmt creds pointer const and pass creds to bprm_set_creds David Howells
` (9 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: David Howells @ 2011-04-21 14:30 UTC (permalink / raw)
To: serge.hallyn, eparis
Cc: linux-security-module, selinux, David Howells, Eric Paris
From: Eric Paris <eparis@redhat.com>
Permit commit_creds() to take a pointer to an already published set of
credentials (which will have a const pointer). commit_creds() doesn't actually
modify the credentials it is given (barring the usage count), so this shouldn't
be a problem.
Signed-off-by: David Howells <dhowells@redhat.com>
Cc: Eric Paris <eparis@redhat.com>
---
include/linux/cred.h | 2 +-
kernel/cred.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/cred.h b/include/linux/cred.h
index 9aeeb0b..6a9cec4 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -157,7 +157,7 @@ extern const struct cred *get_task_cred(struct task_struct *);
extern struct cred *cred_alloc_blank(void);
extern struct cred *prepare_creds(void);
extern struct cred *prepare_exec_creds(void);
-extern int commit_creds(struct cred *);
+extern int commit_creds(const struct cred *);
extern void abort_creds(struct cred *);
extern const struct cred *override_creds(const struct cred *);
extern void revert_creds(const struct cred *);
diff --git a/kernel/cred.c b/kernel/cred.c
index 5557b55..1873e87 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -464,7 +464,7 @@ error_put:
* Always returns 0 thus allowing this function to be tail-called at the end
* of, say, sys_setgid().
*/
-int commit_creds(struct cred *new)
+int commit_creds(const struct cred *new)
{
struct task_struct *task = current;
const struct cred *old = task->real_cred;
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/9] LSM: Make the linux_binfmt creds pointer const and pass creds to bprm_set_creds
2011-04-21 14:30 [PATCH 0/9] Open loaders and interpreters with new creds during exec David Howells
` (2 preceding siblings ...)
2011-04-21 14:30 ` [PATCH 3/9] LSM: Permit commit_creds() to take a const pointer David Howells
@ 2011-04-21 14:30 ` David Howells
2011-04-21 14:30 ` [PATCH 5/9] LSM: Install the new credentials earlier in the exec procedure David Howells
` (8 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: David Howells @ 2011-04-21 14:30 UTC (permalink / raw)
To: serge.hallyn, eparis; +Cc: linux-security-module, selinux, David Howells
Make the linux_binfmt credentials pointer const and pass a separate non-const
creds pointer to the bprm_set_creds() security ops. From this point, the
credentials in linux_binfmt can be used at any time, but must be replaced on
each transit through prepare_binfmt().
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/exec.c | 28 ++++++++++++++++++++-------
include/linux/binfmts.h | 2 +-
include/linux/cred.h | 1 +
include/linux/security.h | 12 +++++++----
kernel/cred.c | 38 +++++++++++++++++++++++-------------
security/apparmor/domain.c | 7 ++++---
security/apparmor/include/domain.h | 2 +-
security/commoncap.c | 22 ++++++++++-----------
security/security.c | 4 ++--
security/selinux/hooks.c | 6 +++---
security/smack/smack_lsm.c | 6 +++---
security/tomoyo/common.h | 2 +-
security/tomoyo/domain.c | 5 +++--
security/tomoyo/tomoyo.c | 9 ++++-----
14 files changed, 86 insertions(+), 58 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 5e62d26..f0ddc51 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1142,7 +1142,7 @@ void free_bprm(struct linux_binprm *bprm)
free_arg_pages(bprm);
if (bprm->cred) {
mutex_unlock(¤t->signal->cred_guard_mutex);
- abort_creds(bprm->cred);
+ put_cred(bprm->cred);
}
kfree(bprm);
}
@@ -1210,6 +1210,8 @@ int check_unsafe_exec(struct linux_binprm *bprm)
*/
int prepare_binprm(struct linux_binprm *bprm)
{
+ const struct cred *old;
+ struct cred *cred;
umode_t mode;
struct inode * inode = bprm->file->f_path.dentry->d_inode;
int retval;
@@ -1218,15 +1220,19 @@ int prepare_binprm(struct linux_binprm *bprm)
if (bprm->file->f_op == NULL)
return -EACCES;
+ cred = duplicate_creds(bprm->cred);
+ if (!cred)
+ return -ENOMEM;
+
/* clear any previous set[ug]id data from a previous binary */
- bprm->cred->euid = current_euid();
- bprm->cred->egid = current_egid();
+ cred->euid = current_euid();
+ cred->egid = current_egid();
if (!(bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)) {
/* Set-uid? */
if (mode & S_ISUID) {
bprm->per_clear |= PER_CLEAR_ON_SETID;
- bprm->cred->euid = inode->i_uid;
+ cred->euid = inode->i_uid;
}
/* Set-gid? */
@@ -1237,15 +1243,23 @@ int prepare_binprm(struct linux_binprm *bprm)
*/
if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) {
bprm->per_clear |= PER_CLEAR_ON_SETID;
- bprm->cred->egid = inode->i_gid;
+ cred->egid = inode->i_gid;
}
}
/* fill in binprm security blob */
- retval = security_bprm_set_creds(bprm);
- if (retval)
+ retval = security_bprm_set_creds(bprm, cred);
+ if (retval) {
+ abort_creds(cred);
return retval;
+ }
+
+ /* 'commit' the new creds - after this point, the new creds may not be
+ * altered */
+ old = bprm->cred;
+ bprm->cred = cred;
bprm->cred_prepared = 1;
+ put_cred(old);
memset(bprm->buf, 0, BINPRM_BUF_SIZE);
return kernel_read(bprm->file, 0, bprm->buf, BINPRM_BUF_SIZE);
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index c3d6512..fc3bc19 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -47,7 +47,7 @@ struct linux_binprm {
#endif
unsigned int recursion_depth;
struct file * file;
- struct cred *cred; /* new credentials */
+ const struct cred *cred; /* new credentials */
int unsafe; /* how unsafe this exec is (mask of LSM_UNSAFE_*) */
unsigned int per_clear; /* bits to clear in current->personality */
int argc, envc;
diff --git a/include/linux/cred.h b/include/linux/cred.h
index 6a9cec4..37783de 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -155,6 +155,7 @@ extern void exit_creds(struct task_struct *);
extern int copy_creds(struct task_struct *, unsigned long);
extern const struct cred *get_task_cred(struct task_struct *);
extern struct cred *cred_alloc_blank(void);
+extern struct cred *duplicate_creds(const struct cred *);
extern struct cred *prepare_creds(void);
extern struct cred *prepare_exec_creds(void);
extern int commit_creds(const struct cred *);
diff --git a/include/linux/security.h b/include/linux/security.h
index ca02f17..6b99225 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -63,7 +63,7 @@ extern int cap_capset(struct cred *new, const struct cred *old,
const kernel_cap_t *effective,
const kernel_cap_t *inheritable,
const kernel_cap_t *permitted);
-extern int cap_bprm_set_creds(struct linux_binprm *bprm);
+extern int cap_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred);
extern int cap_bprm_secureexec(struct linux_binprm *bprm);
extern int cap_inode_setxattr(struct dentry *dentry, const char *name,
const void *value, size_t size, int flags);
@@ -197,6 +197,7 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
* may decide either to retain the security information saved earlier or
* to replace it.
* @bprm contains the linux_binprm structure.
+ * @cred contains the new credentials under construction at this point
* Return 0 if the hook is successful and permission is granted.
* @bprm_check_security:
* This hook mediates the point when a search for a binary handler will
@@ -1393,7 +1394,7 @@ struct security_operations {
int (*settime) (const struct timespec *ts, const struct timezone *tz);
int (*vm_enough_memory) (struct mm_struct *mm, long pages);
- int (*bprm_set_creds) (struct linux_binprm *bprm);
+ int (*bprm_set_creds) (struct linux_binprm *bprm, struct cred *cred);
int (*bprm_check_security) (struct linux_binprm *bprm);
int (*bprm_secureexec) (struct linux_binprm *bprm);
void (*bprm_committing_creds) (struct linux_binprm *bprm);
@@ -1680,7 +1681,7 @@ int security_settime(const struct timespec *ts, const struct timezone *tz);
int security_vm_enough_memory(long pages);
int security_vm_enough_memory_mm(struct mm_struct *mm, long pages);
int security_vm_enough_memory_kern(long pages);
-int security_bprm_set_creds(struct linux_binprm *bprm);
+int security_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred);
int security_bprm_check(struct linux_binprm *bprm);
void security_bprm_committing_creds(struct linux_binprm *bprm);
void security_bprm_committed_creds(struct linux_binprm *bprm);
@@ -1934,9 +1935,10 @@ static inline int security_vm_enough_memory_kern(long pages)
return cap_vm_enough_memory(current->mm, pages);
}
-static inline int security_bprm_set_creds(struct linux_binprm *bprm)
+static inline
+int security_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred)
{
- return cap_bprm_set_creds(bprm);
+ return cap_bprm_set_creds(bprm, cred);
}
static inline int security_bprm_check(struct linux_binprm *bprm)
diff --git a/kernel/cred.c b/kernel/cred.c
index 1873e87..b2a81d3 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -267,34 +267,24 @@ error:
}
/**
- * prepare_creds - Prepare a new set of credentials for modification
- *
- * Prepare a new set of task credentials for modification. A task's creds
- * shouldn't generally be modified directly, therefore this function is used to
- * prepare a new copy, which the caller then modifies and then commits by
- * calling commit_creds().
+ * duplicate_creds - Prepare a duplicate set of credentials for modification
*
- * Preparation involves making a copy of the objective creds for modification.
+ * Prepare a duplicate set of credentials for modification.
*
* Returns a pointer to the new creds-to-be if successful, NULL otherwise.
*
* Call commit_creds() or abort_creds() to clean up.
*/
-struct cred *prepare_creds(void)
+struct cred *duplicate_creds(const struct cred *old)
{
- struct task_struct *task = current;
- const struct cred *old;
struct cred *new;
- validate_process_creds();
-
new = kmem_cache_alloc(cred_jar, GFP_KERNEL);
if (!new)
return NULL;
- kdebug("prepare_creds() alloc %p", new);
+ kdebug("duplicate_creds() alloc %p", new);
- old = task->cred;
memcpy(new, old, sizeof(struct cred));
atomic_set(&new->usage, 1);
@@ -321,6 +311,26 @@ error:
abort_creds(new);
return NULL;
}
+
+/**
+ * prepare_creds - Prepare a new set of credentials for modification
+ *
+ * Prepare a new set of task credentials for modification. A task's creds
+ * shouldn't generally be modified directly, therefore this function is used to
+ * prepare a new copy, which the caller then modifies and then commits by
+ * calling commit_creds().
+ *
+ * Preparation involves making a copy of the objective creds for modification.
+ *
+ * Returns a pointer to the new creds-to-be if successful, NULL otherwise.
+ *
+ * Call commit_creds() or abort_creds() to clean up.
+ */
+struct cred *prepare_creds(void)
+{
+ validate_process_creds();
+ return duplicate_creds(current->cred);
+}
EXPORT_SYMBOL(prepare_creds);
/*
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index c825c6e..c96832e 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -337,10 +337,11 @@ static struct aa_profile *x_to_profile(struct aa_profile *profile,
/**
* apparmor_bprm_set_creds - set the new creds on the bprm struct
* @bprm: binprm for the exec (NOT NULL)
+ * @cred: the credentials under construction.
*
* Returns: %0 or error on failure
*/
-int apparmor_bprm_set_creds(struct linux_binprm *bprm)
+int apparmor_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred)
{
struct aa_task_cxt *cxt;
struct aa_profile *profile, *new_profile = NULL;
@@ -353,14 +354,14 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
bprm->file->f_path.dentry->d_inode->i_mode
};
const char *name = NULL, *target = NULL, *info = NULL;
- int error = cap_bprm_set_creds(bprm);
+ int error = cap_bprm_set_creds(bprm, cred);
if (error)
return error;
if (bprm->cred_prepared)
return 0;
- cxt = bprm->cred->security;
+ cxt = cred->security;
BUG_ON(!cxt);
profile = aa_get_profile(aa_newest_version(cxt->profile));
diff --git a/security/apparmor/include/domain.h b/security/apparmor/include/domain.h
index de04464..963a97d 100644
--- a/security/apparmor/include/domain.h
+++ b/security/apparmor/include/domain.h
@@ -23,7 +23,7 @@ struct aa_domain {
char **table;
};
-int apparmor_bprm_set_creds(struct linux_binprm *bprm);
+int apparmor_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred);
int apparmor_bprm_secureexec(struct linux_binprm *bprm);
void apparmor_bprm_committing_creds(struct linux_binprm *bprm);
void apparmor_bprm_committed_creds(struct linux_binprm *bprm);
diff --git a/security/commoncap.c b/security/commoncap.c
index f20e984..62d8c5c 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -277,9 +277,9 @@ int cap_capset(struct cred *new,
/*
* Clear proposed capability sets for execve().
*/
-static inline void bprm_clear_caps(struct linux_binprm *bprm)
+static inline void bprm_clear_caps(struct linux_binprm *bprm, struct cred *cred)
{
- cap_clear(bprm->cred->cap_permitted);
+ cap_clear(cred->cap_permitted);
bprm->cap_effective = false;
}
@@ -332,9 +332,8 @@ int cap_inode_killpriv(struct dentry *dentry)
*/
static inline int bprm_caps_from_vfs_caps(struct cpu_vfs_cap_data *caps,
struct linux_binprm *bprm,
- bool *effective)
+ bool *effective, struct cred *new)
{
- struct cred *new = bprm->cred;
unsigned i;
int ret = 0;
@@ -424,13 +423,14 @@ int get_vfs_caps_from_disk(const struct dentry *dentry, struct cpu_vfs_cap_data
* its xattrs and, if present, apply them to the proposed credentials being
* constructed by execve().
*/
-static int get_file_caps(struct linux_binprm *bprm, bool *effective)
+static int get_file_caps(struct linux_binprm *bprm, bool *effective,
+ struct cred *new)
{
struct dentry *dentry;
int rc = 0;
struct cpu_vfs_cap_data vcaps;
- bprm_clear_caps(bprm);
+ bprm_clear_caps(bprm, new);
if (!file_caps_enabled)
return 0;
@@ -450,7 +450,7 @@ static int get_file_caps(struct linux_binprm *bprm, bool *effective)
goto out;
}
- rc = bprm_caps_from_vfs_caps(&vcaps, bprm, effective);
+ rc = bprm_caps_from_vfs_caps(&vcaps, bprm, effective, new);
if (rc == -EINVAL)
printk(KERN_NOTICE "%s: cap_from_disk returned %d for %s\n",
__func__, rc, bprm->filename);
@@ -458,7 +458,7 @@ static int get_file_caps(struct linux_binprm *bprm, bool *effective)
out:
dput(dentry);
if (rc)
- bprm_clear_caps(bprm);
+ bprm_clear_caps(bprm, new);
return rc;
}
@@ -466,20 +466,20 @@ out:
/**
* cap_bprm_set_creds - Set up the proposed credentials for execve().
* @bprm: The execution parameters, including the proposed creds
+ * @new: The credentials being constructed
*
* Set up the proposed credentials for a new execution context being
* constructed by execve(). The proposed creds in @bprm->cred is altered,
* which won't take effect immediately. Returns 0 if successful, -ve on error.
*/
-int cap_bprm_set_creds(struct linux_binprm *bprm)
+int cap_bprm_set_creds(struct linux_binprm *bprm, struct cred *new)
{
const struct cred *old = current_cred();
- struct cred *new = bprm->cred;
bool effective;
int ret;
effective = false;
- ret = get_file_caps(bprm, &effective);
+ ret = get_file_caps(bprm, &effective, new);
if (ret < 0)
return ret;
diff --git a/security/security.c b/security/security.c
index 1011423..2499e94 100644
--- a/security/security.c
+++ b/security/security.c
@@ -224,9 +224,9 @@ int security_vm_enough_memory_kern(long pages)
return security_ops->vm_enough_memory(current->mm, pages);
}
-int security_bprm_set_creds(struct linux_binprm *bprm)
+int security_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred)
{
- return security_ops->bprm_set_creds(bprm);
+ return security_ops->bprm_set_creds(bprm, cred);
}
int security_bprm_check(struct linux_binprm *bprm)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index f9c3764..1b63c11 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1943,7 +1943,7 @@ static int selinux_vm_enough_memory(struct mm_struct *mm, long pages)
/* binprm security operations */
-static int selinux_bprm_set_creds(struct linux_binprm *bprm)
+static int selinux_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred)
{
const struct task_security_struct *old_tsec;
struct task_security_struct *new_tsec;
@@ -1952,7 +1952,7 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
struct inode *inode = bprm->file->f_path.dentry->d_inode;
int rc;
- rc = cap_bprm_set_creds(bprm);
+ rc = cap_bprm_set_creds(bprm, cred);
if (rc)
return rc;
@@ -1962,7 +1962,7 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
return 0;
old_tsec = current_security();
- new_tsec = bprm->cred->security;
+ new_tsec = cred->security;
isec = inode->i_security;
/* Default to the current task SID. */
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index c6f8fca..ba4c992 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -438,14 +438,14 @@ static int smack_sb_umount(struct vfsmount *mnt, int flags)
* BPRM hooks
*/
-static int smack_bprm_set_creds(struct linux_binprm *bprm)
+static int smack_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred)
{
- struct task_smack *tsp = bprm->cred->security;
+ struct task_smack *tsp = cred->security;
struct inode_smack *isp;
struct dentry *dp;
int rc;
- rc = cap_bprm_set_creds(bprm);
+ rc = cap_bprm_set_creds(bprm, cred);
if (rc != 0)
return rc;
diff --git a/security/tomoyo/common.h b/security/tomoyo/common.h
index 7c66bd8..7b4c330 100644
--- a/security/tomoyo/common.h
+++ b/security/tomoyo/common.h
@@ -851,7 +851,7 @@ int tomoyo_mkdev_perm(const u8 operation, struct path *path,
int tomoyo_path_perm(const u8 operation, struct path *path);
int tomoyo_path2_perm(const u8 operation, struct path *path1,
struct path *path2);
-int tomoyo_find_next_domain(struct linux_binprm *bprm);
+int tomoyo_find_next_domain(struct linux_binprm *bprm, struct cred *cred);
void tomoyo_print_ulong(char *buffer, const int buffer_len,
const unsigned long value, const u8 type);
diff --git a/security/tomoyo/domain.c b/security/tomoyo/domain.c
index 3538840..f76431d 100644
--- a/security/tomoyo/domain.c
+++ b/security/tomoyo/domain.c
@@ -406,12 +406,13 @@ struct tomoyo_domain_info *tomoyo_assign_domain(const char *domainname,
* tomoyo_find_next_domain - Find a domain.
*
* @bprm: Pointer to "struct linux_binprm".
+ * @cred: The credentials under construction.
*
* Returns 0 on success, negative value otherwise.
*
* Caller holds tomoyo_read_lock().
*/
-int tomoyo_find_next_domain(struct linux_binprm *bprm)
+int tomoyo_find_next_domain(struct linux_binprm *bprm, struct cred *cred)
{
struct tomoyo_request_info r;
char *tmp = kzalloc(TOMOYO_EXEC_TMPSIZE, GFP_NOFS);
@@ -534,7 +535,7 @@ int tomoyo_find_next_domain(struct linux_binprm *bprm)
domain = old_domain;
/* Update reference count on "struct tomoyo_domain_info". */
atomic_inc(&domain->users);
- bprm->cred->security = domain;
+ cred->security = domain;
if (need_kfree)
kfree(rn.name);
kfree(tmp);
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index 5a72868..6f0f325 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -37,11 +37,11 @@ static void tomoyo_cred_free(struct cred *cred)
atomic_dec(&domain->users);
}
-static int tomoyo_bprm_set_creds(struct linux_binprm *bprm)
+static int tomoyo_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred)
{
int rc, idx, err;
- rc = cap_bprm_set_creds(bprm);
+ rc = cap_bprm_set_creds(bprm, cred);
if (rc)
return rc;
@@ -63,15 +63,14 @@ static int tomoyo_bprm_set_creds(struct linux_binprm *bprm)
* stored inside "bprm->cred->security" will be acquired later inside
* tomoyo_find_next_domain().
*/
- atomic_dec(&((struct tomoyo_domain_info *)
- bprm->cred->security)->users);
+ atomic_dec(&((struct tomoyo_domain_info *)cred->security)->users);
/* Check that the caller has execute permission on the program they
* actually asked to run and install the new domain into the
* credentials being constructed.
*/
idx = tomoyo_read_lock();
- err = tomoyo_find_next_domain(bprm);
+ err = tomoyo_find_next_domain(bprm, cred);
tomoyo_read_unlock(idx);
return err;
}
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 5/9] LSM: Install the new credentials earlier in the exec procedure
2011-04-21 14:30 [PATCH 0/9] Open loaders and interpreters with new creds during exec David Howells
` (3 preceding siblings ...)
2011-04-21 14:30 ` [PATCH 4/9] LSM: Make the linux_binfmt creds pointer const and pass creds to bprm_set_creds David Howells
@ 2011-04-21 14:30 ` David Howells
2011-04-21 14:31 ` [PATCH 6/9] LSM: Pass linux_binprm pointer to kernel_read() so creds are available David Howells
` (7 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: David Howells @ 2011-04-21 14:30 UTC (permalink / raw)
To: serge.hallyn, eparis; +Cc: linux-security-module, selinux, David Howells
Install the new credentials earlier in the exec procedure, immediately after
setup_new_exec() rather than after mapping the executable and interpreter into
the new VM space.
This has the possibility that it will now fail because of security_file_mmap()
disallowing it, though Fedora 13 on my test box still boots okay and the
SELinux testsuite completes okay.
This is in preparation for a subsequent patch whereby the executable file is
reopened in the new context in prepare_binprm()and the interpreter file is
opened with the new context in open_exec().
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/binfmt_elf.c | 2 +-
fs/binfmt_elf_fdpic.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 303983f..5dd78d1 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -726,6 +726,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
current->flags |= PF_RANDOMIZE;
setup_new_exec(bprm);
+ install_exec_creds(bprm);
/* Do this so that we can load the interpreter, if need be. We will
change some of these later */
@@ -925,7 +926,6 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
}
#endif /* ARCH_HAS_SETUP_ADDITIONAL_PAGES */
- install_exec_creds(bprm);
current->flags &= ~PF_FORKNOEXEC;
retval = create_elf_tables(bprm, &loc->elf_ex,
load_addr, interp_load_addr);
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 63039ed..a7ddd4e 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -351,6 +351,8 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm,
}
#endif
+ install_exec_creds(bprm);
+
/* load the executable and interpreter into memory */
retval = elf_fdpic_map_file(&exec_params, bprm->file, current->mm,
"executable");
@@ -413,7 +415,6 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm,
current->mm->start_stack = current->mm->start_brk + stack_size;
#endif
- install_exec_creds(bprm);
current->flags &= ~PF_FORKNOEXEC;
if (create_elf_fdpic_tables(bprm, current->mm,
&exec_params, &interp_params) < 0)
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 6/9] LSM: Pass linux_binprm pointer to kernel_read() so creds are available
2011-04-21 14:30 [PATCH 0/9] Open loaders and interpreters with new creds during exec David Howells
` (4 preceding siblings ...)
2011-04-21 14:30 ` [PATCH 5/9] LSM: Install the new credentials earlier in the exec procedure David Howells
@ 2011-04-21 14:31 ` David Howells
2011-04-21 14:31 ` [PATCH 7/9] LSM: Allow an LSM to indicate that it wants bprm->file reopening with new creds David Howells
` (6 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: David Howells @ 2011-04-21 14:31 UTC (permalink / raw)
To: serge.hallyn, eparis; +Cc: linux-security-module, selinux, David Howells
Pass the exec state (struct linux_binprm) pointer to kernel_read() so new creds
are available for a later patch to use.
Also:
(1) Provide a wrapper function - exec_read_header() - for filling the bprm
buffer from a file.
(2) Move the declaration of kernel_read() to linux/binfmts.h.
(3) Make the buffer pointer of kernel_read() of 'void *' type and drop the
casts required of some callers.
(4) Stick a kerneldoc banner comment on kernel_read() describing it.
Signed-off-by: David Howells <dhowells@redhat.com>
---
arch/x86/ia32/ia32_aout.c | 2 +-
fs/binfmt_aout.c | 2 +-
fs/binfmt_elf.c | 24 +++++++++++++-----------
fs/binfmt_elf_fdpic.c | 17 +++++++++--------
fs/binfmt_misc.c | 2 +-
fs/binfmt_som.c | 4 ++--
fs/coda/dir.c | 2 +-
fs/exec.c | 18 ++++++++++++++----
include/linux/binfmts.h | 9 +++++++++
include/linux/fs.h | 1 -
net/9p/trans_fd.c | 2 +-
security/integrity/ima/ima_crypto.c | 2 +-
12 files changed, 53 insertions(+), 32 deletions(-)
diff --git a/arch/x86/ia32/ia32_aout.c b/arch/x86/ia32/ia32_aout.c
index fd84387..a8dddd2 100644
--- a/arch/x86/ia32/ia32_aout.c
+++ b/arch/x86/ia32/ia32_aout.c
@@ -445,7 +445,7 @@ static int load_aout_library(struct file *file)
inode = file->f_path.dentry->d_inode;
retval = -ENOEXEC;
- error = kernel_read(file, 0, (char *) &ex, sizeof(ex));
+ error = kernel_read(NULL, file, 0, &ex, sizeof(ex));
if (error != sizeof(ex))
goto out;
diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index a6395bd..8890161 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -379,7 +379,7 @@ static int load_aout_library(struct file *file)
inode = file->f_path.dentry->d_inode;
retval = -ENOEXEC;
- error = kernel_read(file, 0, (char *) &ex, sizeof(ex));
+ error = kernel_read(NULL, file, 0, &ex, sizeof(ex));
if (error != sizeof(ex))
goto out;
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 5dd78d1..c8060bb 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -375,7 +375,8 @@ static unsigned long total_mapping_size(struct elf_phdr *cmds, int nr)
is only provided so that we can read a.out libraries that have
an ELF header */
-static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
+static unsigned long load_elf_interp(struct linux_binprm *bprm,
+ struct elfhdr *interp_elf_ex,
struct file *interpreter, unsigned long *interp_map_addr,
unsigned long no_base)
{
@@ -415,8 +416,8 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
if (!elf_phdata)
goto out;
- retval = kernel_read(interpreter, interp_elf_ex->e_phoff,
- (char *)elf_phdata, size);
+ retval = kernel_read(NULL, interpreter, interp_elf_ex->e_phoff,
+ elf_phdata, size);
error = -EIO;
if (retval != size) {
if (retval < 0)
@@ -611,8 +612,8 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
if (!elf_phdata)
goto out;
- retval = kernel_read(bprm->file, loc->elf_ex.e_phoff,
- (char *)elf_phdata, size);
+ retval = kernel_read(bprm, bprm->file, loc->elf_ex.e_phoff,
+ elf_phdata, size);
if (retval != size) {
if (retval >= 0)
retval = -EIO;
@@ -645,7 +646,8 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
if (!elf_interpreter)
goto out_free_ph;
- retval = kernel_read(bprm->file, elf_ppnt->p_offset,
+ retval = kernel_read(bprm, bprm->file,
+ elf_ppnt->p_offset,
elf_interpreter,
elf_ppnt->p_filesz);
if (retval != elf_ppnt->p_filesz) {
@@ -671,8 +673,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
if (file_permission(interpreter, MAY_READ) < 0)
bprm->interp_flags |= BINPRM_FLAGS_ENFORCE_NONDUMP;
- retval = kernel_read(interpreter, 0, bprm->buf,
- BINPRM_BUF_SIZE);
+ retval = exec_read_header(bprm, interpreter);
if (retval != BINPRM_BUF_SIZE) {
if (retval >= 0)
retval = -EIO;
@@ -882,7 +883,8 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
if (elf_interpreter) {
unsigned long uninitialized_var(interp_map_addr);
- elf_entry = load_elf_interp(&loc->interp_elf_ex,
+ elf_entry = load_elf_interp(bprm,
+ &loc->interp_elf_ex,
interpreter,
&interp_map_addr,
load_bias);
@@ -1005,7 +1007,7 @@ static int load_elf_library(struct file *file)
struct elfhdr elf_ex;
error = -ENOEXEC;
- retval = kernel_read(file, 0, (char *)&elf_ex, sizeof(elf_ex));
+ retval = kernel_read(NULL, file, 0, &elf_ex, sizeof(elf_ex));
if (retval != sizeof(elf_ex))
goto out;
@@ -1029,7 +1031,7 @@ static int load_elf_library(struct file *file)
eppnt = elf_phdata;
error = -ENOEXEC;
- retval = kernel_read(file, elf_ex.e_phoff, (char *)eppnt, j);
+ retval = kernel_read(NULL, file, elf_ex.e_phoff, eppnt, j);
if (retval != j)
goto out_free_ph;
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index a7ddd4e..eee4f70 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -57,7 +57,8 @@ typedef char *elf_caddr_t;
MODULE_LICENSE("GPL");
static int load_elf_fdpic_binary(struct linux_binprm *, struct pt_regs *);
-static int elf_fdpic_fetch_phdrs(struct elf_fdpic_params *, struct file *);
+static int elf_fdpic_fetch_phdrs(struct linux_binprm *,
+ struct elf_fdpic_params *, struct file *);
static int elf_fdpic_map_file(struct elf_fdpic_params *, struct file *,
struct mm_struct *, const char *);
@@ -119,7 +120,8 @@ static int is_elf_fdpic(struct elfhdr *hdr, struct file *file)
/*
* read the program headers table into memory
*/
-static int elf_fdpic_fetch_phdrs(struct elf_fdpic_params *params,
+static int elf_fdpic_fetch_phdrs(struct linux_binprm *bprm,
+ struct elf_fdpic_params *params,
struct file *file)
{
struct elf32_phdr *phdr;
@@ -136,7 +138,7 @@ static int elf_fdpic_fetch_phdrs(struct elf_fdpic_params *params,
if (!params->phdrs)
return -ENOMEM;
- retval = kernel_read(file, params->hdr.e_phoff,
+ retval = kernel_read(bprm, file, params->hdr.e_phoff,
(char *) params->phdrs, size);
if (unlikely(retval != size))
return retval < 0 ? retval : -ENOEXEC;
@@ -194,7 +196,7 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm,
goto error;
/* read the program header table */
- retval = elf_fdpic_fetch_phdrs(&exec_params, bprm->file);
+ retval = elf_fdpic_fetch_phdrs(bprm, &exec_params, bprm->file);
if (retval < 0)
goto error;
@@ -216,7 +218,7 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm,
if (!interpreter_name)
goto error;
- retval = kernel_read(bprm->file,
+ retval = kernel_read(bprm, bprm->file,
phdr->p_offset,
interpreter_name,
phdr->p_filesz);
@@ -248,8 +250,7 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm,
if (file_permission(interpreter, MAY_READ) < 0)
bprm->interp_flags |= BINPRM_FLAGS_ENFORCE_NONDUMP;
- retval = kernel_read(interpreter, 0, bprm->buf,
- BINPRM_BUF_SIZE);
+ retval = exec_read_header(bprm, interpreter);
if (unlikely(retval != BINPRM_BUF_SIZE)) {
if (retval >= 0)
retval = -ENOEXEC;
@@ -281,7 +282,7 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm,
interp_params.flags = ELF_FDPIC_FLAG_PRESENT;
/* read the interpreter's program header table */
- retval = elf_fdpic_fetch_phdrs(&interp_params, interpreter);
+ retval = elf_fdpic_fetch_phdrs(bprm, &interp_params, interpreter);
if (retval < 0)
goto error;
}
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 1befe2e..5209293 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -190,7 +190,7 @@ static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs)
* done. bprm->buf is stale, update from interp_file.
*/
memset(bprm->buf, 0, BINPRM_BUF_SIZE);
- retval = kernel_read(bprm->file, 0, bprm->buf, BINPRM_BUF_SIZE);
+ retval = exec_read_header(bprm, bprm->file);
} else
retval = prepare_binprm (bprm);
diff --git a/fs/binfmt_som.c b/fs/binfmt_som.c
index cc8560f..e27f6ad 100644
--- a/fs/binfmt_som.c
+++ b/fs/binfmt_som.c
@@ -211,8 +211,8 @@ load_som_binary(struct linux_binprm * bprm, struct pt_regs * regs)
if (!hpuxhdr)
goto out;
- retval = kernel_read(bprm->file, som_ex->aux_header_location,
- (char *) hpuxhdr, size);
+ retval = kernel_read(bprm, bprm->file, som_ex->aux_header_location,
+ hpuxhdr, size);
if (retval != size) {
if (retval >= 0)
retval = -EIO;
diff --git a/fs/coda/dir.c b/fs/coda/dir.c
index 2b8dae4..e0c7f4a 100644
--- a/fs/coda/dir.c
+++ b/fs/coda/dir.c
@@ -482,7 +482,7 @@ static int coda_venus_readdir(struct file *coda_file, void *buf,
}
while (1) {
/* read entries from the directory file */
- ret = kernel_read(host_file, coda_file->f_pos - 2, (char *)vdir,
+ ret = kernel_read(NULL, host_file, coda_file->f_pos - 2, vdir,
sizeof(*vdir));
if (ret < 0) {
printk(KERN_ERR "coda readdir: read dir %s failed %d\n",
diff --git a/fs/exec.c b/fs/exec.c
index f0ddc51..84625a1 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -756,8 +756,19 @@ exit:
}
EXPORT_SYMBOL(open_exec);
-int kernel_read(struct file *file, loff_t offset,
- char *addr, unsigned long count)
+/**
+ * kernel_read - Read data from an executable
+ * @bprm: Execution state (or NULL if after the point of no return)
+ * @file: File to read from
+ * @offset: File offset to read from
+ * @addr: The buffer to read into
+ * @count: The amount of data to read
+ *
+ * Read some data from an executable
+ */
+int kernel_read(struct linux_binprm *bprm,
+ struct file *file, loff_t offset,
+ void *addr, unsigned long count)
{
mm_segment_t old_fs;
loff_t pos = offset;
@@ -770,7 +781,6 @@ int kernel_read(struct file *file, loff_t offset,
set_fs(old_fs);
return result;
}
-
EXPORT_SYMBOL(kernel_read);
static int exec_mmap(struct mm_struct *mm)
@@ -1262,7 +1272,7 @@ int prepare_binprm(struct linux_binprm *bprm)
put_cred(old);
memset(bprm->buf, 0, BINPRM_BUF_SIZE);
- return kernel_read(bprm->file, 0, bprm->buf, BINPRM_BUF_SIZE);
+ return exec_read_header(bprm, bprm->file);
}
EXPORT_SYMBOL(prepare_binprm);
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index fc3bc19..82577f7 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -134,9 +134,18 @@ extern int copy_strings_kernel(int argc, const char *const *argv,
struct linux_binprm *bprm);
extern int prepare_bprm_creds(struct linux_binprm *bprm);
extern void install_exec_creds(struct linux_binprm *bprm);
+extern int kernel_read(struct linux_binprm *, struct file *,
+ loff_t, void *, unsigned long);
+extern void install_exec_creds(struct linux_binprm *bprmw);
extern void do_coredump(long signr, int exit_code, struct pt_regs *regs);
extern void set_binfmt(struct linux_binfmt *new);
extern void free_bprm(struct linux_binprm *);
+static inline int exec_read_header(struct linux_binprm *bprm, struct file *file)
+{
+ return kernel_read(bprm, file, 0, bprm->buf, BINPRM_BUF_SIZE);
+
+}
+
#endif /* __KERNEL__ */
#endif /* _LINUX_BINFMTS_H */
diff --git a/include/linux/fs.h b/include/linux/fs.h
index dbd860a..74543c9 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2224,7 +2224,6 @@ extern struct file *create_read_pipe(struct file *f, int flags);
extern struct file *create_write_pipe(int flags);
extern void free_write_pipe(struct file *);
-extern int kernel_read(struct file *, loff_t, char *, unsigned long);
extern struct file * open_exec(const char *);
/* fs/dcache.c -- generic fs support functions */
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index aa5672b..6877035 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -277,7 +277,7 @@ static int p9_fd_read(struct p9_client *client, void *v, int len)
if (!(ts->rd->f_flags & O_NONBLOCK))
P9_DPRINTK(P9_DEBUG_ERROR, "blocking read ...\n");
- ret = kernel_read(ts->rd, ts->rd->f_pos, v, len);
+ ret = kernel_read(NULL, ts->rd, ts->rd->f_pos, v, len);
if (ret <= 0 && ret != -ERESTARTSYS && ret != -EAGAIN)
client->status = Disconnected;
return ret;
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 9b3ade7..0c93027 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -63,7 +63,7 @@ int ima_calc_hash(struct file *file, char *digest)
while (offset < i_size) {
int rbuf_len;
- rbuf_len = kernel_read(file, offset, rbuf, PAGE_SIZE);
+ rbuf_len = kernel_read(NULL, file, offset, rbuf, PAGE_SIZE);
if (rbuf_len < 0) {
rc = rbuf_len;
break;
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 7/9] LSM: Allow an LSM to indicate that it wants bprm->file reopening with new creds
2011-04-21 14:30 [PATCH 0/9] Open loaders and interpreters with new creds during exec David Howells
` (5 preceding siblings ...)
2011-04-21 14:31 ` [PATCH 6/9] LSM: Pass linux_binprm pointer to kernel_read() so creds are available David Howells
@ 2011-04-21 14:31 ` David Howells
2011-04-21 14:31 ` [PATCH 8/9] LSM: Pass linux_binprm pointer to open_exec() so creds are available David Howells
` (5 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: David Howells @ 2011-04-21 14:31 UTC (permalink / raw)
To: serge.hallyn, eparis; +Cc: linux-security-module, selinux, David Howells
Allow an LSM module's bprm_set_creds() security op to indicate that it wants
bprm->file reopening with the newly calculated credentials. This is done
towards the end of prepare_binprm() after all the security modules have had a
say, but before the bprm buffer is loaded from the file.
The LSM modules should set this flag if they changed the process security
context that will be used to access the file. The LSM policies will then need
a rule to permit a process to open, read, execute or mmap their own executable
or script interpreter image file and binary loader file.
This patch does not actually reopen the file - that is left to a subsequent
patch.
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/exec.c | 9 ++++++++-
include/linux/security.h | 15 ++++++++++-----
security/apparmor/domain.c | 7 +++++--
security/apparmor/include/domain.h | 3 ++-
security/commoncap.c | 4 +++-
security/security.c | 5 +++--
security/selinux/hooks.c | 15 ++++++++++++---
security/smack/smack_lsm.c | 9 ++++++---
security/tomoyo/common.h | 3 ++-
security/tomoyo/domain.c | 6 +++++-
security/tomoyo/tomoyo.c | 7 ++++---
11 files changed, 60 insertions(+), 23 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 84625a1..4c47395 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1224,6 +1224,7 @@ int prepare_binprm(struct linux_binprm *bprm)
struct cred *cred;
umode_t mode;
struct inode * inode = bprm->file->f_path.dentry->d_inode;
+ bool reopen_file = false;
int retval;
mode = inode->i_mode;
@@ -1243,6 +1244,7 @@ int prepare_binprm(struct linux_binprm *bprm)
if (mode & S_ISUID) {
bprm->per_clear |= PER_CLEAR_ON_SETID;
cred->euid = inode->i_uid;
+ reopen_file = true;
}
/* Set-gid? */
@@ -1254,11 +1256,12 @@ int prepare_binprm(struct linux_binprm *bprm)
if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) {
bprm->per_clear |= PER_CLEAR_ON_SETID;
cred->egid = inode->i_gid;
+ reopen_file = true;
}
}
/* fill in binprm security blob */
- retval = security_bprm_set_creds(bprm, cred);
+ retval = security_bprm_set_creds(bprm, cred, &reopen_file);
if (retval) {
abort_creds(cred);
return retval;
@@ -1271,6 +1274,10 @@ int prepare_binprm(struct linux_binprm *bprm)
bprm->cred_prepared = 1;
put_cred(old);
+ /* TODO: Reopen the executable image file if any significant security
+ * data changed during calculation of the new credentials
+ */
+
memset(bprm->buf, 0, BINPRM_BUF_SIZE);
return exec_read_header(bprm, bprm->file);
}
diff --git a/include/linux/security.h b/include/linux/security.h
index 6b99225..5555dee 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -63,7 +63,8 @@ extern int cap_capset(struct cred *new, const struct cred *old,
const kernel_cap_t *effective,
const kernel_cap_t *inheritable,
const kernel_cap_t *permitted);
-extern int cap_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred);
+extern int cap_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred,
+ bool *_reopen_file);
extern int cap_bprm_secureexec(struct linux_binprm *bprm);
extern int cap_inode_setxattr(struct dentry *dentry, const char *name,
const void *value, size_t size, int flags);
@@ -198,6 +199,7 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
* to replace it.
* @bprm contains the linux_binprm structure.
* @cred contains the new credentials under construction at this point
+ * *@_reopen_file should be set if the file needs reopening with the new creds
* Return 0 if the hook is successful and permission is granted.
* @bprm_check_security:
* This hook mediates the point when a search for a binary handler will
@@ -1394,7 +1396,8 @@ struct security_operations {
int (*settime) (const struct timespec *ts, const struct timezone *tz);
int (*vm_enough_memory) (struct mm_struct *mm, long pages);
- int (*bprm_set_creds) (struct linux_binprm *bprm, struct cred *cred);
+ int (*bprm_set_creds) (struct linux_binprm *bprm, struct cred *cred,
+ bool *_reopen_file);
int (*bprm_check_security) (struct linux_binprm *bprm);
int (*bprm_secureexec) (struct linux_binprm *bprm);
void (*bprm_committing_creds) (struct linux_binprm *bprm);
@@ -1681,7 +1684,8 @@ int security_settime(const struct timespec *ts, const struct timezone *tz);
int security_vm_enough_memory(long pages);
int security_vm_enough_memory_mm(struct mm_struct *mm, long pages);
int security_vm_enough_memory_kern(long pages);
-int security_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred);
+int security_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred,
+ bool *_reopen_file);
int security_bprm_check(struct linux_binprm *bprm);
void security_bprm_committing_creds(struct linux_binprm *bprm);
void security_bprm_committed_creds(struct linux_binprm *bprm);
@@ -1936,9 +1940,10 @@ static inline int security_vm_enough_memory_kern(long pages)
}
static inline
-int security_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred)
+int security_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred,
+ bool *_reopen_file)
{
- return cap_bprm_set_creds(bprm, cred);
+ return cap_bprm_set_creds(bprm, cred, _reopen_file);
}
static inline int security_bprm_check(struct linux_binprm *bprm)
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index c96832e..cdef6bb 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -338,10 +338,12 @@ static struct aa_profile *x_to_profile(struct aa_profile *profile,
* apparmor_bprm_set_creds - set the new creds on the bprm struct
* @bprm: binprm for the exec (NOT NULL)
* @cred: the credentials under construction.
+ * *@_reopen_file: set to true to reopen bprm->file under the new creds
*
* Returns: %0 or error on failure
*/
-int apparmor_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred)
+int apparmor_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred,
+ bool *_reopen_file)
{
struct aa_task_cxt *cxt;
struct aa_profile *profile, *new_profile = NULL;
@@ -354,7 +356,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred)
bprm->file->f_path.dentry->d_inode->i_mode
};
const char *name = NULL, *target = NULL, *info = NULL;
- int error = cap_bprm_set_creds(bprm, cred);
+ int error = cap_bprm_set_creds(bprm, cred, _reopen_file);
if (error)
return error;
@@ -498,6 +500,7 @@ x_clear:
aa_put_profile(cxt->profile);
/* transfer new profile reference will be released when cxt is freed */
cxt->profile = new_profile;
+ *_reopen_file = true;
/* clear out all temporary/transitional state from the context */
aa_put_profile(cxt->previous);
diff --git a/security/apparmor/include/domain.h b/security/apparmor/include/domain.h
index 963a97d..fda5f88 100644
--- a/security/apparmor/include/domain.h
+++ b/security/apparmor/include/domain.h
@@ -23,7 +23,8 @@ struct aa_domain {
char **table;
};
-int apparmor_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred);
+int apparmor_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred,
+ bool *_reopen_file);
int apparmor_bprm_secureexec(struct linux_binprm *bprm);
void apparmor_bprm_committing_creds(struct linux_binprm *bprm);
void apparmor_bprm_committed_creds(struct linux_binprm *bprm);
diff --git a/security/commoncap.c b/security/commoncap.c
index 62d8c5c..44567bf 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -467,12 +467,14 @@ out:
* cap_bprm_set_creds - Set up the proposed credentials for execve().
* @bprm: The execution parameters, including the proposed creds
* @new: The credentials being constructed
+ * *@_reopen_file: Set to reopen bprm->file with the new creds
*
* Set up the proposed credentials for a new execution context being
* constructed by execve(). The proposed creds in @bprm->cred is altered,
* which won't take effect immediately. Returns 0 if successful, -ve on error.
*/
-int cap_bprm_set_creds(struct linux_binprm *bprm, struct cred *new)
+int cap_bprm_set_creds(struct linux_binprm *bprm, struct cred *new,
+ bool *_reopen_file)
{
const struct cred *old = current_cred();
bool effective;
diff --git a/security/security.c b/security/security.c
index 2499e94..2de57f5 100644
--- a/security/security.c
+++ b/security/security.c
@@ -224,9 +224,10 @@ int security_vm_enough_memory_kern(long pages)
return security_ops->vm_enough_memory(current->mm, pages);
}
-int security_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred)
+int security_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred,
+ bool *_reopen_file)
{
- return security_ops->bprm_set_creds(bprm, cred);
+ return security_ops->bprm_set_creds(bprm, cred, _reopen_file);
}
int security_bprm_check(struct linux_binprm *bprm)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 1b63c11..b55d3bb 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1943,7 +1943,13 @@ static int selinux_vm_enough_memory(struct mm_struct *mm, long pages)
/* binprm security operations */
-static int selinux_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred)
+/*
+ * Calculate the new security data for the exec'd process and store it in the
+ * creds provided. If the task sid is to be changed, then the caller is asked
+ * to reopen bprm->file with the new creds.
+ */
+static int selinux_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred,
+ bool *_reopen_file)
{
const struct task_security_struct *old_tsec;
struct task_security_struct *new_tsec;
@@ -1952,7 +1958,7 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred)
struct inode *inode = bprm->file->f_path.dentry->d_inode;
int rc;
- rc = cap_bprm_set_creds(bprm, cred);
+ rc = cap_bprm_set_creds(bprm, cred, _reopen_file);
if (rc)
return rc;
@@ -2046,6 +2052,9 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred)
/* Clear any possibly unsafe personality bits on exec: */
bprm->per_clear |= PER_CLEAR_ON_SETID;
+
+ /* The file needs reopening with the new SID */
+ *_reopen_file = true;
}
return 0;
@@ -2625,7 +2634,7 @@ static int selinux_inode_readlink(struct dentry *dentry)
{
const struct cred *cred = current_cred();
- return dentry_has_perm(cred, NULL, dentry, FILE__READ);
+ return dentry_has_perm(cred, NULL, dentry, LNK_FILE__READ);
}
static int selinux_inode_follow_link(struct dentry *dentry, struct nameidata *nameidata)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index ba4c992..36a3e84 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -438,14 +438,15 @@ static int smack_sb_umount(struct vfsmount *mnt, int flags)
* BPRM hooks
*/
-static int smack_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred)
+static int smack_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred,
+ bool *_reopen_file)
{
struct task_smack *tsp = cred->security;
struct inode_smack *isp;
struct dentry *dp;
int rc;
- rc = cap_bprm_set_creds(bprm, cred);
+ rc = cap_bprm_set_creds(bprm, cred, _reopen_file);
if (rc != 0)
return rc;
@@ -462,8 +463,10 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred)
isp = dp->d_inode->i_security;
- if (isp->smk_task != NULL)
+ if (isp->smk_task != NULL && tsp->smk_task != isp->smk_task) {
+ *_reopen_file = true;
tsp->smk_task = isp->smk_task;
+ }
return 0;
}
diff --git a/security/tomoyo/common.h b/security/tomoyo/common.h
index 7b4c330..814b1ec 100644
--- a/security/tomoyo/common.h
+++ b/security/tomoyo/common.h
@@ -851,7 +851,8 @@ int tomoyo_mkdev_perm(const u8 operation, struct path *path,
int tomoyo_path_perm(const u8 operation, struct path *path);
int tomoyo_path2_perm(const u8 operation, struct path *path1,
struct path *path2);
-int tomoyo_find_next_domain(struct linux_binprm *bprm, struct cred *cred);
+int tomoyo_find_next_domain(struct linux_binprm *bprm, struct cred *cred,
+ bool *_reopen_file);
void tomoyo_print_ulong(char *buffer, const int buffer_len,
const unsigned long value, const u8 type);
diff --git a/security/tomoyo/domain.c b/security/tomoyo/domain.c
index f76431d..a33fc82 100644
--- a/security/tomoyo/domain.c
+++ b/security/tomoyo/domain.c
@@ -407,12 +407,14 @@ struct tomoyo_domain_info *tomoyo_assign_domain(const char *domainname,
*
* @bprm: Pointer to "struct linux_binprm".
* @cred: The credentials under construction.
+ * *@_reopen_file: Set to reopen bprm->file with new creds.
*
* Returns 0 on success, negative value otherwise.
*
* Caller holds tomoyo_read_lock().
*/
-int tomoyo_find_next_domain(struct linux_binprm *bprm, struct cred *cred)
+int tomoyo_find_next_domain(struct linux_binprm *bprm, struct cred *cred,
+ bool *_reopen_file)
{
struct tomoyo_request_info r;
char *tmp = kzalloc(TOMOYO_EXEC_TMPSIZE, GFP_NOFS);
@@ -533,6 +535,8 @@ int tomoyo_find_next_domain(struct linux_binprm *bprm, struct cred *cred)
out:
if (!domain)
domain = old_domain;
+ else if (domain != old_domain)
+ *_reopen_file = true;
/* Update reference count on "struct tomoyo_domain_info". */
atomic_inc(&domain->users);
cred->security = domain;
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index 6f0f325..ecf24a7 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -37,11 +37,12 @@ static void tomoyo_cred_free(struct cred *cred)
atomic_dec(&domain->users);
}
-static int tomoyo_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred)
+static int tomoyo_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred,
+ bool *_reopen_file)
{
int rc, idx, err;
- rc = cap_bprm_set_creds(bprm, cred);
+ rc = cap_bprm_set_creds(bprm, cred, _reopen_file);
if (rc)
return rc;
@@ -70,7 +71,7 @@ static int tomoyo_bprm_set_creds(struct linux_binprm *bprm, struct cred *cred)
* credentials being constructed.
*/
idx = tomoyo_read_lock();
- err = tomoyo_find_next_domain(bprm, cred);
+ err = tomoyo_find_next_domain(bprm, cred, _reopen_file);
tomoyo_read_unlock(idx);
return err;
}
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 8/9] LSM: Pass linux_binprm pointer to open_exec() so creds are available
2011-04-21 14:30 [PATCH 0/9] Open loaders and interpreters with new creds during exec David Howells
` (6 preceding siblings ...)
2011-04-21 14:31 ` [PATCH 7/9] LSM: Allow an LSM to indicate that it wants bprm->file reopening with new creds David Howells
@ 2011-04-21 14:31 ` David Howells
[not found] ` <201104220225.p3M2PjJR020777@www262.sakura.ne.jp>
2011-04-21 14:31 ` [PATCH 9/9] LSM: Use derived creds for accessing an executable's interpreter and binary loader David Howells
` (4 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: David Howells @ 2011-04-21 14:31 UTC (permalink / raw)
To: serge.hallyn, eparis; +Cc: linux-security-module, selinux, David Howells
Pass the exec state (struct linux_binprm) pointer to open_exec() so new creds
are available for a later patch to use.
Also:
(1) Move the declaration of open_exec() to linux/binfmts.h.
(2) Stick a kerneldoc banner comment on open_exec() describing it.
Signed-off-by: David Howells <dhowells@redhat.com>
---
arch/alpha/kernel/binfmt_loader.c | 2 +-
fs/binfmt_elf.c | 2 +-
fs/binfmt_elf_fdpic.c | 2 +-
fs/binfmt_em86.c | 2 +-
fs/binfmt_flat.c | 17 ++++++-----------
fs/binfmt_misc.c | 2 +-
fs/binfmt_script.c | 2 +-
fs/compat.c | 2 +-
fs/exec.c | 23 +++++++++++++++++++++--
include/linux/binfmts.h | 1 +
include/linux/fs.h | 2 --
11 files changed, 35 insertions(+), 22 deletions(-)
diff --git a/arch/alpha/kernel/binfmt_loader.c b/arch/alpha/kernel/binfmt_loader.c
index 3fcfad4..1d19a44 100644
--- a/arch/alpha/kernel/binfmt_loader.c
+++ b/arch/alpha/kernel/binfmt_loader.c
@@ -24,7 +24,7 @@ static int load_binary(struct linux_binprm *bprm, struct pt_regs *regs)
loader = bprm->vma->vm_end - sizeof(void *);
- file = open_exec("/sbin/loader");
+ file = open_exec("/sbin/loader", bprm);
retval = PTR_ERR(file);
if (IS_ERR(file))
return retval;
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index c8060bb..9f08098 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -660,7 +660,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
if (elf_interpreter[elf_ppnt->p_filesz - 1] != '\0')
goto out_free_interp;
- interpreter = open_exec(elf_interpreter);
+ interpreter = open_exec(elf_interpreter, bprm);
retval = PTR_ERR(interpreter);
if (IS_ERR(interpreter))
goto out_free_interp;
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index eee4f70..a8e07b9 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -235,7 +235,7 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm,
kdebug("Using ELF interpreter %s", interpreter_name);
/* replace the program with the interpreter */
- interpreter = open_exec(interpreter_name);
+ interpreter = open_exec(interpreter_name, bprm);
retval = PTR_ERR(interpreter);
if (IS_ERR(interpreter)) {
interpreter = NULL;
diff --git a/fs/binfmt_em86.c b/fs/binfmt_em86.c
index b8e8b0a..44f3bb7 100644
--- a/fs/binfmt_em86.c
+++ b/fs/binfmt_em86.c
@@ -80,7 +80,7 @@ static int load_em86(struct linux_binprm *bprm,struct pt_regs *regs)
* Note that we use open_exec() as the name is now in kernel
* space, and we don't need to copy it.
*/
- file = open_exec(interp);
+ file = open_exec(interp, bprm);
if (IS_ERR(file))
return PTR_ERR(file);
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 397d305..e9fe5bb 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -823,29 +823,24 @@ static int load_flat_shared_library(int id, struct lib_info *libs)
/* Create the file name */
sprintf(buf, "/lib/lib%d.so", id);
+ bprm.cred = (struct cred *)get_current_cred();
+
/* Open the file up */
bprm.filename = buf;
- bprm.file = open_exec(bprm.filename);
+ bprm.file = open_exec(bprm.filename, bprm);
res = PTR_ERR(bprm.file);
if (IS_ERR(bprm.file))
- return res;
-
- bprm.cred = prepare_exec_creds();
- res = -ENOMEM;
- if (!bprm.cred)
- goto out;
+ goto out_cred;
res = prepare_binprm(&bprm);
if (!IS_ERR_VALUE(res))
res = load_flat_file(&bprm, libs, id, NULL);
- abort_creds(bprm.cred);
-
-out:
allow_write_access(bprm.file);
fput(bprm.file);
-
+out_cred:
+ put_creds(bprm.cred);
return(res);
}
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index 5209293..36a3297 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -178,7 +178,7 @@ static int load_misc_binary(struct linux_binprm *bprm, struct pt_regs *regs)
bprm->interp = iname; /* for binfmt_script */
- interp_file = open_exec (iname);
+ interp_file = open_exec (iname, bprm);
retval = PTR_ERR (interp_file);
if (IS_ERR (interp_file))
goto _error;
diff --git a/fs/binfmt_script.c b/fs/binfmt_script.c
index 396a988..78d7d47 100644
--- a/fs/binfmt_script.c
+++ b/fs/binfmt_script.c
@@ -87,7 +87,7 @@ static int load_script(struct linux_binprm *bprm,struct pt_regs *regs)
/*
* OK, now restart the process with the interpreter's dentry.
*/
- file = open_exec(interp);
+ file = open_exec(interp, bprm);
if (IS_ERR(file))
return PTR_ERR(file);
diff --git a/fs/compat.c b/fs/compat.c
index 72fe6cd..4b10306 100644
--- a/fs/compat.c
+++ b/fs/compat.c
@@ -1461,7 +1461,7 @@ int compat_do_execve(char * filename,
clear_in_exec = retval;
current->in_execve = 1;
- file = open_exec(filename);
+ file = open_exec(filename, bprm);
retval = PTR_ERR(file);
if (IS_ERR(file))
goto out_unmark;
diff --git a/fs/exec.c b/fs/exec.c
index 4c47395..e1584c6 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -720,7 +720,26 @@ EXPORT_SYMBOL(setup_arg_pages);
#endif /* CONFIG_MMU */
-struct file *open_exec(const char *name)
+/**
+ * open_exec - Open an executable binary, loader, script or interpreter file
+ * @name: The path of the file to open
+ * @bprm: The exec state
+ *
+ * Open an executable binary, loader, script or interpreter file for program
+ * execution to process.
+ *
+ * The file is opened with the currently proposed credentials for execution.
+ * This means that the file specified to execve() is opened with the caller's
+ * current credentials, but files opened after that - such as binary loaders
+ * (e.g. ld-config.so) and script interpreters (e.g. /bin/sh) - will be opened
+ * with the credentials that will be installed if that executable is the thing
+ * actually run.
+ *
+ * Note that each iteration through prepare_binprm() may further modify the
+ * credentials to be. The credentials at the time the returned file is opened
+ * will remain attached to file->f_cred until the file is closed.
+ */
+struct file *open_exec(const char *name, struct linux_binprm *bprm)
{
struct file *file;
int err;
@@ -1440,7 +1459,7 @@ int do_execve(const char * filename,
clear_in_exec = retval;
current->in_execve = 1;
- file = open_exec(filename);
+ file = open_exec(filename, bprm);
retval = PTR_ERR(file);
if (IS_ERR(file))
goto out_unmark;
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 82577f7..57ea660 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -133,6 +133,7 @@ extern int bprm_mm_init(struct linux_binprm *bprm);
extern int copy_strings_kernel(int argc, const char *const *argv,
struct linux_binprm *bprm);
extern int prepare_bprm_creds(struct linux_binprm *bprm);
+extern struct file *open_exec(const char *filename, struct linux_binprm *bprm);
extern void install_exec_creds(struct linux_binprm *bprm);
extern int kernel_read(struct linux_binprm *, struct file *,
loff_t, void *, unsigned long);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 74543c9..e1a44a9 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2224,8 +2224,6 @@ extern struct file *create_read_pipe(struct file *f, int flags);
extern struct file *create_write_pipe(int flags);
extern void free_write_pipe(struct file *);
-extern struct file * open_exec(const char *);
-
/* fs/dcache.c -- generic fs support functions */
extern int is_subdir(struct dentry *, struct dentry *);
extern int path_is_under(struct path *, struct path *);
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 9/9] LSM: Use derived creds for accessing an executable's interpreter and binary loader
2011-04-21 14:30 [PATCH 0/9] Open loaders and interpreters with new creds during exec David Howells
` (7 preceding siblings ...)
2011-04-21 14:31 ` [PATCH 8/9] LSM: Pass linux_binprm pointer to open_exec() so creds are available David Howells
@ 2011-04-21 14:31 ` David Howells
2011-04-21 16:12 ` [PATCH 0/9] Open loaders and interpreters with new creds during exec Stephen Smalley
` (3 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: David Howells @ 2011-04-21 14:31 UTC (permalink / raw)
To: serge.hallyn, eparis; +Cc: linux-security-module, selinux, David Howells
Currently, the caller must be able to open the script interpreter and/or the
binary loader that an executable wants to make use of, even if the executable
will be transitioned to a different context that can make use of that
interpreter or loader when the caller's context does not permit it.
Override credentials in open_exec() and kernel_read() with the currently
constructed new credentials so that if the executable file or its interpreter
specifies a transition to a different security context (DAC or MAC), then the
caller only has to provide access to the file to be executed, and not to the
interpreter (e.g. perl) or binary loader (e.g. ld.so) for the executable or
interpreter.
This means that if the caller does not have access to a script interpreter or
binary loader, it can still use scripts and executables that transition to a
security context that do.
For the initial opening of the executable file specified to execve(), this
won't make much difference as the new creds at that point are a clone of the
old ones (except that the new creds do not have thread or process keyrings).
For the opening of script interpreters (e.g. /bin/sh), the file will be opened
with the credentials-to-be rather than the credentials of the caller of
execve() by the script binfmt passing the bprm to open_exec(), and the file
will be reopened on the subsequent pass through prepare_binprm() if a security
context transition takes place.
For the opening of binary loaders (e.g. ld-linux.so), the file will be opened
with the credentials-to-be rather than the credentials of the caller of
execve() by the binary binfmt passing the bprm to open_exec().
If we publish the new credentials by making use of them, however, we may not
change them thereafter. This relies on the previous patches to make the cred
pointer in struct linux_binfmt semi-committed once it's assigned there.
Note that reopen_exec() uses dentry_open() rather than do_file_open(). To use
the latter, prepare_binprm() has to be furnished with a pointer to the filename
for whatever was opened for bprm->file and the core SELinux policy requires an
additional rule:
allow setfiles_t bin_t:lnk_file read;
so that /sbin/setfiles can be exec'd as /sbin/restorecon (which is a symlink).
To make the SELinux testsuite work after this patch, the following two rules
need to be added to the policy:
[policy/test_ptrace.te]
allow test_ptrace_traced_t bin_t:file { execute read open };
[policy/test_file.te]
allow fileop_t fileop_exec_t:file { execute read open };
The first allows the ptrace test to use the perl interpreter (labelled bin_t)
to run a perl script and the second allows the SIGIO file test's wait program
to use its binary (labelled fileop_exec_t).
Reported-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/exec.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 75 insertions(+), 7 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index e1584c6..c40a126 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -741,6 +741,7 @@ EXPORT_SYMBOL(setup_arg_pages);
*/
struct file *open_exec(const char *name, struct linux_binprm *bprm)
{
+ const struct cred *saved_cred;
struct file *file;
int err;
static const struct open_flags open_exec_flags = {
@@ -749,7 +750,9 @@ struct file *open_exec(const char *name, struct linux_binprm *bprm)
.intent = LOOKUP_OPEN
};
+ saved_cred = override_creds(bprm->cred);
file = do_filp_open(AT_FDCWD, name, &open_exec_flags, LOOKUP_FOLLOW);
+ revert_creds(saved_cred);
if (IS_ERR(file))
goto out;
@@ -789,15 +792,22 @@ int kernel_read(struct linux_binprm *bprm,
struct file *file, loff_t offset,
void *addr, unsigned long count)
{
+ const struct cred *saved_cred = NULL;
mm_segment_t old_fs;
loff_t pos = offset;
int result;
+ if (bprm && bprm->cred)
+ saved_cred = override_creds(bprm->cred);
+
old_fs = get_fs();
set_fs(get_ds());
/* The cast to a user pointer is valid due to the set_fs() */
result = vfs_read(file, (void __user *)addr, count, &pos);
set_fs(old_fs);
+
+ if (saved_cred)
+ revert_creds(saved_cred);
return result;
}
EXPORT_SYMBOL(kernel_read);
@@ -1231,11 +1241,65 @@ int check_unsafe_exec(struct linux_binprm *bprm)
return res;
}
-/*
- * Fill the binprm structure from the inode.
- * Check permissions, then read the first 128 (BINPRM_BUF_SIZE) bytes
+/*
+ * Reopen the file currently held by bprm->file with the newly calculated
+ * credentials.
+ *
+ * It's possible we should use open_exec() here instead, but that requires at
+ * least one additional SELinux rule.
+ */
+static int reopen_exec(struct linux_binprm *bprm)
+{
+ struct file *file;
+ int retval;
+
+ dget(bprm->file->f_path.dentry);
+ mntget(bprm->file->f_path.mnt);
+ file = dentry_open(bprm->file->f_path.dentry,
+ bprm->file->f_path.mnt,
+ bprm->file->f_flags,
+ bprm->cred);
+
+ if (IS_ERR(file))
+ return PTR_ERR(file);
+
+ fsnotify_open(file);
+ retval = deny_write_access(file);
+ if (retval < 0) {
+ fput(file);
+ return retval;
+ }
+
+ allow_write_access(bprm->file);
+ fput(bprm->file);
+ bprm->file = file;
+ return 0;
+}
+
+/**
+ * prepare_binprm - Set up the program execution state.
+ * @bprm: The exec state
+ *
+ * Set up the execution state for this particular iteration of the program
+ * execution procedure (there may be multiple iterations due to scripts and
+ * other interpreted executables).
+ *
+ * The proposed new credentials for the process are generated at this stage and
+ * may not be modified thereafter until and unless prepare_binprm() is called
+ * again - say for a script interpreter.
+ *
+ * The permissions on the file to be read (previously opened and attached to
+ * bprm->file) are checked at this point and credential transitions, such as
+ * SGID, SUID, capability escalations and LSM label changes are handled here
+ * too.
*
- * This may be called multiple times for binary chains (scripts for example).
+ * bprm->file will be reopened with the new credentials so that mappings made
+ * with it will belong to the same security context as the new program and
+ * bprm->file->f_cred will refer to the new creds and not the old creds.
+ *
+ * The first chunk of the file to be executed is read and placed in bprm->buf
+ * for the binfmt drivers to examine. It is presumed that this will be
+ * sufficient to provide the magic number and script interpreter filename.
*/
int prepare_binprm(struct linux_binprm *bprm)
{
@@ -1293,14 +1357,18 @@ int prepare_binprm(struct linux_binprm *bprm)
bprm->cred_prepared = 1;
put_cred(old);
- /* TODO: Reopen the executable image file if any significant security
- * data changed during calculation of the new credentials
+ /* Reopen the executable image file if any significant security data
+ * changed during calculation of the new credentials
*/
+ if (reopen_file) {
+ retval = reopen_exec(bprm);
+ if (retval < 0)
+ return retval;
+ }
memset(bprm->buf, 0, BINPRM_BUF_SIZE);
return exec_read_header(bprm, bprm->file);
}
-
EXPORT_SYMBOL(prepare_binprm);
/*
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 0/9] Open loaders and interpreters with new creds during exec
2011-04-21 14:30 [PATCH 0/9] Open loaders and interpreters with new creds during exec David Howells
` (8 preceding siblings ...)
2011-04-21 14:31 ` [PATCH 9/9] LSM: Use derived creds for accessing an executable's interpreter and binary loader David Howells
@ 2011-04-21 16:12 ` Stephen Smalley
2011-04-21 16:30 ` Daniel J Walsh
2011-04-21 17:32 ` Casey Schaufler
` (2 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Stephen Smalley @ 2011-04-21 16:12 UTC (permalink / raw)
To: David Howells; +Cc: serge.hallyn, eparis, linux-security-module, selinux
On Thu, 2011-04-21 at 15:30 +0100, David Howells wrote:
> Currently, the caller must be able to open the script interpreter and/or the
> binary loader that an executable wants to make use of, even if the executable
> will be transitioned to a different context that can make use of that
> interpreter or loader when the caller's context does not permit it.
>
> Override credentials in open_exec() and kernel_read() with the currently
> constructed new credentials so that if the executable file or its interpreter
> specifies a transition to a different security context (DAC or MAC), then the
> caller only has to provide access to the file to be executed, and not to the
> interpreter (e.g. perl) or binary loader (e.g. ld.so) for the executable or
> interpreter.
>
> This means that if the caller does not have access to a script interpreter or
> binary loader, it can still use scripts and executables that transition to a
> security context that do.
>
> Tetsuo Handa pointed out that TOMOYO had a problem because the loader image
> (such as ld-config.so) was checked with the wrong credentials (open_exec() is
> using the caller's credentials rather than credentials-to-be of the exec'd
> process.
>
> I note that this also applies to scripts and their interpreters. A script may
> end up getting run in a context that allows access to its interpreter, but
> that's no good if the caller of execve() doesn't permit the script interpreter
> to be opened.
>
> Here's a set of patches to deal with this. The last patch is the main
> ingredient.
>
>
> I have a couple of comments/questions on the above:
>
> (1) Consider a SUID binary. If the loader for that binary is executable by
> the uid to which the binary changes its uid on execution, but not by the
> uid of the caller, should execution succeed?
>
> For example, if, as root, I do this:
>
> cp -v /bin/ls /tmp/ls
> perl -p -i -e s/ld-linux/ld-linuQ/ /tmp/ls
> cp -v /lib64/ld-linux-x86-64.so.2 /lib64/ld-linuQ-x86-64.so.2
> chmod -v 0700 /lib64/ld-linuQ-x86-64.so.2
>
> then as myself do this:
>
> /tmp/ls
>
> this currently fails with permission denied; but with patch (5) above
> applied, it works. Obviously, this is a contrived example, but it also
> may applies to script interpreters if an LSM provides a security ID
> transition.
>
> (2) Conversely, if the loader is not executable by the uid to which an SUID
> executable switches, should that execution succeed?
>
> So if I alter the ownership and permissions on the above binaries:
>
> chown -v dhowells /lib64/ld-linuQ-x86-64.so.2
> chown -v bin /tmp/ls
> chmod u+s /tmp/ls
>
> then as myself do:
>
> /tmp/ls
>
> this works with patches removed and gives permission denied with it
> applied.
>
> (3) The SUID/SGID changes from patch 5 that I've noted in comments (1) and (2)
> should, I think, have no impact on the execution environment because:
>
> (*) loaders and interpreters are normally accessible to everyone in
> their DAC permissions (UID, GID, umask), and
>
> (*) recursing through prepare_binprm() multiple times only takes
> account of the SUID/SGID settings of the final level (which means
> that whilst you can have a SUID/SGID interpreter, you can't have a
> SUID/SGID script).
>
> An exception to this are binfmt_misc cases that have
> MISC_FMT_CREDENTIALS flagged - in those the credentials remain
> unchanged for the iteration, even if the next executable is
> SUID/SGID.
>
> (4) SELinux's testsuite needs the following additional rules:
>
> [policy/test_ptrace.te]
> allow test_ptrace_traced_t bin_t:file { execute read open };
>
> [policy/test_file.te]
> allow fileop_t fileop_exec_t:file { execute read open };
>
> to permit the ptrace test to access its script interpreter (perl) and the
> SIGIO file test's wait_io program to access its own executable.
This is an interesting change, but as it does change user-visible
behavior, does it need to be subject to some /proc/sys/kernel toggle so
that users can retain the existing behavior? And does at least the SUID
behavior change require discussion on lkml?
--
Stephen Smalley
National Security Agency
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/9] Open loaders and interpreters with new creds during exec
2011-04-21 16:12 ` [PATCH 0/9] Open loaders and interpreters with new creds during exec Stephen Smalley
@ 2011-04-21 16:30 ` Daniel J Walsh
0 siblings, 0 replies; 21+ messages in thread
From: Daniel J Walsh @ 2011-04-21 16:30 UTC (permalink / raw)
To: Stephen Smalley
Cc: David Howells, serge.hallyn, eparis, linux-security-module,
selinux
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 04/21/2011 12:12 PM, Stephen Smalley wrote:
> On Thu, 2011-04-21 at 15:30 +0100, David Howells wrote:
>> Currently, the caller must be able to open the script interpreter and/or the
>> binary loader that an executable wants to make use of, even if the executable
>> will be transitioned to a different context that can make use of that
>> interpreter or loader when the caller's context does not permit it.
>>
>> Override credentials in open_exec() and kernel_read() with the currently
>> constructed new credentials so that if the executable file or its interpreter
>> specifies a transition to a different security context (DAC or MAC), then the
>> caller only has to provide access to the file to be executed, and not to the
>> interpreter (e.g. perl) or binary loader (e.g. ld.so) for the executable or
>> interpreter.
>>
>> This means that if the caller does not have access to a script interpreter or
>> binary loader, it can still use scripts and executables that transition to a
>> security context that do.
>>
>> Tetsuo Handa pointed out that TOMOYO had a problem because the loader image
>> (such as ld-config.so) was checked with the wrong credentials (open_exec() is
>> using the caller's credentials rather than credentials-to-be of the exec'd
>> process.
>>
>> I note that this also applies to scripts and their interpreters. A script may
>> end up getting run in a context that allows access to its interpreter, but
>> that's no good if the caller of execve() doesn't permit the script interpreter
>> to be opened.
>>
>> Here's a set of patches to deal with this. The last patch is the main
>> ingredient.
>>
>>
>> I have a couple of comments/questions on the above:
>>
>> (1) Consider a SUID binary. If the loader for that binary is executable by
>> the uid to which the binary changes its uid on execution, but not by the
>> uid of the caller, should execution succeed?
>>
>> For example, if, as root, I do this:
>>
>> cp -v /bin/ls /tmp/ls
>> perl -p -i -e s/ld-linux/ld-linuQ/ /tmp/ls
>> cp -v /lib64/ld-linux-x86-64.so.2 /lib64/ld-linuQ-x86-64.so.2
>> chmod -v 0700 /lib64/ld-linuQ-x86-64.so.2
>>
>> then as myself do this:
>>
>> /tmp/ls
>>
>> this currently fails with permission denied; but with patch (5) above
>> applied, it works. Obviously, this is a contrived example, but it also
>> may applies to script interpreters if an LSM provides a security ID
>> transition.
>>
>> (2) Conversely, if the loader is not executable by the uid to which an SUID
>> executable switches, should that execution succeed?
>>
>> So if I alter the ownership and permissions on the above binaries:
>>
>> chown -v dhowells /lib64/ld-linuQ-x86-64.so.2
>> chown -v bin /tmp/ls
>> chmod u+s /tmp/ls
>>
>> then as myself do:
>>
>> /tmp/ls
>>
>> this works with patches removed and gives permission denied with it
>> applied.
>>
>> (3) The SUID/SGID changes from patch 5 that I've noted in comments (1) and (2)
>> should, I think, have no impact on the execution environment because:
>>
>> (*) loaders and interpreters are normally accessible to everyone in
>> their DAC permissions (UID, GID, umask), and
>>
>> (*) recursing through prepare_binprm() multiple times only takes
>> account of the SUID/SGID settings of the final level (which means
>> that whilst you can have a SUID/SGID interpreter, you can't have a
>> SUID/SGID script).
>>
>> An exception to this are binfmt_misc cases that have
>> MISC_FMT_CREDENTIALS flagged - in those the credentials remain
>> unchanged for the iteration, even if the next executable is
>> SUID/SGID.
>>
>> (4) SELinux's testsuite needs the following additional rules:
>>
>> [policy/test_ptrace.te]
>> allow test_ptrace_traced_t bin_t:file { execute read open };
>>
>> [policy/test_file.te]
>> allow fileop_t fileop_exec_t:file { execute read open };
>>
>> to permit the ptrace test to access its script interpreter (perl) and the
>> SIGIO file test's wait_io program to access its own executable.
>
> This is an interesting change, but as it does change user-visible
> behavior, does it need to be subject to some /proc/sys/kernel toggle so
> that users can retain the existing behavior? And does at least the SUID
> behavior change require discussion on lkml?
>
Could we use this change to prevent the execution of scripts in the
homedir?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAk2wW5kACgkQrlYvE4MpobMHZACgwrHyHF9hzs0lAhftf9D0Gy/W
oKUAoMXxECkganLDs2RLsauufNCz9R6i
=82tP
-----END PGP SIGNATURE-----
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/9] Open loaders and interpreters with new creds during exec
2011-04-21 14:30 [PATCH 0/9] Open loaders and interpreters with new creds during exec David Howells
` (9 preceding siblings ...)
2011-04-21 16:12 ` [PATCH 0/9] Open loaders and interpreters with new creds during exec Stephen Smalley
@ 2011-04-21 17:32 ` Casey Schaufler
2011-04-21 18:13 ` David Howells
2011-04-21 18:53 ` David Howells
[not found] ` <20110428200218.GB9186@hallyn.com>
12 siblings, 1 reply; 21+ messages in thread
From: Casey Schaufler @ 2011-04-21 17:32 UTC (permalink / raw)
To: David Howells
Cc: serge.hallyn, eparis, linux-security-module, selinux,
Casey Schaufler
On 4/21/2011 7:30 AM, David Howells wrote:
> Currently, the caller must be able to open the script interpreter and/or the
> binary loader that an executable wants to make use of, even if the executable
> will be transitioned to a different context that can make use of that
> interpreter or loader when the caller's context does not permit it.
Yes. This is good and correct behavior. You need access to all of
the resources you are going to use before you can start using them.
> Override credentials in open_exec() and kernel_read() with the currently
> constructed new credentials so that if the executable file or its interpreter
> specifies a transition to a different security context (DAC or MAC), then the
> caller only has to provide access to the file to be executed, and not to the
> interpreter (e.g. perl) or binary loader (e.g. ld.so) for the executable or
> interpreter.
No. This is bad and incorrect behavior.
> This means that if the caller does not have access to a script interpreter or
> binary loader, it can still use scripts and executables that transition to a
> security context that do.
Which is very very bad.
Let me provide an example using groups where this creates a serious
security problem.
/usr/local/bin/swine is an interpreter with known security flaws.
It is kept on the system because one specific group of users have
a swine script (wallow) that requires that exact version and that
has been vetted to not be vulnerable to those flaws. It is owned
by root.swill and mode 550 (-r-xr-x--- for you kids out there) to
prevent it being used by anyone else. The group that needs to use
wallow are allowed to use newgroup to add the group to their lists.
/usr/bin/swine is a newer version of the interpreter with all
known security flaws repaired. The users of the system have taken
to writing swine scripts like pigs to ... well, you get the idea.
They share scripts freely and there is a large collection, many
provided by members of the swill group.
One member (wilbur) of the swill group creates a swine script (mud)
that does some of the things that expose a security flaw in the old
version of the interpreter. Either through ignorance or malice he
creates the script in a public place, owned by wilbur.swill and
sets the setgid bit.
Another user of the system who is not in the swill group runs
% mud --ssn=538-99-9163
Because mud is setgid to swill it gets the old interpreter and
the flaw in the old swine sends sensitive information to a place
it does not belong. Even though the old bad version of swine is
clearly marked as inaccessible to the user it gets used.
Bad. Very bad.
Clear violation of the principle of least astonishment.
> Tetsuo Handa pointed out that TOMOYO had a problem because the loader image
> (such as ld-config.so) was checked with the wrong credentials (open_exec() is
> using the caller's credentials rather than credentials-to-be of the exec'd
> process.
I believe that our esteemed colleague is in error. I understand the
argument for the change, but I am not swayed by it.
> I note that this also applies to scripts and their interpreters. A script may
> end up getting run in a context that allows access to its interpreter, but
> that's no good if the caller of execve() doesn't permit the script interpreter
> to be opened.
I follow the logic, but it is based on extrapolation from what I
see as a flawed premise.
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/9] Open loaders and interpreters with new creds during exec
2011-04-21 17:32 ` Casey Schaufler
@ 2011-04-21 18:13 ` David Howells
0 siblings, 0 replies; 21+ messages in thread
From: David Howells @ 2011-04-21 18:13 UTC (permalink / raw)
To: Casey Schaufler
Cc: dhowells, serge.hallyn, eparis, linux-security-module, selinux
Casey Schaufler <casey@schaufler-ca.com> wrote:
> Because mud is setgid to swill it gets the old interpreter
SGID is cancelled because mud is a script.
David
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/9] Open loaders and interpreters with new creds during exec
2011-04-21 14:30 [PATCH 0/9] Open loaders and interpreters with new creds during exec David Howells
` (10 preceding siblings ...)
2011-04-21 17:32 ` Casey Schaufler
@ 2011-04-21 18:53 ` David Howells
[not found] ` <20110428200218.GB9186@hallyn.com>
12 siblings, 0 replies; 21+ messages in thread
From: David Howells @ 2011-04-21 18:53 UTC (permalink / raw)
Cc: dhowells, serge.hallyn, eparis, linux-security-module, selinux
David Howells <dhowells@redhat.com> wrote:
> (1) Consider a SUID binary. If the loader for that binary is executable by
> the uid to which the binary changes its uid on execution, but not by the
> uid of the caller, should execution succeed?
>
> For example, if, as root, I do this:
>
> cp -v /bin/ls /tmp/ls
> perl -p -i -e s/ld-linux/ld-linuQ/ /tmp/ls
> cp -v /lib64/ld-linux-x86-64.so.2 /lib64/ld-linuQ-x86-64.so.2
> chmod -v 0700 /lib64/ld-linuQ-x86-64.so.2
I forgot to add to that:
chmod u+s /tmp/ls
David
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 8/9] LSM: Pass linux_binprm pointer to open_exec() so creds are available
[not found] ` <201104220225.p3M2PjJR020777@www262.sakura.ne.jp>
@ 2011-04-28 15:27 ` David Howells
0 siblings, 0 replies; 21+ messages in thread
From: David Howells @ 2011-04-28 15:27 UTC (permalink / raw)
To: Tetsuo Handa
Cc: dhowells, linux-security-module, selinux, serge.hallyn, eparis
Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote:
> 819 struct linux_binprm bprm;
>
> Here bprm is allocated from stack memory without initialization
> whereas bprm in do_execve() is allocated using kzalloc().
That's true; most of it doesn't need initialisation, but some bits of it do -
but aren't initialised here. Not only that, it's quite a large struct, so
probably shouldn't be on the stack anyway - especially in NOMMU mode.
Anyway, I've posted a patch independent of this patch series to fix this.
David
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/9] Open loaders and interpreters with new creds during exec
[not found] ` <20110428200218.GB9186@hallyn.com>
@ 2011-04-30 10:48 ` David Howells
2011-05-02 13:28 ` Stephen Smalley
2011-05-04 12:12 ` David Howells
1 sibling, 1 reply; 21+ messages in thread
From: David Howells @ 2011-04-30 10:48 UTC (permalink / raw)
To: Serge Hallyn
Cc: dhowells, eparis, linux-security-module, selinux, Oleg Nesterov,
casey
Serge Hallyn <serge.hallyn@canonical.com> wrote:
> Did you mean to add
>
> chmod u+s /tmp/ls
>
> to the recipe above? If not, then I don't understand your point and think
> current behavior is right.
>
> If so, then I think what you say makes sense.
I did post a correction saying I'd forgotten to add that line.
> > (2) Conversely, if the loader is not executable by the uid to which an SUID
> > executable switches, should that execution succeed?
>
> The crux of the matter, then, is: do we consider the interpreter to be
> executed by the caller, or the new task?
I guess that's a good way of stating the case. I looked on it like this: The
interpreter, whether specified by a binary (eg. ELF's PT_INTERP) or a script
(eg. #!/bin/sh), is not specified by the user that called execve(), but is
rather built into the executable; indeed, the user may not be able to read the
executable and see what these details are (ie. they may only have execute
permission).
Of course, this may not apply to scripts, since we don't normally allow those
to effect SUID/SGID transitions. Should set-security-label transitions be
ignored on scripts too (which I think was one of the points Casey was taking
about)? Should the script interpreter simply reset the credentials to those
of the current user?
> I think what you're suggestion makes sense. I'd like to get input
> from someone like Roland or Oleg, though. Furthermore, while it
> seems to make sense, that does not mean that it'll be safe in a real
> system. Have you run LTP and some real workloads like WAS or
> Oracle to make sure there were no regressions?
I've run the SELinux test suite. That requires an extra rule or two to allow
its executables to access their loaders.
I'll look at running LTP too. I'm not sure what 'WAS' is[1], and I don't have
access to Oracle.
> (With that out of the way, I'll start looking at the patches under
> the assumption that the semantics are correct.)
Thanks!
David
[1] Things named 'WAS' are a bit tricky to Google for...
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/9] Open loaders and interpreters with new creds during exec
2011-04-30 10:48 ` David Howells
@ 2011-05-02 13:28 ` Stephen Smalley
0 siblings, 0 replies; 21+ messages in thread
From: Stephen Smalley @ 2011-05-02 13:28 UTC (permalink / raw)
To: David Howells
Cc: Serge Hallyn, eparis, linux-security-module, selinux,
Oleg Nesterov, casey
On Sat, 2011-04-30 at 11:48 +0100, David Howells wrote:
> Of course, this may not apply to scripts, since we don't normally allow those
> to effect SUID/SGID transitions. Should set-security-label transitions be
> ignored on scripts too (which I think was one of the points Casey was taking
> about)? Should the script interpreter simply reset the credentials to those
> of the current user?
The kernel should allow set-security-label transitions on scripts;
SELinux makes use of such transitions particularly for system
initialization, where the caller is already at least as trusted as the
callee.
--
Stephen Smalley
National Security Agency
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/9] Open loaders and interpreters with new creds during exec
[not found] ` <20110428200218.GB9186@hallyn.com>
2011-04-30 10:48 ` David Howells
@ 2011-05-04 12:12 ` David Howells
2011-05-04 13:15 ` David Howells
[not found] ` <20110504144558.GC917@mail.hallyn.com>
1 sibling, 2 replies; 21+ messages in thread
From: David Howells @ 2011-05-04 12:12 UTC (permalink / raw)
To: Serge Hallyn
Cc: dhowells, eparis, linux-security-module, selinux, Oleg Nesterov
Serge Hallyn <serge.hallyn@canonical.com> wrote:
> Have you run LTP
Okay... I've now run LTP with the patched (1st May) and unpatched (4th May)
kernel (attached is the diff between the logs).
Note that I forgot to load all the modules for the unpatched test, so that may
affect things.
I added fork13 to the skipfile for the second test as it doesn't seem to test
anything relevant to the matter at hand, but takes hours to complete.
Some tests failed with the patches removed but succeeded with the patches
applied, so I think I need to rerun LTP against the patched kernel with no
modules loaded for consistency.
So:
(*) fstatfs02 and fstatfs02_64 failed in the unpatched kernel.
(*) sched_getaffinity01 and proc01 failed in the patched kernel.
(*) su01 failed in both kernels, but differently.
(*) fork13 takes ages so is now skipped.
(*) pidns05 appears to be a broken test.
David
---
--- results/LTP_RUN_ON-2011_May_01-12h_13m_07s.log 2011-05-02 20:10:58.000000000 +0100
+++ results/LTP_RUN_ON-2011_May_04-00h_50m_25s.log 2011-05-04 10:45:58.000000000 +0100
@@ -1,4 +1,4 @@
-Test Start Time: Sun May 1 12:13:07 2011
+Test Start Time: Wed May 4 00:50:27 2011
-----------------------------------------
Testcase Result Exit Value
-------- ------ ----------
@@ -226,7 +226,6 @@
fork09 PASS 0
fork10 PASS 0
fork11 PASS 0
-fork13 PASS 0
fpathconf01 PASS 0
fstat01 PASS 0
fstat01_64 PASS 0
@@ -242,8 +241,8 @@
fstatat01_64 PASS 0
fstatfs01 PASS 0
fstatfs01_64 PASS 0
-fstatfs02 PASS 0
-fstatfs02_64 PASS 0
+fstatfs02 FAIL 1
+fstatfs02_64 FAIL 1
fsync01 PASS 0
fsync02 PASS 0
fsync03 PASS 0
@@ -639,7 +638,7 @@
sched_setscheduler01 PASS 0
sched_setscheduler02 PASS 0
sched_yield01 PASS 0
-sched_getaffinity01 FAIL 1
+sched_getaffinity01 PASS 0
select01 PASS 0
select02 PASS 0
select03 PASS 0
@@ -1025,7 +1024,7 @@
lftest01 PASS 0
writetest01 PASS 0
fs_di PASS 0
-proc01 FAIL 2
+proc01 PASS 0
fs_racer PASS 0
quota_remount_test01 FAIL 2
fs_perms01 PASS 0
@@ -1204,7 +1203,7 @@
Cap_bounds PASS 0
FCNTL_LOCKTESTS PASS 0
Connectors PASS 0
-su01 FAIL 126
+su01 FAIL 1
cron02 PASS 0
cron_deny01 PASS 0
cron_allow01 PASS 0
@@ -1273,7 +1272,7 @@
ht_interrupt PASS 0
-----------------------------------------------
-Total Tests: 1272
+Total Tests: 1271
Total Failures: 51
Kernel Version: 2.6.39-rc5-fsdevel+
Machine Architecture: x86_64
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/9] Open loaders and interpreters with new creds during exec
2011-05-04 12:12 ` David Howells
@ 2011-05-04 13:15 ` David Howells
[not found] ` <20110504144558.GC917@mail.hallyn.com>
1 sibling, 0 replies; 21+ messages in thread
From: David Howells @ 2011-05-04 13:15 UTC (permalink / raw)
To: Serge Hallyn
Cc: dhowells, eparis, linux-security-module, selinux, Oleg Nesterov
David Howells <dhowells@redhat.com> wrote:
> So:
I reran these tests by hand.
> (*) fstatfs02 and fstatfs02_64 failed in the unpatched kernel.
Works when I run the test manually under the unpatched kernel. Looking back at
the saved stdout, I see:
fstatfs02 2 TFAIL : unexpected error - 38 : Function not implemented - expected 14
so it may have been run on a filesystem that doesn't support this, but I'm not
sure what.
> (*) sched_getaffinity01
Works fine now with no modules loaded in the patched kernel. Not sure what
the problem was.
> and proc01 failed in the patched kernel.
I remember now... I killed this during the original run since it seemed to be
endlessly redoing:
read(10, "\0\0\0\0\0\0\0\6\0\0\0\0\0\0\0\6\0\0\0\0\0\0\0\6\0\0\0\0\0\0\0\6"..., 1024) = 1024
reading /proc/pid/task/tid/pagemap, but it can actually complete if left alone;
it just takes ages - it has one 64-bit entry per potential PTE[*] in a
process's VM space - and on a 64-bit machine that's a *lot* (~140 quadrillion
entries?).
> (*) su01 failed in both kernels, but differently.
Ah, yes. su01 failed in the original run with 126 because the "expect"
program wasn't installed. I install the rpm as soon as I saw it. I should've
restarted, I suppose.
Now it fails with exit 1 under both kernels because the su01_s1 expect script
reports:
YOU NEED TO SET ENVIROMENT VARIABLE PASSWD.
so that test is faulty.
David
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/9] Open loaders and interpreters with new creds during exec
[not found] ` <20110504144558.GC917@mail.hallyn.com>
@ 2011-05-04 16:27 ` David Howells
0 siblings, 0 replies; 21+ messages in thread
From: David Howells @ 2011-05-04 16:27 UTC (permalink / raw)
To: Serge Hallyn
Cc: dhowells, eparis, linux-security-module, selinux, Oleg Nesterov
Serge Hallyn <serge.hallyn@canonical.com> wrote:
> Thanks, David. I've also compiled and done some basic boot testing. I did
> need to add #include <linux/binfmts.h> to fs/coda/dir.c (this again may have
> been in one of your follow-on emails, but I didn't notice it).
Good point. Hmmm... I wonder if I should move kernel_read() to be next to
vfs_read() in fs/read_write.c and make a new function read_exec() to take a
bprm and call kernel_read() with the cred override applied. See the attached
patch.
This way non-binfmt stuff isn't calling into exec code.
Also, kernel_read() and read_exec() really ought to take a size_t count and
return an ssize_t value, though it isn't really a problem via count isn't too
big.
David
---
diff --git a/arch/x86/ia32/ia32_aout.c b/arch/x86/ia32/ia32_aout.c
index a8dddd2..6deedd3 100644
--- a/arch/x86/ia32/ia32_aout.c
+++ b/arch/x86/ia32/ia32_aout.c
@@ -445,7 +445,7 @@ static int load_aout_library(struct file *file)
inode = file->f_path.dentry->d_inode;
retval = -ENOEXEC;
- error = kernel_read(NULL, file, 0, &ex, sizeof(ex));
+ error = kernel_read(file, 0, &ex, sizeof(ex));
if (error != sizeof(ex))
goto out;
diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index 8890161..aef4f91 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -379,7 +379,7 @@ static int load_aout_library(struct file *file)
inode = file->f_path.dentry->d_inode;
retval = -ENOEXEC;
- error = kernel_read(NULL, file, 0, &ex, sizeof(ex));
+ error = kernel_read(file, 0, &ex, sizeof(ex));
if (error != sizeof(ex))
goto out;
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 9f08098..5f19711 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -416,7 +416,7 @@ static unsigned long load_elf_interp(struct linux_binprm *bprm,
if (!elf_phdata)
goto out;
- retval = kernel_read(NULL, interpreter, interp_elf_ex->e_phoff,
+ retval = kernel_read(interpreter, interp_elf_ex->e_phoff,
elf_phdata, size);
error = -EIO;
if (retval != size) {
@@ -612,8 +612,8 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
if (!elf_phdata)
goto out;
- retval = kernel_read(bprm, bprm->file, loc->elf_ex.e_phoff,
- elf_phdata, size);
+ retval = read_exec(bprm, bprm->file, loc->elf_ex.e_phoff,
+ elf_phdata, size);
if (retval != size) {
if (retval >= 0)
retval = -EIO;
@@ -646,10 +646,10 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
if (!elf_interpreter)
goto out_free_ph;
- retval = kernel_read(bprm, bprm->file,
- elf_ppnt->p_offset,
- elf_interpreter,
- elf_ppnt->p_filesz);
+ retval = read_exec(bprm, bprm->file,
+ elf_ppnt->p_offset,
+ elf_interpreter,
+ elf_ppnt->p_filesz);
if (retval != elf_ppnt->p_filesz) {
if (retval >= 0)
retval = -EIO;
@@ -1007,7 +1007,7 @@ static int load_elf_library(struct file *file)
struct elfhdr elf_ex;
error = -ENOEXEC;
- retval = kernel_read(NULL, file, 0, &elf_ex, sizeof(elf_ex));
+ retval = kernel_read(file, 0, &elf_ex, sizeof(elf_ex));
if (retval != sizeof(elf_ex))
goto out;
@@ -1031,7 +1031,7 @@ static int load_elf_library(struct file *file)
eppnt = elf_phdata;
error = -ENOEXEC;
- retval = kernel_read(NULL, file, elf_ex.e_phoff, eppnt, j);
+ retval = kernel_read(file, elf_ex.e_phoff, eppnt, j);
if (retval != j)
goto out_free_ph;
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index a8e07b9..fb7bab1 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -138,8 +138,8 @@ static int elf_fdpic_fetch_phdrs(struct linux_binprm *bprm,
if (!params->phdrs)
return -ENOMEM;
- retval = kernel_read(bprm, file, params->hdr.e_phoff,
- (char *) params->phdrs, size);
+ retval = read_exec(bprm, file, params->hdr.e_phoff,
+ params->phdrs, size);
if (unlikely(retval != size))
return retval < 0 ? retval : -ENOEXEC;
@@ -218,10 +218,10 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm,
if (!interpreter_name)
goto error;
- retval = kernel_read(bprm, bprm->file,
- phdr->p_offset,
- interpreter_name,
- phdr->p_filesz);
+ retval = read_exec(bprm, bprm->file,
+ phdr->p_offset,
+ interpreter_name,
+ phdr->p_filesz);
if (unlikely(retval != phdr->p_filesz)) {
if (retval >= 0)
retval = -ENOEXEC;
diff --git a/fs/binfmt_som.c b/fs/binfmt_som.c
index e27f6ad..7663575 100644
--- a/fs/binfmt_som.c
+++ b/fs/binfmt_som.c
@@ -211,8 +211,8 @@ load_som_binary(struct linux_binprm * bprm, struct pt_regs * regs)
if (!hpuxhdr)
goto out;
- retval = kernel_read(bprm, bprm->file, som_ex->aux_header_location,
- hpuxhdr, size);
+ retval = read_exec(bprm, bprm->file, som_ex->aux_header_location,
+ hpuxhdr, size);
if (retval != size) {
if (retval >= 0)
retval = -EIO;
diff --git a/fs/coda/dir.c b/fs/coda/dir.c
index e0c7f4a..b363d77 100644
--- a/fs/coda/dir.c
+++ b/fs/coda/dir.c
@@ -482,7 +482,7 @@ static int coda_venus_readdir(struct file *coda_file, void *buf,
}
while (1) {
/* read entries from the directory file */
- ret = kernel_read(NULL, host_file, coda_file->f_pos - 2, vdir,
+ ret = kernel_read(host_file, coda_file->f_pos - 2, vdir,
sizeof(*vdir));
if (ret < 0) {
printk(KERN_ERR "coda readdir: read dir %s failed %d\n",
diff --git a/fs/exec.c b/fs/exec.c
index c40a126..7b66a5d 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -779,7 +779,7 @@ exit:
EXPORT_SYMBOL(open_exec);
/**
- * kernel_read - Read data from an executable
+ * read_exec - Read data from an executable
* @bprm: Execution state (or NULL if after the point of no return)
* @file: File to read from
* @offset: File offset to read from
@@ -788,29 +788,20 @@ EXPORT_SYMBOL(open_exec);
*
* Read some data from an executable
*/
-int kernel_read(struct linux_binprm *bprm,
- struct file *file, loff_t offset,
- void *addr, unsigned long count)
+ssize_t read_exec(struct linux_binprm *bprm,
+ struct file *file, loff_t offset, void *addr, size_t count)
{
const struct cred *saved_cred = NULL;
- mm_segment_t old_fs;
- loff_t pos = offset;
- int result;
+ ssize_t result;
- if (bprm && bprm->cred)
+ if (bprm->cred)
saved_cred = override_creds(bprm->cred);
-
- old_fs = get_fs();
- set_fs(get_ds());
- /* The cast to a user pointer is valid due to the set_fs() */
- result = vfs_read(file, (void __user *)addr, count, &pos);
- set_fs(old_fs);
-
+ result = kernel_read(file, offset, addr, count);
if (saved_cred)
revert_creds(saved_cred);
return result;
}
-EXPORT_SYMBOL(kernel_read);
+EXPORT_SYMBOL(read_exec);
static int exec_mmap(struct mm_struct *mm)
{
diff --git a/fs/read_write.c b/fs/read_write.c
index 5520f8a..875dbfc 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -333,6 +333,27 @@ ssize_t vfs_read(struct file *file, char __user *buf, size_t count, loff_t *pos)
EXPORT_SYMBOL(vfs_read);
+/**
+ * kernel_read - Read data to a kernel buffer
+ * @file: File to read from
+ * @offset: File offset to read from
+ * @addr: The buffer to read into
+ * @count: The amount of data to read
+ */
+ssize_t kernel_read(struct file *file, loff_t offset, void *addr, size_t count)
+{
+ mm_segment_t old_fs = get_fs();
+ loff_t pos = offset;
+ ssize_t result;
+
+ set_fs(get_ds());
+ /* The cast to a user pointer is valid due to the set_fs() */
+ result = vfs_read(file, (void __user *)addr, count, &pos);
+ set_fs(old_fs);
+ return result;
+}
+EXPORT_SYMBOL(kernel_read);
+
ssize_t do_sync_write(struct file *filp, const char __user *buf, size_t len, loff_t *ppos)
{
struct iovec iov = { .iov_base = (void __user *)buf, .iov_len = len };
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 57ea660..b394f85 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -135,8 +135,8 @@ extern int copy_strings_kernel(int argc, const char *const *argv,
extern int prepare_bprm_creds(struct linux_binprm *bprm);
extern struct file *open_exec(const char *filename, struct linux_binprm *bprm);
extern void install_exec_creds(struct linux_binprm *bprm);
-extern int kernel_read(struct linux_binprm *, struct file *,
- loff_t, void *, unsigned long);
+extern ssize_t read_exec(struct linux_binprm *, struct file *,
+ loff_t, void *, unsigned long);
extern void install_exec_creds(struct linux_binprm *bprmw);
extern void do_coredump(long signr, int exit_code, struct pt_regs *regs);
extern void set_binfmt(struct linux_binfmt *new);
@@ -144,7 +144,7 @@ extern void free_bprm(struct linux_binprm *);
static inline int exec_read_header(struct linux_binprm *bprm, struct file *file)
{
- return kernel_read(bprm, file, 0, bprm->buf, BINPRM_BUF_SIZE);
+ return read_exec(bprm, file, 0, bprm->buf, BINPRM_BUF_SIZE);
}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e1a44a9..42a1be2 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1610,6 +1610,7 @@ extern ssize_t vfs_readv(struct file *, const struct iovec __user *,
unsigned long, loff_t *);
extern ssize_t vfs_writev(struct file *, const struct iovec __user *,
unsigned long, loff_t *);
+extern ssize_t kernel_read(struct file *, loff_t, void *, size_t);
struct super_operations {
struct inode *(*alloc_inode)(struct super_block *sb);
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index 6877035..aa5672b 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -277,7 +277,7 @@ static int p9_fd_read(struct p9_client *client, void *v, int len)
if (!(ts->rd->f_flags & O_NONBLOCK))
P9_DPRINTK(P9_DEBUG_ERROR, "blocking read ...\n");
- ret = kernel_read(NULL, ts->rd, ts->rd->f_pos, v, len);
+ ret = kernel_read(ts->rd, ts->rd->f_pos, v, len);
if (ret <= 0 && ret != -ERESTARTSYS && ret != -EAGAIN)
client->status = Disconnected;
return ret;
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 0c93027..9b3ade7 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -63,7 +63,7 @@ int ima_calc_hash(struct file *file, char *digest)
while (offset < i_size) {
int rbuf_len;
- rbuf_len = kernel_read(NULL, file, offset, rbuf, PAGE_SIZE);
+ rbuf_len = kernel_read(file, offset, rbuf, PAGE_SIZE);
if (rbuf_len < 0) {
rc = rbuf_len;
break;
--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.
^ permalink raw reply related [flat|nested] 21+ messages in thread
end of thread, other threads:[~2011-05-04 16:28 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-21 14:30 [PATCH 0/9] Open loaders and interpreters with new creds during exec David Howells
2011-04-21 14:30 ` [PATCH 1/9] TOMOYO: Fix tomoyo_dentry_open() to use the right creds David Howells
2011-04-21 14:30 ` [PATCH 2/9] TOMOYO: Derive the new domain for an exec'd process in tomoyo_bprm_set_creds() David Howells
2011-04-21 14:30 ` [PATCH 3/9] LSM: Permit commit_creds() to take a const pointer David Howells
2011-04-21 14:30 ` [PATCH 4/9] LSM: Make the linux_binfmt creds pointer const and pass creds to bprm_set_creds David Howells
2011-04-21 14:30 ` [PATCH 5/9] LSM: Install the new credentials earlier in the exec procedure David Howells
2011-04-21 14:31 ` [PATCH 6/9] LSM: Pass linux_binprm pointer to kernel_read() so creds are available David Howells
2011-04-21 14:31 ` [PATCH 7/9] LSM: Allow an LSM to indicate that it wants bprm->file reopening with new creds David Howells
2011-04-21 14:31 ` [PATCH 8/9] LSM: Pass linux_binprm pointer to open_exec() so creds are available David Howells
[not found] ` <201104220225.p3M2PjJR020777@www262.sakura.ne.jp>
2011-04-28 15:27 ` David Howells
2011-04-21 14:31 ` [PATCH 9/9] LSM: Use derived creds for accessing an executable's interpreter and binary loader David Howells
2011-04-21 16:12 ` [PATCH 0/9] Open loaders and interpreters with new creds during exec Stephen Smalley
2011-04-21 16:30 ` Daniel J Walsh
2011-04-21 17:32 ` Casey Schaufler
2011-04-21 18:13 ` David Howells
2011-04-21 18:53 ` David Howells
[not found] ` <20110428200218.GB9186@hallyn.com>
2011-04-30 10:48 ` David Howells
2011-05-02 13:28 ` Stephen Smalley
2011-05-04 12:12 ` David Howells
2011-05-04 13:15 ` David Howells
[not found] ` <20110504144558.GC917@mail.hallyn.com>
2011-05-04 16:27 ` David Howells
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.