@@ -2518,6 +2518,18 @@ static int proc_pident_readdir(struct file *file, struct dir_context *ctx,
}
#ifdef CONFIG_SECURITY
+static int proc_pid_attr_open(struct inode *inode, struct file *file)
+{
+ struct luid *opener_privunit;
+
+ opener_privunit = kmalloc(sizeof(struct luid), GFP_KERNEL);
+ if (opener_privunit == NULL)
+ return -ENOMEM;
+ *opener_privunit = current->privunit;
+ file->private_data = opener_privunit;
+ return 0;
+}
+
static ssize_t proc_pid_attr_read(struct file * file, char __user * buf,
size_t count, loff_t *ppos)
{
@@ -2546,6 +2558,7 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
void *page;
ssize_t length;
struct task_struct *task = get_proc_task(inode);
+ struct luid *opener_privunit = file->private_data;
length = -ESRCH;
if (!task)
@@ -2569,9 +2582,29 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
if (length < 0)
goto out_free;
+ /*
+ * Ensure that a process can't be tricked into writing into its own attr
+ * files without intending to do so.
+ *
+ * SELinux has a rule that prevents anyone other than `task` from
+ * writing, but if the fd stays open across execve, or is sent across a
+ * unix domain socket or whatever, that is bypassable.
+ * Same thing in AppArmor and in Smack.
+ *
+ * To prevent this, compare the opener's exec_id with the target's to
+ * ensure that they're in the same task group and no exec happened in
+ * the meantime.
+ *
+ * Why is this a file and not a prctl or whatever. :/
+ */
+ length = -EACCES;
+ if (!luid_eq(opener_privunit, &task->privunit))
+ goto out_unlock;
+
length = security_setprocattr(task,
(char*)file->f_path.dentry->d_name.name,
page, count);
+out_unlock:
mutex_unlock(&task->signal->cred_guard_mutex);
out_free:
kfree(page);
@@ -2581,10 +2614,20 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
return length;
}
+static int proc_pid_attr_release(struct inode *inode, struct file *file)
+{
+ struct luid *opener_privunit = file->private_data;
+
+ kfree(opener_privunit);
+ return 0;
+}
+
static const struct file_operations proc_pid_attr_operations = {
+ .open = proc_pid_attr_open,
.read = proc_pid_attr_read,
.write = proc_pid_attr_write,
.llseek = generic_file_llseek,
+ .release = proc_pid_attr_release,
};
static const struct pid_entry attr_dir_stuff[] = {
Make sure files in /proc/$pid/attr/ can only be written by the task that opened them. This prevents an attacking process from changing the security context of another process that it can force to write attacker-controlled data into an attacker-supplied file descriptor. I'm not sure what the impact of this is. changed in v2: - changed privunit-using code Signed-off-by: Jann Horn <jann@thejh.net> --- fs/proc/base.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)