On Thu, Dec 24, 2015 at 7:30 AM, Jann Horn <jann@xxxxxxxxx> wrote:
> This adds a new ptrace_may_access_file() method that extracts
> the caller credentials from the supplied file instead of the
> current task. (However, the current task may still be
> inspected for auditing purposes, e.g. by the Smack LSM.)
>
> procfs used the caller's creds for a few ptrace_may_access()
> checks at read() time, which made a confused deputy attack
> by passing an FD to a procfs file to a setuid program
> possible. Therefore, the following was a local userspace
> ASLR bypass:
>
> rm -f /tmp/foobar
> procmail <(echo 'DEFAULT=/tmp/foobar') </proc/1/stat
> (
> echo 'obase=16'
> cut -d' ' -f26-30,45-51 </tmp/foobar | tr ' ' '\n'
> ) | bc
> rm /tmp/foobar
We keep poking around the edges of all these /proc permission-check
corner-cases, and I get more and more nervous the more complex we make
it. I'd really love it if there was tools/testing/selftests/proc/ set
of tests to check all these corner cases. We need to have a single
place where we call out all the specific details of what behaviors are
expected, and that we have a useful regression test for it as well.
Would you be willing to design these tests?
-Kees
>
> procmail is installed setuid root on Debian and read()s
> data from stdin before dropping privs, so the
> ptrace_may_access() check in the VFS read handler of
> /proc/1/stat passes. Procmail then dumps the read data
> to a user-accessible file (/tmp/foobar here).
>
> I know that this is way larger than 100 lines, but is it
> possible to get a stable backport for it anyway?
>
> The hole in the PTRACE_MODE_... constants is intentional,
> it should reduce the merge conflicts with abc5df4106 a bit.
>
> If this is backported, ab5931b90b also needs to be
> backported.
>
> checkpatch throws a ton of warnings about overlong lines.
> Should I split all those lines?
>
> Can anyone tell me whether I violated the locking rules
> somewhere?
>
> CC: Stable <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Jann Horn <jann@xxxxxxxxx>
> ---
> fs/file_table.c | 2 ++
> fs/proc/array.c | 2 +-
> fs/proc/base.c | 29 +++++++++++++++++------------
> fs/seq_file.c | 1 +
> include/linux/fs.h | 2 ++
> include/linux/lsm_hooks.h | 3 ++-
> include/linux/ptrace.h | 2 ++
> include/linux/security.h | 8 ++++----
> include/linux/seq_file.h | 1 +
> kernel/ptrace.c | 35 +++++++++++++++++++++++------------
> security/apparmor/include/ipc.h | 2 +-
> security/apparmor/ipc.c | 4 +---
> security/apparmor/lsm.c | 12 +++++++++---
> security/commoncap.c | 7 +++----
> security/security.c | 4 ++--
> security/selinux/hooks.c | 4 ++--
> security/smack/smack_lsm.c | 16 ++++++++++------
> security/yama/yama_lsm.c | 8 +++++---
> 18 files changed, 88 insertions(+), 54 deletions(-)
>
> diff --git a/fs/file_table.c b/fs/file_table.c
> index ad17e05..671033e 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
> @@ -125,6 +125,7 @@ struct file *get_empty_filp(void)
>
> percpu_counter_inc(&nr_files);
> f->f_cred = get_cred(cred);
> + f->f_tgid = get_pid(task_tgid(current));
> error = security_file_alloc(f);
> if (unlikely(error)) {
> file_free(f);
> @@ -222,6 +223,7 @@ static void __fput(struct file *file)
> file->f_path.dentry = NULL;
> file->f_path.mnt = NULL;
> file->f_inode = NULL;
> + put_pid(file->f_tgid);
> file_free(file);
> dput(dentry);
> mntput(mnt);
> diff --git a/fs/proc/array.c b/fs/proc/array.c
> index d73291f..db5aeb7 100644
> --- a/fs/proc/array.c
> +++ b/fs/proc/array.c
> @@ -395,7 +395,7 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
>
> state = *get_task_state(task);
> vsize = eip = esp = 0;
> - permitted = ptrace_may_access(task, PTRACE_MODE_READ | PTRACE_MODE_NOAUDIT);
> + permitted = ptrace_may_access_file(task, PTRACE_MODE_READ | PTRACE_MODE_NOAUDIT, m->file);
> mm = get_task_mm(task);
> if (mm) {
> vsize = task_vsize(mm);
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 4bd5d31..2d0ce7a 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -430,7 +430,8 @@ static int proc_pid_wchan(struct seq_file *m, struct pid_namespace *ns,
>
> wchan = get_wchan(task);
>
> - if (wchan && ptrace_may_access(task, PTRACE_MODE_READ) && !lookup_symbol_name(wchan, symname))
> + if (wchan && ptrace_may_access_file(task, PTRACE_MODE_READ, m->file) &&
> + !lookup_symbol_name(wchan, symname))
> seq_printf(m, "%s", symname);
> else
> seq_putc(m, '0');
> @@ -439,12 +440,12 @@ static int proc_pid_wchan(struct seq_file *m, struct pid_namespace *ns,
> }
> #endif /* CONFIG_KALLSYMS */
>
> -static int lock_trace(struct task_struct *task)
> +static int lock_trace(struct seq_file *m, struct task_struct *task)
> {
> int err = mutex_lock_killable(&task->signal->cred_guard_mutex);
> if (err)
> return err;
> - if (!ptrace_may_access(task, PTRACE_MODE_ATTACH)) {
> + if (!ptrace_may_access_file(task, PTRACE_MODE_ATTACH, m->file)) {
> mutex_unlock(&task->signal->cred_guard_mutex);
> return -EPERM;
> }
> @@ -477,7 +478,7 @@ static int proc_pid_stack(struct seq_file *m, struct pid_namespace *ns,
> trace.entries = entries;
> trace.skip = 0;
>
> - err = lock_trace(task);
> + err = lock_trace(m, task);
> if (!err) {
> save_stack_trace_tsk(task, &trace);
>
> @@ -662,7 +663,7 @@ static int proc_pid_syscall(struct seq_file *m, struct pid_namespace *ns,
> unsigned long args[6], sp, pc;
> int res;
>
> - res = lock_trace(task);
> + res = lock_trace(m, task);
> if (res)
> return res;
>
> @@ -726,13 +727,17 @@ int proc_setattr(struct dentry *dentry, struct iattr *attr)
> */
> static bool has_pid_permissions(struct pid_namespace *pid,
> struct task_struct *task,
> + struct file *cred_file,
> int hide_pid_min)
> {
> if (pid->hide_pid < hide_pid_min)
> return true;
> if (in_group_p(pid->pid_gid))
> return true;
> - return ptrace_may_access(task, PTRACE_MODE_READ);
> + if (cred_file == NULL)
> + return ptrace_may_access(task, PTRACE_MODE_READ);
> + else
> + return ptrace_may_access_file(task, PTRACE_MODE_READ, cred_file);
> }
>
>
> @@ -745,7 +750,7 @@ static int proc_pid_permission(struct inode *inode, int mask)
> task = get_proc_task(inode);
> if (!task)
> return -ESRCH;
> - has_perms = has_pid_permissions(pid, task, 1);
> + has_perms = has_pid_permissions(pid, task, NULL, 1);
> put_task_struct(task);
>
> if (!has_perms) {
> @@ -1693,7 +1698,7 @@ int pid_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
> stat->gid = GLOBAL_ROOT_GID;
> task = pid_task(proc_pid(inode), PIDTYPE_PID);
> if (task) {
> - if (!has_pid_permissions(pid, task, 2)) {
> + if (!has_pid_permissions(pid, task, NULL, 2)) {
> rcu_read_unlock();
> /*
> * This doesn't prevent learning whether PID exists,
> @@ -2060,7 +2065,7 @@ proc_map_files_readdir(struct file *file, struct dir_context *ctx)
> goto out;
>
> ret = -EACCES;
> - if (!ptrace_may_access(task, PTRACE_MODE_READ))
> + if (!ptrace_may_access_file(task, PTRACE_MODE_READ, file))
> goto out_put_task;
>
> ret = 0;
> @@ -2530,7 +2535,7 @@ static int do_io_accounting(struct task_struct *task, struct seq_file *m, int wh
> if (result)
> return result;
>
> - if (!ptrace_may_access(task, PTRACE_MODE_READ)) {
> + if (!ptrace_may_access_file(task, PTRACE_MODE_READ, m->file)) {
> result = -EACCES;
> goto out_unlock;
> }
> @@ -2714,7 +2719,7 @@ static const struct file_operations proc_setgroups_operations = {
> static int proc_pid_personality(struct seq_file *m, struct pid_namespace *ns,
> struct pid *pid, struct task_struct *task)
> {
> - int err = lock_trace(task);
> + int err = lock_trace(m, task);
> if (!err) {
> seq_printf(m, "%08x\n", task->personality);
> unlock_trace(task);
> @@ -3061,7 +3066,7 @@ int proc_pid_readdir(struct file *file, struct dir_context *ctx)
> iter.tgid += 1, iter = next_tgid(ns, iter)) {
> char name[PROC_NUMBUF];
> int len;
> - if (!has_pid_permissions(ns, iter.task, 2))
> + if (!has_pid_permissions(ns, iter.task, file, 2))
> continue;
>
> len = snprintf(name, sizeof(name), "%d", iter.tgid);
> diff --git a/fs/seq_file.c b/fs/seq_file.c
> index e85664b..32a070e 100644
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -68,6 +68,7 @@ int seq_open(struct file *file, const struct seq_operations *op)
> if (!p)
> return -ENOMEM;
>
> + p->file = file;
> file->private_data = p;
>
> mutex_init(&p->lock);
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 3aa5142..9be60d4 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -857,6 +857,8 @@ struct file {
> loff_t f_pos;
> struct fown_struct f_owner;
> const struct cred *f_cred;
> + /* for opener-based access checks, mostly in procfs */
> + struct pid *f_tgid;
> struct file_ra_state f_ra;
>
> u64 f_version;
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index ec3a6ba..d2995cb 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1148,6 +1148,7 @@
> * attributes would be changed by the execve.
> * @child contains the task_struct structure for the target process.
> * @mode contains the PTRACE_MODE flags indicating the form of access.
> + * @cred contains the caller's credentials
> * Return 0 if permission is granted.
> * @ptrace_traceme:
> * Check that the @parent process has sufficient permission to trace the
> @@ -1311,7 +1312,7 @@ union security_list_options {
> struct file *file);
>
> int (*ptrace_access_check)(struct task_struct *child,
> - unsigned int mode);
> + unsigned int mode, const struct cred *cred);
> int (*ptrace_traceme)(struct task_struct *parent);
> int (*capget)(struct task_struct *target, kernel_cap_t *effective,
> kernel_cap_t *inheritable, kernel_cap_t *permitted);
> diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
> index 061265f..f29390a 100644
> --- a/include/linux/ptrace.h
> +++ b/include/linux/ptrace.h
> @@ -57,8 +57,10 @@ extern void exit_ptrace(struct task_struct *tracer, struct list_head *dead);
> #define PTRACE_MODE_READ 0x01
> #define PTRACE_MODE_ATTACH 0x02
> #define PTRACE_MODE_NOAUDIT 0x04
> +#define PTRACE_MODE_NON_CURRENT 0x20 /* don't use privs of current task */
> /* Returns true on success, false on denial. */
> extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
> +bool ptrace_may_access_file(struct task_struct *task, unsigned int mode, struct file *f);
>
> static inline int ptrace_reparented(struct task_struct *child)
> {
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 2f4c1f7..d3eec58 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -70,7 +70,7 @@ struct timezone;
> extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
> int cap, int audit);
> extern int cap_settime(const struct timespec *ts, const struct timezone *tz);
> -extern int cap_ptrace_access_check(struct task_struct *child, unsigned int mode);
> +extern int cap_ptrace_access_check(struct task_struct *child, unsigned int mode, const struct cred *cred);
> extern int cap_ptrace_traceme(struct task_struct *parent);
> extern int cap_capget(struct task_struct *target, kernel_cap_t *effective, kernel_cap_t *inheritable, kernel_cap_t *permitted);
> extern int cap_capset(struct cred *new, const struct cred *old,
> @@ -189,7 +189,7 @@ int security_binder_transfer_binder(struct task_struct *from,
> struct task_struct *to);
> int security_binder_transfer_file(struct task_struct *from,
> struct task_struct *to, struct file *file);
> -int security_ptrace_access_check(struct task_struct *child, unsigned int mode);
> +int security_ptrace_access_check(struct task_struct *child, unsigned int mode, const struct cred *cred);
> int security_ptrace_traceme(struct task_struct *parent);
> int security_capget(struct task_struct *target,
> kernel_cap_t *effective,
> @@ -403,9 +403,9 @@ static inline int security_binder_transfer_file(struct task_struct *from,
> }
>
> static inline int security_ptrace_access_check(struct task_struct *child,
> - unsigned int mode)
> + unsigned int mode, const struct cred *cred)
> {
> - return cap_ptrace_access_check(child, mode);
> + return cap_ptrace_access_check(child, mode, cred);
> }
>
> static inline int security_ptrace_traceme(struct task_struct *parent)
> diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
> index dde00de..55ab0d8 100644
> --- a/include/linux/seq_file.h
> +++ b/include/linux/seq_file.h
> @@ -30,6 +30,7 @@ struct seq_file {
> #ifdef CONFIG_USER_NS
> struct user_namespace *user_ns;
> #endif
> + struct file *file;
> void *private;
> };
>
> diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> index b760bae..3ee70e9 100644
> --- a/kernel/ptrace.c
> +++ b/kernel/ptrace.c
> @@ -207,18 +207,18 @@ static int ptrace_check_attach(struct task_struct *child, bool ignore_state)
> return ret;
> }
>
> -static int ptrace_has_cap(struct user_namespace *ns, unsigned int mode)
> +static int ptrace_has_cap(const struct cred *cred, struct user_namespace *ns, unsigned int mode)
> {
> if (mode & PTRACE_MODE_NOAUDIT)
> - return has_ns_capability_noaudit(current, ns, CAP_SYS_PTRACE);
> + return security_capable_noaudit(cred, ns, CAP_SYS_PTRACE) == 0;
> else
> - return has_ns_capability(current, ns, CAP_SYS_PTRACE);
> + return security_capable(cred, ns, CAP_SYS_PTRACE) == 0;
> }
>
> /* Returns 0 on success, -errno on denial. */
> -static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> +static int __ptrace_may_access(struct task_struct *task, unsigned int mode, const struct cred *cred, struct pid *caller_tgid)
> {
> - const struct cred *cred = current_cred(), *tcred;
> + const struct cred *tcred;
>
> /* May we inspect the given task?
> * This check is used both for attaching with ptrace
> @@ -229,10 +229,12 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> * or halting the specified task is impossible.
> */
> int dumpable = 0;
> + rcu_read_lock();
> /* Don't let security modules deny introspection */
> - if (same_thread_group(task, current))
> + if (task_tgid(task) == caller_tgid) {
> + rcu_read_unlock();
> return 0;
> - rcu_read_lock();
> + }
> tcred = __task_cred(task);
> if (uid_eq(cred->uid, tcred->euid) &&
> uid_eq(cred->uid, tcred->suid) &&
> @@ -241,7 +243,7 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> gid_eq(cred->gid, tcred->sgid) &&
> gid_eq(cred->gid, tcred->gid))
> goto ok;
> - if (ptrace_has_cap(tcred->user_ns, mode))
> + if (ptrace_has_cap(cred, tcred->user_ns, mode))
> goto ok;
> rcu_read_unlock();
> return -EPERM;
> @@ -252,20 +254,29 @@ ok:
> dumpable = get_dumpable(task->mm);
> rcu_read_lock();
> if (dumpable != SUID_DUMP_USER &&
> - !ptrace_has_cap(__task_cred(task)->user_ns, mode)) {
> + !ptrace_has_cap(cred, __task_cred(task)->user_ns, mode)) {
> rcu_read_unlock();
> return -EPERM;
> }
> rcu_read_unlock();
>
> - return security_ptrace_access_check(task, mode);
> + return security_ptrace_access_check(task, mode, cred);
> }
>
> bool ptrace_may_access(struct task_struct *task, unsigned int mode)
> {
> int err;
> task_lock(task);
> - err = __ptrace_may_access(task, mode);
> + err = __ptrace_may_access(task, mode, current_cred(), task_tgid(current));
> + task_unlock(task);
> + return !err;
> +}
> +
> +bool ptrace_may_access_file(struct task_struct *task, unsigned int mode, struct file *f)
> +{
> + int err;
> + task_lock(task);
> + err = __ptrace_may_access(task, mode | PTRACE_MODE_NON_CURRENT, f->f_cred, f->f_tgid);
> task_unlock(task);
> return !err;
> }
> @@ -306,7 +317,7 @@ static int ptrace_attach(struct task_struct *task, long request,
> goto out;
>
> task_lock(task);
> - retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH);
> + retval = __ptrace_may_access(task, PTRACE_MODE_ATTACH, current_cred(), task_tgid(current));
> task_unlock(task);
> if (retval)
> goto unlock_creds;
> diff --git a/security/apparmor/include/ipc.h b/security/apparmor/include/ipc.h
> index 288ca76..eb685c1 100644
> --- a/security/apparmor/include/ipc.h
> +++ b/security/apparmor/include/ipc.h
> @@ -22,7 +22,7 @@ struct aa_profile;
> int aa_may_ptrace(struct aa_profile *tracer, struct aa_profile *tracee,
> unsigned int mode);
>
> -int aa_ptrace(struct task_struct *tracer, struct task_struct *tracee,
> +int aa_ptrace(struct aa_profile *tracer_p, struct task_struct *tracee,
> unsigned int mode);
>
> #endif /* __AA_IPC_H */
> diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c
> index 777ac1c..c60b374 100644
> --- a/security/apparmor/ipc.c
> +++ b/security/apparmor/ipc.c
> @@ -82,7 +82,7 @@ int aa_may_ptrace(struct aa_profile *tracer, struct aa_profile *tracee,
> *
> * Returns: %0 else error code if permission denied or error
> */
> -int aa_ptrace(struct task_struct *tracer, struct task_struct *tracee,
> +int aa_ptrace(struct aa_profile *tracer_p, struct task_struct *tracee,
> unsigned int mode)
> {
> /*
> @@ -94,7 +94,6 @@ int aa_ptrace(struct task_struct *tracer, struct task_struct *tracee,
> * - tracer profile has CAP_SYS_PTRACE
> */
>
> - struct aa_profile *tracer_p = aa_get_task_profile(tracer);
> int error = 0;
>
> if (!unconfined(tracer_p)) {
> @@ -105,7 +104,6 @@ int aa_ptrace(struct task_struct *tracer, struct task_struct *tracee,
>
> aa_put_profile(tracee_p);
> }
> - aa_put_profile(tracer_p);
>
> return error;
> }
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index dec607c..a4f3f40 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -94,14 +94,20 @@ static void apparmor_cred_transfer(struct cred *new, const struct cred *old)
> }
>
> static int apparmor_ptrace_access_check(struct task_struct *child,
> - unsigned int mode)
> + unsigned int mode, const struct cred *cred)
> {
> - return aa_ptrace(current, child, mode);
> + struct aa_profile *tracer_p = aa_get_profile(aa_cred_profile(cred));
> + int res = aa_ptrace(tracer_p, child, mode);
> + aa_put_profile(tracer_p);
> + return res;
> }
>
> static int apparmor_ptrace_traceme(struct task_struct *parent)
> {
> - return aa_ptrace(parent, current, PTRACE_MODE_ATTACH);
> + struct aa_profile *tracer_p = aa_get_task_profile(tracer);
> + int res = aa_ptrace(tracer_p, current, PTRACE_MODE_ATTACH);
> + aa_put_profile(tracer_p);
> + return res;
> }
>
> /* Derived from security/commoncap.c:cap_capget */
> diff --git a/security/commoncap.c b/security/commoncap.c
> index 1832cf7..7a385b2 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -133,18 +133,17 @@ int cap_settime(const struct timespec *ts, const struct timezone *tz)
> * Determine whether a process may access another, returning 0 if permission
> * granted, -ve if denied.
> */
> -int cap_ptrace_access_check(struct task_struct *child, unsigned int mode)
> +int cap_ptrace_access_check(struct task_struct *child, unsigned int mode, const struct cred *cred)
> {
> int ret = 0;
> - const struct cred *cred, *child_cred;
> + const struct cred *child_cred;
>
> rcu_read_lock();
> - cred = current_cred();
> child_cred = __task_cred(child);
> if (cred->user_ns == child_cred->user_ns &&
> cap_issubset(child_cred->cap_permitted, cred->cap_permitted))
> goto out;
> - if (ns_capable(child_cred->user_ns, CAP_SYS_PTRACE))
> + if (security_capable(cred, child_cred->user_ns, CAP_SYS_PTRACE) == 0)
> goto out;
> ret = -EPERM;
> out:
> diff --git a/security/security.c b/security/security.c
> index 46f405c..e01899b 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -153,9 +153,9 @@ int security_binder_transfer_file(struct task_struct *from,
> return call_int_hook(binder_transfer_file, 0, from, to, file);
> }
>
> -int security_ptrace_access_check(struct task_struct *child, unsigned int mode)
> +int security_ptrace_access_check(struct task_struct *child, unsigned int mode, const struct cred *cred)
> {
> - return call_int_hook(ptrace_access_check, 0, child, mode);
> + return call_int_hook(ptrace_access_check, 0, child, mode, cred);
> }
>
> int security_ptrace_traceme(struct task_struct *parent)
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index d0cfaa9..0e45c89 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2036,10 +2036,10 @@ static int selinux_binder_transfer_file(struct task_struct *from,
> }
>
> static int selinux_ptrace_access_check(struct task_struct *child,
> - unsigned int mode)
> + unsigned int mode, const struct cred *cred)
> {
> if (mode & PTRACE_MODE_READ) {
> - u32 sid = current_sid();
> + u32 sid = cred_sid(cred);
> u32 csid = task_sid(child);
> return avc_has_perm(sid, csid, SECCLASS_FILE, FILE__READ, NULL);
> }
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index ff81026..866ebf5 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -410,7 +410,8 @@ static inline unsigned int smk_ptrace_mode(unsigned int mode)
>
> /**
> * smk_ptrace_rule_check - helper for ptrace access
> - * @tracer: tracer process
> + * @tracer: tracer process - only used for auditing if @tracer_cred is set
> + * @tracer_cred: creds for checks, may be different from @tracer's creds
> * @tracee_known: label entry of the process that's about to be traced
> * @mode: ptrace attachment mode (PTRACE_MODE_*)
> * @func: name of the function that called us, used for audit
> @@ -418,6 +419,7 @@ static inline unsigned int smk_ptrace_mode(unsigned int mode)
> * Returns 0 on access granted, -error on error
> */
> static int smk_ptrace_rule_check(struct task_struct *tracer,
> + const struct cred *tracer_cred,
> struct smack_known *tracee_known,
> unsigned int mode, const char *func)
> {
> @@ -433,7 +435,9 @@ static int smk_ptrace_rule_check(struct task_struct *tracer,
> }
>
> rcu_read_lock();
> - tsp = __task_cred(tracer)->security;
> + if (tracer_cred == NULL)
> + tracer_cred = __task_cred(tracer);
> + tsp = tracer_cred->security;
> tracer_known = smk_of_task(tsp);
>
> if ((mode & PTRACE_MODE_ATTACH) &&
> @@ -478,13 +482,13 @@ static int smk_ptrace_rule_check(struct task_struct *tracer,
> *
> * Do the capability checks.
> */
> -static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode)
> +static int smack_ptrace_access_check(struct task_struct *ctp, unsigned int mode, const struct cred *cred)
> {
> struct smack_known *skp;
>
> skp = smk_of_task_struct(ctp);
>
> - return smk_ptrace_rule_check(current, skp, mode, __func__);
> + return smk_ptrace_rule_check(current, cred, skp, mode, __func__);
> }
>
> /**
> @@ -502,7 +506,7 @@ static int smack_ptrace_traceme(struct task_struct *ptp)
>
> skp = smk_of_task(current_security());
>
> - rc = smk_ptrace_rule_check(ptp, skp, PTRACE_MODE_ATTACH, __func__);
> + rc = smk_ptrace_rule_check(ptp, NULL, skp, PTRACE_MODE_ATTACH, __func__);
> return rc;
> }
>
> @@ -926,7 +930,7 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm)
> rcu_read_lock();
> tracer = ptrace_parent(current);
> if (likely(tracer != NULL))
> - rc = smk_ptrace_rule_check(tracer,
> + rc = smk_ptrace_rule_check(tracer, NULL,
> isp->smk_task,
> PTRACE_MODE_ATTACH,
> __func__);
> diff --git a/security/yama/yama_lsm.c b/security/yama/yama_lsm.c
> index d3c19c9..4300181 100644
> --- a/security/yama/yama_lsm.c
> +++ b/security/yama/yama_lsm.c
> @@ -276,7 +276,7 @@ static int ptracer_exception_found(struct task_struct *tracer,
> * Returns 0 if following the ptrace is allowed, -ve on error.
> */
> static int yama_ptrace_access_check(struct task_struct *child,
> - unsigned int mode)
> + unsigned int mode, const struct cred *cred)
> {
> int rc = 0;
>
> @@ -288,7 +288,9 @@ static int yama_ptrace_access_check(struct task_struct *child,
> break;
> case YAMA_SCOPE_RELATIONAL:
> rcu_read_lock();
> - if (!task_is_descendant(current, child) &&
> + /* fail open for some procfs-based ATTACH accesses! */
> + if ((mode & PTRACE_MODE_NON_CURRENT) == 0 &&
> + !task_is_descendant(current, child) &&
> !ptracer_exception_found(current, child) &&
> !ns_capable(__task_cred(child)->user_ns, CAP_SYS_PTRACE))
> rc = -EPERM;
> @@ -296,7 +298,7 @@ static int yama_ptrace_access_check(struct task_struct *child,
> break;
> case YAMA_SCOPE_CAPABILITY:
> rcu_read_lock();
> - if (!ns_capable(__task_cred(child)->user_ns, CAP_SYS_PTRACE))
> + if (security_capable(cred, __task_cred(child)->user_ns, CAP_SYS_PTRACE) != 0)
> rc = -EPERM;
> rcu_read_unlock();
> break;
> --
> 2.1.4
>
--
Kees Cook
Chrome OS & Brillo Security
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html