Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
James Pearson wrote: > Arvin Moezzi wrote: > >> I think that's not true. 'count' is changing through the iteration. >> The difference in the mem_read(): >> >> * while (count > 0) { >> * int this_len, retval; >> * >> * this_len = (count > PAGE_SIZE) ? PAGE_SIZE : count; >> * retval = access_process_vm(task, src, page, this_len, 0); >> * >> * ... >> * } >> >> is the fact, that this_len = min(PAGE_SIZE, count) is in the >> iteration block, hence retval <= this_len <= count in each iteration >> step. So this is ok. But IMHO in your code 'retval' may be bigger than >> 'count' in the last iteration of the block, because 'max_len' is fix >> through your iteration but 'count' is changing. Or am i missing >> something? > > > Yes, you are correct ... Here is a new patch that fixes the above issue ... However, I'm not sure if I should be using GFP_TEMPORARY, GFP_KERNEL or GFP_USER ? Thanks James Pearson Patch against 2.6.23-rc6-mm1 --- /proc/PID/environ currently truncates at 4096 characters, patch based on the /proc/PID/mem code. Signed-off-by: James Pearson <[EMAIL PROTECTED]> --- ./fs/proc/base.c.dist 2007-09-19 12:29:46.244929651 +0100 +++ ./fs/proc/base.c2007-09-25 12:40:53.194497911 +0100 @@ -202,27 +202,6 @@ static int proc_root_link(struct inode * (task->state == TASK_STOPPED || task->state == TASK_TRACED) && \ security_ptrace(current,task) == 0)) -static int proc_pid_environ(struct task_struct *task, char * buffer) -{ - int res = 0; - struct mm_struct *mm = get_task_mm(task); - if (mm) { - unsigned int len; - - res = -ESRCH; - if (!ptrace_may_attach(task)) - goto out; - - len = mm->env_end - mm->env_start; - if (len > PAGE_SIZE) - len = PAGE_SIZE; - res = access_process_vm(task, mm->env_start, buffer, len, 0); -out: - mmput(mm); - } - return res; -} - static int proc_pid_cmdline(struct task_struct *task, char * buffer) { int res = 0; @@ -740,6 +719,76 @@ static const struct file_operations proc .open = mem_open, }; +static ssize_t environ_read(struct file *file, char __user *buf, + size_t count, loff_t *ppos) +{ + struct task_struct *task = get_proc_task(file->f_dentry->d_inode); + char *page; + unsigned long src = *ppos; + int ret = -ESRCH; + struct mm_struct *mm; + + if (!task) + goto out_no_task; + + if (!ptrace_may_attach(task)) + goto out; + + ret = -ENOMEM; + page = (char *)__get_free_page(GFP_TEMPORARY); + if (!page) + goto out; + + ret = 0; + + mm = get_task_mm(task); + if (!mm) + goto out_free; + + while (count > 0) { + int this_len, retval, max_len; + + this_len = mm->env_end - (mm->env_start + src); + + if (this_len <= 0) + break; + + max_len = (count > PAGE_SIZE) ? PAGE_SIZE : count; + this_len = (this_len > max_len) ? max_len : this_len; + + retval = access_process_vm(task, (mm->env_start + src), + page, this_len, 0); + + if (retval <= 0) { + ret = retval; + break; + } + + if (copy_to_user(buf, page, retval)) { + ret = -EFAULT; + break; + } + + ret += retval; + src += retval; + buf += retval; + count -= retval; + } + *ppos = src; + + mmput(mm); +out_free: + free_page((unsigned long) page); +out: + put_task_struct(task); +out_no_task: + return ret; +} + +static const struct file_operations proc_environ_operations = { + .read = environ_read, +}; + static ssize_t oom_adjust_read(struct file *file, char __user *buf, size_t count, loff_t *ppos) { @@ -2092,7 +2141,7 @@ static const struct pid_entry tgid_base_ DIR("task", S_IRUGO|S_IXUGO, task), DIR("fd", S_IRUSR|S_IXUSR, fd), DIR("fdinfo", S_IRUSR|S_IXUSR, fdinfo), - INF("environ",S_IRUSR, pid_environ), + REG("environ",S_IRUSR, environ), INF("auxv", S_IRUSR, pid_auxv), INF("status", S_IRUGO, pid_status), INF("limits", S_IRUSR, pid_limits), @@ -2421,7 +2470,7 @@ out_no_task: static const struct pid_entry tid_base_stuff[] = { DIR("fd",S_IRUSR|S_IXUSR, fd), DIR("fdinfo",S_IRUSR|S_IXUSR, fdinfo), - INF("environ", S_IRUSR, pid_environ), + REG("environ", S_IRUSR, environ), INF("auxv", S_IRUSR, pid_auxv), INF("status",S_IRUGO, pid_status), INF("limits",
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
James Pearson wrote: Arvin Moezzi wrote: I think that's not true. 'count' is changing through the iteration. The difference in the mem_read(): * while (count 0) { * int this_len, retval; * * this_len = (count PAGE_SIZE) ? PAGE_SIZE : count; * retval = access_process_vm(task, src, page, this_len, 0); * * ... * } is the fact, that this_len = min(PAGE_SIZE, count) is in the iteration block, hence retval = this_len = count in each iteration step. So this is ok. But IMHO in your code 'retval' may be bigger than 'count' in the last iteration of the block, because 'max_len' is fix through your iteration but 'count' is changing. Or am i missing something? Yes, you are correct ... Here is a new patch that fixes the above issue ... However, I'm not sure if I should be using GFP_TEMPORARY, GFP_KERNEL or GFP_USER ? Thanks James Pearson Patch against 2.6.23-rc6-mm1 --- /proc/PID/environ currently truncates at 4096 characters, patch based on the /proc/PID/mem code. Signed-off-by: James Pearson [EMAIL PROTECTED] --- ./fs/proc/base.c.dist 2007-09-19 12:29:46.244929651 +0100 +++ ./fs/proc/base.c2007-09-25 12:40:53.194497911 +0100 @@ -202,27 +202,6 @@ static int proc_root_link(struct inode * (task-state == TASK_STOPPED || task-state == TASK_TRACED) \ security_ptrace(current,task) == 0)) -static int proc_pid_environ(struct task_struct *task, char * buffer) -{ - int res = 0; - struct mm_struct *mm = get_task_mm(task); - if (mm) { - unsigned int len; - - res = -ESRCH; - if (!ptrace_may_attach(task)) - goto out; - - len = mm-env_end - mm-env_start; - if (len PAGE_SIZE) - len = PAGE_SIZE; - res = access_process_vm(task, mm-env_start, buffer, len, 0); -out: - mmput(mm); - } - return res; -} - static int proc_pid_cmdline(struct task_struct *task, char * buffer) { int res = 0; @@ -740,6 +719,76 @@ static const struct file_operations proc .open = mem_open, }; +static ssize_t environ_read(struct file *file, char __user *buf, + size_t count, loff_t *ppos) +{ + struct task_struct *task = get_proc_task(file-f_dentry-d_inode); + char *page; + unsigned long src = *ppos; + int ret = -ESRCH; + struct mm_struct *mm; + + if (!task) + goto out_no_task; + + if (!ptrace_may_attach(task)) + goto out; + + ret = -ENOMEM; + page = (char *)__get_free_page(GFP_TEMPORARY); + if (!page) + goto out; + + ret = 0; + + mm = get_task_mm(task); + if (!mm) + goto out_free; + + while (count 0) { + int this_len, retval, max_len; + + this_len = mm-env_end - (mm-env_start + src); + + if (this_len = 0) + break; + + max_len = (count PAGE_SIZE) ? PAGE_SIZE : count; + this_len = (this_len max_len) ? max_len : this_len; + + retval = access_process_vm(task, (mm-env_start + src), + page, this_len, 0); + + if (retval = 0) { + ret = retval; + break; + } + + if (copy_to_user(buf, page, retval)) { + ret = -EFAULT; + break; + } + + ret += retval; + src += retval; + buf += retval; + count -= retval; + } + *ppos = src; + + mmput(mm); +out_free: + free_page((unsigned long) page); +out: + put_task_struct(task); +out_no_task: + return ret; +} + +static const struct file_operations proc_environ_operations = { + .read = environ_read, +}; + static ssize_t oom_adjust_read(struct file *file, char __user *buf, size_t count, loff_t *ppos) { @@ -2092,7 +2141,7 @@ static const struct pid_entry tgid_base_ DIR(task, S_IRUGO|S_IXUGO, task), DIR(fd, S_IRUSR|S_IXUSR, fd), DIR(fdinfo, S_IRUSR|S_IXUSR, fdinfo), - INF(environ,S_IRUSR, pid_environ), + REG(environ,S_IRUSR, environ), INF(auxv, S_IRUSR, pid_auxv), INF(status, S_IRUGO, pid_status), INF(limits, S_IRUSR, pid_limits), @@ -2421,7 +2470,7 @@ out_no_task: static const struct pid_entry tid_base_stuff[] = { DIR(fd,S_IRUSR|S_IXUSR, fd), DIR(fdinfo,S_IRUSR|S_IXUSR, fdinfo), - INF(environ, S_IRUSR, pid_environ), + REG(environ, S_IRUSR, environ), INF(auxv, S_IRUSR, pid_auxv), INF(status,S_IRUGO, pid_status), INF(limits,S_IRUSR, pid_limits), - To unsubscribe from this list: send the line unsubscribe linux-kernel
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
Arvin Moezzi wrote: I think that's not true. 'count' is changing through the iteration. The difference in the mem_read(): * while (count > 0) { * int this_len, retval; * * this_len = (count > PAGE_SIZE) ? PAGE_SIZE : count; * retval = access_process_vm(task, src, page, this_len, 0); * * ... * } is the fact, that this_len = min(PAGE_SIZE, count) is in the iteration block, hence retval <= this_len <= count in each iteration step. So this is ok. But IMHO in your code 'retval' may be bigger than 'count' in the last iteration of the block, because 'max_len' is fix through your iteration but 'count' is changing. Or am i missing something? Yes, you are correct ... Thanks James Pearson - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
> >>+ > >>+ if (copy_to_user(buf, page, retval)) { > > > > > > shouldn't you only copy min(count,retval) bytes? otherwise you could > > write beyond the users buffer "buf", right? > > AFAIK, 'retval' can never be greater than 'this_len', which can never be > greater than 'max_len', which can never be greater than 'count' I think that's not true. 'count' is changing through the iteration. The difference in the mem_read(): * while (count > 0) { * int this_len, retval; * * this_len = (count > PAGE_SIZE) ? PAGE_SIZE : count; * retval = access_process_vm(task, src, page, this_len, 0); * * ... * } is the fact, that this_len = min(PAGE_SIZE, count) is in the iteration block, hence retval <= this_len <= count in each iteration step. So this is ok. But IMHO in your code 'retval' may be bigger than 'count' in the last iteration of the block, because 'max_len' is fix through your iteration but 'count' is changing. Or am i missing something? > James Pearson Arvin - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
On (20/09/07 16:46), Andrew Morton didst pronounce: > On Wed, 19 Sep 2007 14:35:29 +0100 > "James Pearson" <[EMAIL PROTECTED]> wrote: > > > > > From: James Pearson <[EMAIL PROTECTED]> > > > > /proc/PID/environ currently truncates at 4096 characters, patch based on > > the /proc/PID/mem code. > > patch needs to be carefully reviewed from the security POV (ie: permissions) > as well as for correctness. Does anyone have time to do that? > > > Signed-off-by: James Pearson <[EMAIL PROTECTED]> > > > > --- ./fs/proc/base.c.dist 2007-09-19 12:29:46.244929651 +0100 > > +++ ./fs/proc/base.c2007-09-19 12:36:18.155648760 +0100 > > @@ -202,27 +202,6 @@ static int proc_root_link(struct inode * > > (task->state == TASK_STOPPED || task->state == TASK_TRACED) && \ > > security_ptrace(current,task) == 0)) > > > > -static int proc_pid_environ(struct task_struct *task, char * buffer) > > -{ > > - int res = 0; > > - struct mm_struct *mm = get_task_mm(task); > > - if (mm) { > > - unsigned int len; > > - > > - res = -ESRCH; > > - if (!ptrace_may_attach(task)) > > - goto out; > > - > > - len = mm->env_end - mm->env_start; > > - if (len > PAGE_SIZE) > > - len = PAGE_SIZE; > > - res = access_process_vm(task, mm->env_start, buffer, len, 0); > > -out: > > - mmput(mm); > > - } > > - return res; > > -} > > - > > static int proc_pid_cmdline(struct task_struct *task, char * buffer) > > { > > int res = 0; > > @@ -740,6 +719,79 @@ static const struct file_operations proc > > .open = mem_open, > > }; > > > > +static ssize_t environ_read(struct file *file, char __user *buf, > > + size_t count, loff_t *ppos) > > +{ > > + struct task_struct *task = get_proc_task(file->f_dentry->d_inode); > > + char *page; > > + unsigned long src = *ppos; > > + int ret = -ESRCH; > > + struct mm_struct *mm; > > + size_t max_len; > > + > > + if (!task) > > + goto out_no_task; > > + > > + if (!ptrace_may_attach(task)) > > + goto out; > > + > > + ret = -ENOMEM; > > + page = (char *)__get_free_page(GFP_TEMPORARY); > > Now I wonder what inspired you to reach for GFP_TEMPORARY? Perhaps the > fact that it is crappily named and undocumented. > Because it's a very short lived allocation? He allocates it here and frees it at the end of the function again. > This should be GFP_KERNEL - the page you're allocating here is not > reclaimable by the VM. > The patch leader documented how this works although you're right in that the code doesn't explain it adequately. Temporary and short-lived allocations are grouped with kernel-reclaimable pages such as inode and dcache pages intentionally. > > + if (!page) > > + goto out; > > + > > + ret = 0; > > + > > + mm = get_task_mm(task); > > + if (!mm) > > + goto out_free; > > + > > + max_len = (count > PAGE_SIZE) ? PAGE_SIZE : count; > > + > > + while (count > 0) { > > + int this_len, retval; > > + > > + this_len = mm->env_end - (mm->env_start + src); > > + > > + if (this_len <= 0) > > + break; > > + > > + if (this_len > max_len) > > + this_len = max_len; > > + > > + retval = access_process_vm(task, (mm->env_start + src), > > + page, this_len, 0); > > + > > + if (retval <= 0) { > > + ret = retval; > > + break; > > + } > > + > > + if (copy_to_user(buf, page, retval)) { > > + ret = -EFAULT; > > + break; > > + } > > + > > + ret += retval; > > + src += retval; > > + buf += retval; > > + count -= retval; > > + } > > Now that's a funky loop. Someone please convince me that there is no way > in which `count - retval' can ever go negative (ie: huge positive). > > > > + *ppos = src; > > + > > + mmput(mm); > > +out_free: > > + free_page((unsigned long) page); > > +out: > > + put_task_struct(task); > > +out_no_task: > > + return ret; > > +} > > + > > +static const struct file_operations proc_environ_operations = { > > + .read = environ_read, > > +}; > > + > > static ssize_t oom_adjust_read(struct file *file, char __user *buf, > > size_t count, loff_t *ppos) > > { > > @@ -2092,7 +2144,7 @@ static const struct pid_entry tgid_base_ > > DIR("task", S_IRUGO|S_IXUGO, task), > > DIR("fd", S_IRUSR|S_IXUSR, fd), > > DIR("fdinfo", S_IRUSR|S_IXUSR, fdinfo), > > - INF("environ",S_IRUSR, pid_environ), > > + REG("environ",S_IRUSR, environ), > > INF("auxv", S_IRUSR, pid_auxv), > > INF("status", S_IRUGO, pid_status), > > INF("limits", S_IRUSR, pid_limits), > > @@ -2421,7 +2473,7 @@ out_no_task: > > static const struct
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
Arvin Moezzi wrote: 2007/9/19, James Pearson <[EMAIL PROTECTED]>: + while (count > 0) { + int this_len, retval; + + this_len = mm->env_end - (mm->env_start + src); + + if (this_len <= 0) + break; + + if (this_len > max_len) + this_len = max_len; + + retval = access_process_vm(task, (mm->env_start + src), + page, this_len, 0); + + if (retval <= 0) { + ret = retval; + break; + } + + if (copy_to_user(buf, page, retval)) { shouldn't you only copy min(count,retval) bytes? otherwise you could write beyond the users buffer "buf", right? AFAIK, 'retval' can never be greater than 'this_len', which can never be greater than 'max_len', which can never be greater than 'count' James Pearson - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
Andrew Morton wrote: On Wed, 19 Sep 2007 14:35:29 +0100 "James Pearson" <[EMAIL PROTECTED]> wrote: From: James Pearson <[EMAIL PROTECTED]> /proc/PID/environ currently truncates at 4096 characters, patch based on the /proc/PID/mem code. patch needs to be carefully reviewed from the security POV (ie: permissions) as well as for correctness. Does anyone have time to do that? Signed-off-by: James Pearson <[EMAIL PROTECTED]> --- ./fs/proc/base.c.dist 2007-09-19 12:29:46.244929651 +0100 +++ ./fs/proc/base.c2007-09-19 12:36:18.155648760 +0100 @@ -202,27 +202,6 @@ static int proc_root_link(struct inode * (task->state == TASK_STOPPED || task->state == TASK_TRACED) && \ security_ptrace(current,task) == 0)) -static int proc_pid_environ(struct task_struct *task, char * buffer) -{ - int res = 0; - struct mm_struct *mm = get_task_mm(task); - if (mm) { - unsigned int len; - - res = -ESRCH; - if (!ptrace_may_attach(task)) - goto out; - - len = mm->env_end - mm->env_start; - if (len > PAGE_SIZE) - len = PAGE_SIZE; - res = access_process_vm(task, mm->env_start, buffer, len, 0); -out: - mmput(mm); - } - return res; -} - static int proc_pid_cmdline(struct task_struct *task, char * buffer) { int res = 0; @@ -740,6 +719,79 @@ static const struct file_operations proc .open = mem_open, }; +static ssize_t environ_read(struct file *file, char __user *buf, + size_t count, loff_t *ppos) +{ + struct task_struct *task = get_proc_task(file->f_dentry->d_inode); + char *page; + unsigned long src = *ppos; + int ret = -ESRCH; + struct mm_struct *mm; + size_t max_len; + + if (!task) + goto out_no_task; + + if (!ptrace_may_attach(task)) + goto out; + + ret = -ENOMEM; + page = (char *)__get_free_page(GFP_TEMPORARY); Now I wonder what inspired you to reach for GFP_TEMPORARY? Perhaps the fact that it is crappily named and undocumented. This should be GFP_KERNEL - the page you're allocating here is not reclaimable by the VM. The code is based on mem_read() - and that is what mem_read() does in 2.6.23rc6-mm1 - my previous patch for 2.6.23rc5 used GFP_USER, as that is what mem_read() does in 2.6.23rc5. + if (!page) + goto out; + + ret = 0; + + mm = get_task_mm(task); + if (!mm) + goto out_free; + + max_len = (count > PAGE_SIZE) ? PAGE_SIZE : count; + + while (count > 0) { + int this_len, retval; + + this_len = mm->env_end - (mm->env_start + src); + + if (this_len <= 0) + break; + + if (this_len > max_len) + this_len = max_len; + + retval = access_process_vm(task, (mm->env_start + src), + page, this_len, 0); + + if (retval <= 0) { + ret = retval; + break; + } + + if (copy_to_user(buf, page, retval)) { + ret = -EFAULT; + break; + } + + ret += retval; + src += retval; + buf += retval; + count -= retval; + } Now that's a funky loop. Someone please convince me that there is no way in which `count - retval' can ever go negative (ie: huge positive). Again, this is exactly the same as in mem_read() James Pearson - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
Arvin Moezzi wrote: 2007/9/19, James Pearson [EMAIL PROTECTED]: + while (count 0) { + int this_len, retval; + + this_len = mm-env_end - (mm-env_start + src); + + if (this_len = 0) + break; + + if (this_len max_len) + this_len = max_len; + + retval = access_process_vm(task, (mm-env_start + src), + page, this_len, 0); + + if (retval = 0) { + ret = retval; + break; + } + + if (copy_to_user(buf, page, retval)) { shouldn't you only copy min(count,retval) bytes? otherwise you could write beyond the users buffer buf, right? AFAIK, 'retval' can never be greater than 'this_len', which can never be greater than 'max_len', which can never be greater than 'count' James Pearson - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
Andrew Morton wrote: On Wed, 19 Sep 2007 14:35:29 +0100 James Pearson [EMAIL PROTECTED] wrote: From: James Pearson [EMAIL PROTECTED] /proc/PID/environ currently truncates at 4096 characters, patch based on the /proc/PID/mem code. patch needs to be carefully reviewed from the security POV (ie: permissions) as well as for correctness. Does anyone have time to do that? Signed-off-by: James Pearson [EMAIL PROTECTED] --- ./fs/proc/base.c.dist 2007-09-19 12:29:46.244929651 +0100 +++ ./fs/proc/base.c2007-09-19 12:36:18.155648760 +0100 @@ -202,27 +202,6 @@ static int proc_root_link(struct inode * (task-state == TASK_STOPPED || task-state == TASK_TRACED) \ security_ptrace(current,task) == 0)) -static int proc_pid_environ(struct task_struct *task, char * buffer) -{ - int res = 0; - struct mm_struct *mm = get_task_mm(task); - if (mm) { - unsigned int len; - - res = -ESRCH; - if (!ptrace_may_attach(task)) - goto out; - - len = mm-env_end - mm-env_start; - if (len PAGE_SIZE) - len = PAGE_SIZE; - res = access_process_vm(task, mm-env_start, buffer, len, 0); -out: - mmput(mm); - } - return res; -} - static int proc_pid_cmdline(struct task_struct *task, char * buffer) { int res = 0; @@ -740,6 +719,79 @@ static const struct file_operations proc .open = mem_open, }; +static ssize_t environ_read(struct file *file, char __user *buf, + size_t count, loff_t *ppos) +{ + struct task_struct *task = get_proc_task(file-f_dentry-d_inode); + char *page; + unsigned long src = *ppos; + int ret = -ESRCH; + struct mm_struct *mm; + size_t max_len; + + if (!task) + goto out_no_task; + + if (!ptrace_may_attach(task)) + goto out; + + ret = -ENOMEM; + page = (char *)__get_free_page(GFP_TEMPORARY); Now I wonder what inspired you to reach for GFP_TEMPORARY? Perhaps the fact that it is crappily named and undocumented. This should be GFP_KERNEL - the page you're allocating here is not reclaimable by the VM. The code is based on mem_read() - and that is what mem_read() does in 2.6.23rc6-mm1 - my previous patch for 2.6.23rc5 used GFP_USER, as that is what mem_read() does in 2.6.23rc5. + if (!page) + goto out; + + ret = 0; + + mm = get_task_mm(task); + if (!mm) + goto out_free; + + max_len = (count PAGE_SIZE) ? PAGE_SIZE : count; + + while (count 0) { + int this_len, retval; + + this_len = mm-env_end - (mm-env_start + src); + + if (this_len = 0) + break; + + if (this_len max_len) + this_len = max_len; + + retval = access_process_vm(task, (mm-env_start + src), + page, this_len, 0); + + if (retval = 0) { + ret = retval; + break; + } + + if (copy_to_user(buf, page, retval)) { + ret = -EFAULT; + break; + } + + ret += retval; + src += retval; + buf += retval; + count -= retval; + } Now that's a funky loop. Someone please convince me that there is no way in which `count - retval' can ever go negative (ie: huge positive). Again, this is exactly the same as in mem_read() James Pearson - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
On (20/09/07 16:46), Andrew Morton didst pronounce: On Wed, 19 Sep 2007 14:35:29 +0100 James Pearson [EMAIL PROTECTED] wrote: From: James Pearson [EMAIL PROTECTED] /proc/PID/environ currently truncates at 4096 characters, patch based on the /proc/PID/mem code. patch needs to be carefully reviewed from the security POV (ie: permissions) as well as for correctness. Does anyone have time to do that? Signed-off-by: James Pearson [EMAIL PROTECTED] --- ./fs/proc/base.c.dist 2007-09-19 12:29:46.244929651 +0100 +++ ./fs/proc/base.c2007-09-19 12:36:18.155648760 +0100 @@ -202,27 +202,6 @@ static int proc_root_link(struct inode * (task-state == TASK_STOPPED || task-state == TASK_TRACED) \ security_ptrace(current,task) == 0)) -static int proc_pid_environ(struct task_struct *task, char * buffer) -{ - int res = 0; - struct mm_struct *mm = get_task_mm(task); - if (mm) { - unsigned int len; - - res = -ESRCH; - if (!ptrace_may_attach(task)) - goto out; - - len = mm-env_end - mm-env_start; - if (len PAGE_SIZE) - len = PAGE_SIZE; - res = access_process_vm(task, mm-env_start, buffer, len, 0); -out: - mmput(mm); - } - return res; -} - static int proc_pid_cmdline(struct task_struct *task, char * buffer) { int res = 0; @@ -740,6 +719,79 @@ static const struct file_operations proc .open = mem_open, }; +static ssize_t environ_read(struct file *file, char __user *buf, + size_t count, loff_t *ppos) +{ + struct task_struct *task = get_proc_task(file-f_dentry-d_inode); + char *page; + unsigned long src = *ppos; + int ret = -ESRCH; + struct mm_struct *mm; + size_t max_len; + + if (!task) + goto out_no_task; + + if (!ptrace_may_attach(task)) + goto out; + + ret = -ENOMEM; + page = (char *)__get_free_page(GFP_TEMPORARY); Now I wonder what inspired you to reach for GFP_TEMPORARY? Perhaps the fact that it is crappily named and undocumented. Because it's a very short lived allocation? He allocates it here and frees it at the end of the function again. This should be GFP_KERNEL - the page you're allocating here is not reclaimable by the VM. The patch leader documented how this works although you're right in that the code doesn't explain it adequately. Temporary and short-lived allocations are grouped with kernel-reclaimable pages such as inode and dcache pages intentionally. + if (!page) + goto out; + + ret = 0; + + mm = get_task_mm(task); + if (!mm) + goto out_free; + + max_len = (count PAGE_SIZE) ? PAGE_SIZE : count; + + while (count 0) { + int this_len, retval; + + this_len = mm-env_end - (mm-env_start + src); + + if (this_len = 0) + break; + + if (this_len max_len) + this_len = max_len; + + retval = access_process_vm(task, (mm-env_start + src), + page, this_len, 0); + + if (retval = 0) { + ret = retval; + break; + } + + if (copy_to_user(buf, page, retval)) { + ret = -EFAULT; + break; + } + + ret += retval; + src += retval; + buf += retval; + count -= retval; + } Now that's a funky loop. Someone please convince me that there is no way in which `count - retval' can ever go negative (ie: huge positive). + *ppos = src; + + mmput(mm); +out_free: + free_page((unsigned long) page); +out: + put_task_struct(task); +out_no_task: + return ret; +} + +static const struct file_operations proc_environ_operations = { + .read = environ_read, +}; + static ssize_t oom_adjust_read(struct file *file, char __user *buf, size_t count, loff_t *ppos) { @@ -2092,7 +2144,7 @@ static const struct pid_entry tgid_base_ DIR(task, S_IRUGO|S_IXUGO, task), DIR(fd, S_IRUSR|S_IXUSR, fd), DIR(fdinfo, S_IRUSR|S_IXUSR, fdinfo), - INF(environ,S_IRUSR, pid_environ), + REG(environ,S_IRUSR, environ), INF(auxv, S_IRUSR, pid_auxv), INF(status, S_IRUGO, pid_status), INF(limits, S_IRUSR, pid_limits), @@ -2421,7 +2473,7 @@ out_no_task: static const struct pid_entry tid_base_stuff[] = { DIR(fd,S_IRUSR|S_IXUSR, fd), DIR(fdinfo,S_IRUSR|S_IXUSR, fdinfo), - INF(environ, S_IRUSR, pid_environ), + REG(environ, S_IRUSR, environ), INF(auxv, S_IRUSR, pid_auxv), INF(status,S_IRUGO, pid_status), INF(limits,S_IRUSR,
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
+ + if (copy_to_user(buf, page, retval)) { shouldn't you only copy min(count,retval) bytes? otherwise you could write beyond the users buffer buf, right? AFAIK, 'retval' can never be greater than 'this_len', which can never be greater than 'max_len', which can never be greater than 'count' I think that's not true. 'count' is changing through the iteration. The difference in the mem_read(): * while (count 0) { * int this_len, retval; * * this_len = (count PAGE_SIZE) ? PAGE_SIZE : count; * retval = access_process_vm(task, src, page, this_len, 0); * * ... * } is the fact, that this_len = min(PAGE_SIZE, count) is in the iteration block, hence retval = this_len = count in each iteration step. So this is ok. But IMHO in your code 'retval' may be bigger than 'count' in the last iteration of the block, because 'max_len' is fix through your iteration but 'count' is changing. Or am i missing something? James Pearson Arvin - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
Arvin Moezzi wrote: I think that's not true. 'count' is changing through the iteration. The difference in the mem_read(): * while (count 0) { * int this_len, retval; * * this_len = (count PAGE_SIZE) ? PAGE_SIZE : count; * retval = access_process_vm(task, src, page, this_len, 0); * * ... * } is the fact, that this_len = min(PAGE_SIZE, count) is in the iteration block, hence retval = this_len = count in each iteration step. So this is ok. But IMHO in your code 'retval' may be bigger than 'count' in the last iteration of the block, because 'max_len' is fix through your iteration but 'count' is changing. Or am i missing something? Yes, you are correct ... Thanks James Pearson - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
2007/9/19, James Pearson <[EMAIL PROTECTED]>: > + while (count > 0) { > + int this_len, retval; > + > + this_len = mm->env_end - (mm->env_start + src); > + > + if (this_len <= 0) > + break; > + > + if (this_len > max_len) > + this_len = max_len; > + > + retval = access_process_vm(task, (mm->env_start + src), > + page, this_len, 0); > + > + if (retval <= 0) { > + ret = retval; > + break; > + } > + > + if (copy_to_user(buf, page, retval)) { shouldn't you only copy min(count,retval) bytes? otherwise you could write beyond the users buffer "buf", right? Arvin - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
On Wed, 19 Sep 2007 14:35:29 +0100 "James Pearson" <[EMAIL PROTECTED]> wrote: > > From: James Pearson <[EMAIL PROTECTED]> > > /proc/PID/environ currently truncates at 4096 characters, patch based on > the /proc/PID/mem code. patch needs to be carefully reviewed from the security POV (ie: permissions) as well as for correctness. Does anyone have time to do that? > Signed-off-by: James Pearson <[EMAIL PROTECTED]> > > --- ./fs/proc/base.c.dist 2007-09-19 12:29:46.244929651 +0100 > +++ ./fs/proc/base.c 2007-09-19 12:36:18.155648760 +0100 > @@ -202,27 +202,6 @@ static int proc_root_link(struct inode * >(task->state == TASK_STOPPED || task->state == TASK_TRACED) && \ >security_ptrace(current,task) == 0)) > > -static int proc_pid_environ(struct task_struct *task, char * buffer) > -{ > - int res = 0; > - struct mm_struct *mm = get_task_mm(task); > - if (mm) { > - unsigned int len; > - > - res = -ESRCH; > - if (!ptrace_may_attach(task)) > - goto out; > - > - len = mm->env_end - mm->env_start; > - if (len > PAGE_SIZE) > - len = PAGE_SIZE; > - res = access_process_vm(task, mm->env_start, buffer, len, 0); > -out: > - mmput(mm); > - } > - return res; > -} > - > static int proc_pid_cmdline(struct task_struct *task, char * buffer) > { > int res = 0; > @@ -740,6 +719,79 @@ static const struct file_operations proc > .open = mem_open, > }; > > +static ssize_t environ_read(struct file *file, char __user *buf, > + size_t count, loff_t *ppos) > +{ > + struct task_struct *task = get_proc_task(file->f_dentry->d_inode); > + char *page; > + unsigned long src = *ppos; > + int ret = -ESRCH; > + struct mm_struct *mm; > + size_t max_len; > + > + if (!task) > + goto out_no_task; > + > + if (!ptrace_may_attach(task)) > + goto out; > + > + ret = -ENOMEM; > + page = (char *)__get_free_page(GFP_TEMPORARY); Now I wonder what inspired you to reach for GFP_TEMPORARY? Perhaps the fact that it is crappily named and undocumented. This should be GFP_KERNEL - the page you're allocating here is not reclaimable by the VM. > + if (!page) > + goto out; > + > + ret = 0; > + > + mm = get_task_mm(task); > + if (!mm) > + goto out_free; > + > + max_len = (count > PAGE_SIZE) ? PAGE_SIZE : count; > + > + while (count > 0) { > + int this_len, retval; > + > + this_len = mm->env_end - (mm->env_start + src); > + > + if (this_len <= 0) > + break; > + > + if (this_len > max_len) > + this_len = max_len; > + > + retval = access_process_vm(task, (mm->env_start + src), > + page, this_len, 0); > + > + if (retval <= 0) { > + ret = retval; > + break; > + } > + > + if (copy_to_user(buf, page, retval)) { > + ret = -EFAULT; > + break; > + } > + > + ret += retval; > + src += retval; > + buf += retval; > + count -= retval; > + } Now that's a funky loop. Someone please convince me that there is no way in which `count - retval' can ever go negative (ie: huge positive). > + *ppos = src; > + > + mmput(mm); > +out_free: > + free_page((unsigned long) page); > +out: > + put_task_struct(task); > +out_no_task: > + return ret; > +} > + > +static const struct file_operations proc_environ_operations = { > + .read = environ_read, > +}; > + > static ssize_t oom_adjust_read(struct file *file, char __user *buf, > size_t count, loff_t *ppos) > { > @@ -2092,7 +2144,7 @@ static const struct pid_entry tgid_base_ > DIR("task", S_IRUGO|S_IXUGO, task), > DIR("fd", S_IRUSR|S_IXUSR, fd), > DIR("fdinfo", S_IRUSR|S_IXUSR, fdinfo), > - INF("environ",S_IRUSR, pid_environ), > + REG("environ",S_IRUSR, environ), > INF("auxv", S_IRUSR, pid_auxv), > INF("status", S_IRUGO, pid_status), > INF("limits", S_IRUSR, pid_limits), > @@ -2421,7 +2473,7 @@ out_no_task: > static const struct pid_entry tid_base_stuff[] = { > DIR("fd",S_IRUSR|S_IXUSR, fd), > DIR("fdinfo",S_IRUSR|S_IXUSR, fdinfo), > - INF("environ", S_IRUSR, pid_environ), > + REG("environ", S_IRUSR, environ), > INF("auxv", S_IRUSR, pid_auxv), > INF("status",S_IRUGO, pid_status), > INF("limits",S_IRUSR, pid_limits), - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
On Wed, 19 Sep 2007 14:35:29 +0100 James Pearson [EMAIL PROTECTED] wrote: From: James Pearson [EMAIL PROTECTED] /proc/PID/environ currently truncates at 4096 characters, patch based on the /proc/PID/mem code. patch needs to be carefully reviewed from the security POV (ie: permissions) as well as for correctness. Does anyone have time to do that? Signed-off-by: James Pearson [EMAIL PROTECTED] --- ./fs/proc/base.c.dist 2007-09-19 12:29:46.244929651 +0100 +++ ./fs/proc/base.c 2007-09-19 12:36:18.155648760 +0100 @@ -202,27 +202,6 @@ static int proc_root_link(struct inode * (task-state == TASK_STOPPED || task-state == TASK_TRACED) \ security_ptrace(current,task) == 0)) -static int proc_pid_environ(struct task_struct *task, char * buffer) -{ - int res = 0; - struct mm_struct *mm = get_task_mm(task); - if (mm) { - unsigned int len; - - res = -ESRCH; - if (!ptrace_may_attach(task)) - goto out; - - len = mm-env_end - mm-env_start; - if (len PAGE_SIZE) - len = PAGE_SIZE; - res = access_process_vm(task, mm-env_start, buffer, len, 0); -out: - mmput(mm); - } - return res; -} - static int proc_pid_cmdline(struct task_struct *task, char * buffer) { int res = 0; @@ -740,6 +719,79 @@ static const struct file_operations proc .open = mem_open, }; +static ssize_t environ_read(struct file *file, char __user *buf, + size_t count, loff_t *ppos) +{ + struct task_struct *task = get_proc_task(file-f_dentry-d_inode); + char *page; + unsigned long src = *ppos; + int ret = -ESRCH; + struct mm_struct *mm; + size_t max_len; + + if (!task) + goto out_no_task; + + if (!ptrace_may_attach(task)) + goto out; + + ret = -ENOMEM; + page = (char *)__get_free_page(GFP_TEMPORARY); Now I wonder what inspired you to reach for GFP_TEMPORARY? Perhaps the fact that it is crappily named and undocumented. This should be GFP_KERNEL - the page you're allocating here is not reclaimable by the VM. + if (!page) + goto out; + + ret = 0; + + mm = get_task_mm(task); + if (!mm) + goto out_free; + + max_len = (count PAGE_SIZE) ? PAGE_SIZE : count; + + while (count 0) { + int this_len, retval; + + this_len = mm-env_end - (mm-env_start + src); + + if (this_len = 0) + break; + + if (this_len max_len) + this_len = max_len; + + retval = access_process_vm(task, (mm-env_start + src), + page, this_len, 0); + + if (retval = 0) { + ret = retval; + break; + } + + if (copy_to_user(buf, page, retval)) { + ret = -EFAULT; + break; + } + + ret += retval; + src += retval; + buf += retval; + count -= retval; + } Now that's a funky loop. Someone please convince me that there is no way in which `count - retval' can ever go negative (ie: huge positive). + *ppos = src; + + mmput(mm); +out_free: + free_page((unsigned long) page); +out: + put_task_struct(task); +out_no_task: + return ret; +} + +static const struct file_operations proc_environ_operations = { + .read = environ_read, +}; + static ssize_t oom_adjust_read(struct file *file, char __user *buf, size_t count, loff_t *ppos) { @@ -2092,7 +2144,7 @@ static const struct pid_entry tgid_base_ DIR(task, S_IRUGO|S_IXUGO, task), DIR(fd, S_IRUSR|S_IXUSR, fd), DIR(fdinfo, S_IRUSR|S_IXUSR, fdinfo), - INF(environ,S_IRUSR, pid_environ), + REG(environ,S_IRUSR, environ), INF(auxv, S_IRUSR, pid_auxv), INF(status, S_IRUGO, pid_status), INF(limits, S_IRUSR, pid_limits), @@ -2421,7 +2473,7 @@ out_no_task: static const struct pid_entry tid_base_stuff[] = { DIR(fd,S_IRUSR|S_IXUSR, fd), DIR(fdinfo,S_IRUSR|S_IXUSR, fdinfo), - INF(environ, S_IRUSR, pid_environ), + REG(environ, S_IRUSR, environ), INF(auxv, S_IRUSR, pid_auxv), INF(status,S_IRUGO, pid_status), INF(limits,S_IRUSR, pid_limits), - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
2007/9/19, James Pearson [EMAIL PROTECTED]: + while (count 0) { + int this_len, retval; + + this_len = mm-env_end - (mm-env_start + src); + + if (this_len = 0) + break; + + if (this_len max_len) + this_len = max_len; + + retval = access_process_vm(task, (mm-env_start + src), + page, this_len, 0); + + if (retval = 0) { + ret = retval; + break; + } + + if (copy_to_user(buf, page, retval)) { shouldn't you only copy min(count,retval) bytes? otherwise you could write beyond the users buffer buf, right? Arvin - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
On Wed, 19 Sep 2007, Mikael Pettersson wrote: > H. Peter Anvin writes: > > Mikael Pettersson wrote: > > > Does /proc/PID/mem even work? If I do `strace cat /proc/PID/mem > > /dev/null' > > > for a known good PID, the first read() from /proc/PID/mem fails with > ESRCH, > > > > Of course it does. Address zero isn't typically mapped. > > Indeed. My bad :-( No, not quite. Peter explains why "cat /proc/self/mem" gets EIO, but you were seeing "cat /proc/other/mem" get ESRCH: that's from the stringent !MAY_PTRACE || !ptrace_may_attach checks. Hugh - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
H. Peter Anvin writes: > Mikael Pettersson wrote: > > On Wed, 19 Sep 2007 14:35:29 +0100, James Pearson wrote: > >> /proc/PID/environ currently truncates at 4096 characters, patch based on > >> the /proc/PID/mem code. > > > > Does /proc/PID/mem even work? If I do `strace cat /proc/PID/mem > > > /dev/null' > > for a known good PID, the first read() from /proc/PID/mem fails with ESRCH, > > Of course it does. Address zero isn't typically mapped. Indeed. My bad :-( - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
Mikael Pettersson wrote: > On Wed, 19 Sep 2007 14:35:29 +0100, James Pearson wrote: >> /proc/PID/environ currently truncates at 4096 characters, patch based on >> the /proc/PID/mem code. > > Does /proc/PID/mem even work? If I do `strace cat /proc/PID/mem > /dev/null' > for a known good PID, the first read() from /proc/PID/mem fails with ESRCH, Of course it does. Address zero isn't typically mapped. -hpa - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
On Wed, 19 Sep 2007 14:35:29 +0100, James Pearson wrote: > /proc/PID/environ currently truncates at 4096 characters, patch based on > the /proc/PID/mem code. Does /proc/PID/mem even work? If I do `strace cat /proc/PID/mem > /dev/null' for a known good PID, the first read() from /proc/PID/mem fails with ESRCH, which is bullocks. cat /proc/PID/environ does work. I'm seeing this on 2.6.22 vanilla, 2.6.20 FC6, and 2.6.18 RHEL5 kernels. /Mikael - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
On Wed, 19 Sep 2007 14:35:29 +0100, James Pearson wrote: /proc/PID/environ currently truncates at 4096 characters, patch based on the /proc/PID/mem code. Does /proc/PID/mem even work? If I do `strace cat /proc/PID/mem /dev/null' for a known good PID, the first read() from /proc/PID/mem fails with ESRCH, which is bullocks. cat /proc/PID/environ does work. I'm seeing this on 2.6.22 vanilla, 2.6.20 FC6, and 2.6.18 RHEL5 kernels. /Mikael - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
Mikael Pettersson wrote: On Wed, 19 Sep 2007 14:35:29 +0100, James Pearson wrote: /proc/PID/environ currently truncates at 4096 characters, patch based on the /proc/PID/mem code. Does /proc/PID/mem even work? If I do `strace cat /proc/PID/mem /dev/null' for a known good PID, the first read() from /proc/PID/mem fails with ESRCH, Of course it does. Address zero isn't typically mapped. -hpa - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
H. Peter Anvin writes: Mikael Pettersson wrote: On Wed, 19 Sep 2007 14:35:29 +0100, James Pearson wrote: /proc/PID/environ currently truncates at 4096 characters, patch based on the /proc/PID/mem code. Does /proc/PID/mem even work? If I do `strace cat /proc/PID/mem /dev/null' for a known good PID, the first read() from /proc/PID/mem fails with ESRCH, Of course it does. Address zero isn't typically mapped. Indeed. My bad :-( - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH -mm] Don't truncate /proc/PID/environ at 4096 characters
On Wed, 19 Sep 2007, Mikael Pettersson wrote: H. Peter Anvin writes: Mikael Pettersson wrote: Does /proc/PID/mem even work? If I do `strace cat /proc/PID/mem /dev/null' for a known good PID, the first read() from /proc/PID/mem fails with ESRCH, Of course it does. Address zero isn't typically mapped. Indeed. My bad :-( No, not quite. Peter explains why cat /proc/self/mem gets EIO, but you were seeing cat /proc/other/mem get ESRCH: that's from the stringent !MAY_PTRACE || !ptrace_may_attach checks. Hugh - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/