Re: [PATCH] proc: avoid leaking eip, esp, or wchan tonon-privileged processes (fwd)

[Jake Edge]

On Sun, 03 May 2009 23:09:24 -0700 Eric W. Biederman wrote:
> James Morris <jmorris@xxxxxxxxx> writes:
> 
> > diff --git a/fs/proc/base.c b/fs/proc/base.c
> > index aa763ab..cd3c0d7 100644
> > --- a/fs/proc/base.c
> > +++ b/fs/proc/base.c
> > @@ -319,6 +319,9 @@ static int proc_pid_wchan(struct task_struct
> > *task, char *buffer)
> >  	unsigned long wchan;
> >  	char symname[KSYM_NAME_LEN];
> >
> > +	if (!ptrace_may_access(task, PTRACE_MODE_READ))
> > +		return 0;
> > +
> >  	wchan = get_wchan(task);
> >
> >  	if (lookup_symbol_name(wchan, symname) < 0)
> 
> If the symbol is in the kernel it should be safe to return it's
> name, all that is an information leak of a different sort.
> Overall I expect we should return  -EPERM here and not simply
> an empty file.

Ah, so if lookup_symbol_name() fails, it should return nothing or an
error?

This was "modeled" after the maps and smaps behavior, in that
they return an empty file if there is insufficient permission rather
than EPERM ... 

> Have you tested these patches against ps top and similar common
> tools?

both ps and top seem to operate just fine ... any of the default values
coming out of stat should be OK as they can be returned under other
circumstances ... the wchan file is a bit different, but it is not
clear to me what uses that rather than the wchan value in the stat file

thanks,

jake

-- 
Jake Edge - LWN - jake@xxxxxxx - http://lwn.net
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/