Messages in this thread | | | Date | Mon, 27 Jun 2011 11:58:08 +0900 | From | KOSAKI Motohiro <> | Subject | Re: [PATCH 1/2] proc: restrict access to /proc/PID/io |
| |
(2011/06/24 21:08), Vasiliy Kulikov wrote: > /proc/PID/io may be used for gathering private information. E.g. for > openssh and vsftpd daemons wchars/rchars may be used to learn the > precise password length. Restrict it to processes being able to ptrace > the target process. > > ptrace_may_access() is needed to prevent keeping open file descriptor of > "io" file, executing setuid binary and gathering io information of the > setuid'ed process. > > Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
This description seems makes sense to me. But Vasilly, I have one question. Doesn't this change break iotop command or other userland tools?
> --- > fs/proc/base.c | 7 +++++-- > 1 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/fs/proc/base.c b/fs/proc/base.c > index 14def99..5ae25d1 100644 > --- a/fs/proc/base.c > +++ b/fs/proc/base.c > @@ -2712,6 +2712,9 @@ static int do_io_accounting(struct task_struct *task, char *buffer, int whole) > struct task_io_accounting acct = task->ioac; > unsigned long flags; > > + if (!ptrace_may_access(task, PTRACE_MODE_READ)) > + return -EACCES; > +
I think this check need a comment. Usually procfs don't use ptrace_may_access() directly (see mm_for_maps) because it's racy against exec(). However I think your code is ok. because a few bytes io accounting leak has no big matter.
| |