Commit b21507e2 authored by Stephen Smalley's avatar Stephen Smalley Committed by Paul Moore

proc,security: move restriction on writing /proc/pid/attr nodes to proc

Processes can only alter their own security attributes via
/proc/pid/attr nodes.  This is presently enforced by each individual
security module and is also imposed by the Linux credentials
implementation, which only allows a task to alter its own credentials.
Move the check enforcing this restriction from the individual
security modules to proc_pid_attr_write() before calling the security hook,
and drop the unnecessary task argument to the security hook since it can
only ever be the current task.
Signed-off-by: Stephen Smalley's avatarStephen Smalley <sds@tycho.nsa.gov>
Acked-by: default avatarCasey Schaufler <casey@schaufler-ca.com>
Acked-by: default avatarJohn Johansen <john.johansen@canonical.com>
Signed-off-by: default avatarPaul Moore <paul@paul-moore.com>
parent be0554c9
......@@ -2488,6 +2488,12 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
length = -ESRCH;
if (!task)
goto out_no_task;
/* A task may only write its own attributes. */
length = -EACCES;
if (current != task)
goto out;
if (count > PAGE_SIZE)
count = PAGE_SIZE;
......@@ -2503,14 +2509,13 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
}
/* Guard against adverse ptrace interaction */
length = mutex_lock_interruptible(&task->signal->cred_guard_mutex);
length = mutex_lock_interruptible(&current->signal->cred_guard_mutex);
if (length < 0)
goto out_free;
length = security_setprocattr(task,
(char*)file->f_path.dentry->d_name.name,
length = security_setprocattr(file->f_path.dentry->d_name.name,
page, count);
mutex_unlock(&task->signal->cred_guard_mutex);
mutex_unlock(&current->signal->cred_guard_mutex);
out_free:
kfree(page);
out:
......
......@@ -1547,8 +1547,7 @@ union security_list_options {
void (*d_instantiate)(struct dentry *dentry, struct inode *inode);
int (*getprocattr)(struct task_struct *p, char *name, char **value);
int (*setprocattr)(struct task_struct *p, char *name, void *value,
size_t size);
int (*setprocattr)(const char *name, void *value, size_t size);
int (*ismaclabel)(const char *name);
int (*secid_to_secctx)(u32 secid, char **secdata, u32 *seclen);
int (*secctx_to_secid)(const char *secdata, u32 seclen, u32 *secid);
......
......@@ -361,7 +361,7 @@ int security_sem_semop(struct sem_array *sma, struct sembuf *sops,
unsigned nsops, int alter);
void security_d_instantiate(struct dentry *dentry, struct inode *inode);
int security_getprocattr(struct task_struct *p, char *name, char **value);
int security_setprocattr(struct task_struct *p, char *name, void *value, size_t size);
int security_setprocattr(const char *name, void *value, size_t size);
int security_netlink_send(struct sock *sk, struct sk_buff *skb);
int security_ismaclabel(const char *name);
int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
......@@ -1106,7 +1106,7 @@ static inline int security_getprocattr(struct task_struct *p, char *name, char *
return -EINVAL;
}
static inline int security_setprocattr(struct task_struct *p, char *name, void *value, size_t size)
static inline int security_setprocattr(char *name, void *value, size_t size)
{
return -EINVAL;
}
......
......@@ -495,8 +495,8 @@ static int apparmor_getprocattr(struct task_struct *task, char *name,
return error;
}
static int apparmor_setprocattr(struct task_struct *task, char *name,
void *value, size_t size)
static int apparmor_setprocattr(const char *name, void *value,
size_t size)
{
struct common_audit_data sa;
struct apparmor_audit_data aad = {0,};
......@@ -506,9 +506,6 @@ static int apparmor_setprocattr(struct task_struct *task, char *name,
if (size == 0)
return -EINVAL;
/* task can only write its own attributes */
if (current != task)
return -EACCES;
/* AppArmor requires that the buffer must be null terminated atm */
if (args[size - 1] != '\0') {
......
......@@ -1170,9 +1170,9 @@ int security_getprocattr(struct task_struct *p, char *name, char **value)
return call_int_hook(getprocattr, -EINVAL, p, name, value);
}
int security_setprocattr(struct task_struct *p, char *name, void *value, size_t size)
int security_setprocattr(const char *name, void *value, size_t size)
{
return call_int_hook(setprocattr, -EINVAL, p, name, value, size);
return call_int_hook(setprocattr, -EINVAL, name, value, size);
}
int security_netlink_send(struct sock *sk, struct sk_buff *skb)
......
......@@ -5862,8 +5862,7 @@ static int selinux_getprocattr(struct task_struct *p,
return error;
}
static int selinux_setprocattr(struct task_struct *p,
char *name, void *value, size_t size)
static int selinux_setprocattr(const char *name, void *value, size_t size)
{
struct task_security_struct *tsec;
struct cred *new;
......@@ -5871,16 +5870,6 @@ static int selinux_setprocattr(struct task_struct *p,
int error;
char *str = value;
if (current != p) {
/*
* A task may only alter its own credentials.
* SELinux has always enforced this restriction,
* and it is now mandated by the Linux credentials
* infrastructure; see Documentation/security/credentials.txt.
*/
return -EACCES;
}
/*
* Basic control over ability to set these attributes at all.
*/
......
......@@ -3620,7 +3620,6 @@ static int smack_getprocattr(struct task_struct *p, char *name, char **value)
/**
* smack_setprocattr - Smack process attribute setting
* @p: the object task
* @name: the name of the attribute in /proc/.../attr
* @value: the value to set
* @size: the size of the value
......@@ -3630,8 +3629,7 @@ static int smack_getprocattr(struct task_struct *p, char *name, char **value)
*
* Returns the length of the smack label or an error code
*/
static int smack_setprocattr(struct task_struct *p, char *name,
void *value, size_t size)
static int smack_setprocattr(const char *name, void *value, size_t size)
{
struct task_smack *tsp = current_security();
struct cred *new;
......@@ -3639,13 +3637,6 @@ static int smack_setprocattr(struct task_struct *p, char *name,
struct smack_known_list_elem *sklep;
int rc;
/*
* Changing another process' Smack value is too dangerous
* and supports no sane use case.
*/
if (p != current)
return -EPERM;
if (!smack_privileged(CAP_MAC_ADMIN) && list_empty(&tsp->smk_relabel))
return -EPERM;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment