Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

proc: clean up and fix /proc/<pid>/mem handling

Jüri Aedla reported that the /proc/<pid>/mem handling really isn't very
robust, and it also doesn't match the permission checking of any of the
other related files.

This changes it to do the permission checks at open time, and instead of
tracking the process, it tracks the VM at the time of the open. That
simplifies the code a lot, but does mean that if you hold the file
descriptor open over an execve(), you'll continue to read from the _old_
VM.

That is different from our previous behavior, but much simpler. If
somebody actually finds a load where this matters, we'll need to revert
this commit.

I suspect that nobody will ever notice - because the process mapping
addresses will also have changed as part of the execve. So you cannot
actually usefully access the fd across a VM change simply because all
the offsets for IO would have changed too.

Reported-by: Jüri Aedla <asd@ut.ee>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

+39 -106
+39 -106
fs/proc/base.c
··· 198 198 return result; 199 199 } 200 200 201 - static struct mm_struct *__check_mem_permission(struct task_struct *task) 202 - { 203 - struct mm_struct *mm; 204 - 205 - mm = get_task_mm(task); 206 - if (!mm) 207 - return ERR_PTR(-EINVAL); 208 - 209 - /* 210 - * A task can always look at itself, in case it chooses 211 - * to use system calls instead of load instructions. 212 - */ 213 - if (task == current) 214 - return mm; 215 - 216 - /* 217 - * If current is actively ptrace'ing, and would also be 218 - * permitted to freshly attach with ptrace now, permit it. 219 - */ 220 - if (task_is_stopped_or_traced(task)) { 221 - int match; 222 - rcu_read_lock(); 223 - match = (ptrace_parent(task) == current); 224 - rcu_read_unlock(); 225 - if (match && ptrace_may_access(task, PTRACE_MODE_ATTACH)) 226 - return mm; 227 - } 228 - 229 - /* 230 - * No one else is allowed. 231 - */ 232 - mmput(mm); 233 - return ERR_PTR(-EPERM); 234 - } 235 - 236 - /* 237 - * If current may access user memory in @task return a reference to the 238 - * corresponding mm, otherwise ERR_PTR. 239 - */ 240 - static struct mm_struct *check_mem_permission(struct task_struct *task) 241 - { 242 - struct mm_struct *mm; 243 - int err; 244 - 245 - /* 246 - * Avoid racing if task exec's as we might get a new mm but validate 247 - * against old credentials. 248 - */ 249 - err = mutex_lock_killable(&task->signal->cred_guard_mutex); 250 - if (err) 251 - return ERR_PTR(err); 252 - 253 - mm = __check_mem_permission(task); 254 - mutex_unlock(&task->signal->cred_guard_mutex); 255 - 256 - return mm; 257 - } 258 - 259 - struct mm_struct *mm_for_maps(struct task_struct *task) 201 + static struct mm_struct *mm_access(struct task_struct *task, unsigned int mode) 260 202 { 261 203 struct mm_struct *mm; 262 204 int err; ··· 209 267 210 268 mm = get_task_mm(task); 211 269 if (mm && mm != current->mm && 212 - !ptrace_may_access(task, PTRACE_MODE_READ)) { 270 + !ptrace_may_access(task, mode)) { 213 271 mmput(mm); 214 272 mm = ERR_PTR(-EACCES); 215 273 } 216 274 mutex_unlock(&task->signal->cred_guard_mutex); 217 275 218 276 return mm; 277 + } 278 + 279 + struct mm_struct *mm_for_maps(struct task_struct *task) 280 + { 281 + return mm_access(task, PTRACE_MODE_READ); 219 282 } 220 283 221 284 static int proc_pid_cmdline(struct task_struct *task, char * buffer) ··· 699 752 700 753 static int mem_open(struct inode* inode, struct file* file) 701 754 { 702 - file->private_data = (void*)((long)current->self_exec_id); 755 + struct task_struct *task = get_proc_task(file->f_path.dentry->d_inode); 756 + struct mm_struct *mm; 757 + 758 + if (!task) 759 + return -ESRCH; 760 + 761 + mm = mm_access(task, PTRACE_MODE_ATTACH); 762 + put_task_struct(task); 763 + 764 + if (IS_ERR(mm)) 765 + return PTR_ERR(mm); 766 + 703 767 /* OK to pass negative loff_t, we can catch out-of-range */ 704 768 file->f_mode |= FMODE_UNSIGNED_OFFSET; 769 + file->private_data = mm; 770 + 705 771 return 0; 706 772 } 707 773 708 774 static ssize_t mem_read(struct file * file, char __user * buf, 709 775 size_t count, loff_t *ppos) 710 776 { 711 - struct task_struct *task = get_proc_task(file->f_path.dentry->d_inode); 777 + int ret; 712 778 char *page; 713 779 unsigned long src = *ppos; 714 - int ret = -ESRCH; 715 - struct mm_struct *mm; 780 + struct mm_struct *mm = file->private_data; 716 781 717 - if (!task) 718 - goto out_no_task; 782 + if (!mm) 783 + return 0; 719 784 720 - ret = -ENOMEM; 721 785 page = (char *)__get_free_page(GFP_TEMPORARY); 722 786 if (!page) 723 - goto out; 724 - 725 - mm = check_mem_permission(task); 726 - ret = PTR_ERR(mm); 727 - if (IS_ERR(mm)) 728 - goto out_free; 729 - 730 - ret = -EIO; 731 - 732 - if (file->private_data != (void*)((long)current->self_exec_id)) 733 - goto out_put; 787 + return -ENOMEM; 734 788 735 789 ret = 0; 736 790 ··· 758 810 } 759 811 *ppos = src; 760 812 761 - out_put: 762 - mmput(mm); 763 - out_free: 764 813 free_page((unsigned long) page); 765 - out: 766 - put_task_struct(task); 767 - out_no_task: 768 814 return ret; 769 815 } 770 816 ··· 767 825 { 768 826 int copied; 769 827 char *page; 770 - struct task_struct *task = get_proc_task(file->f_path.dentry->d_inode); 771 828 unsigned long dst = *ppos; 772 - struct mm_struct *mm; 829 + struct mm_struct *mm = file->private_data; 773 830 774 - copied = -ESRCH; 775 - if (!task) 776 - goto out_no_task; 831 + if (!mm) 832 + return 0; 777 833 778 - copied = -ENOMEM; 779 834 page = (char *)__get_free_page(GFP_TEMPORARY); 780 835 if (!page) 781 - goto out_task; 782 - 783 - mm = check_mem_permission(task); 784 - copied = PTR_ERR(mm); 785 - if (IS_ERR(mm)) 786 - goto out_free; 787 - 788 - copied = -EIO; 789 - if (file->private_data != (void *)((long)current->self_exec_id)) 790 - goto out_mm; 836 + return -ENOMEM; 791 837 792 838 copied = 0; 793 839 while (count > 0) { ··· 799 869 } 800 870 *ppos = dst; 801 871 802 - out_mm: 803 - mmput(mm); 804 - out_free: 805 872 free_page((unsigned long) page); 806 - out_task: 807 - put_task_struct(task); 808 - out_no_task: 809 873 return copied; 810 874 } 811 875 ··· 819 895 return file->f_pos; 820 896 } 821 897 898 + static int mem_release(struct inode *inode, struct file *file) 899 + { 900 + struct mm_struct *mm = file->private_data; 901 + 902 + mmput(mm); 903 + return 0; 904 + } 905 + 822 906 static const struct file_operations proc_mem_operations = { 823 907 .llseek = mem_lseek, 824 908 .read = mem_read, 825 909 .write = mem_write, 826 910 .open = mem_open, 911 + .release = mem_release, 827 912 }; 828 913 829 914 static ssize_t environ_read(struct file *file, char __user *buf,