Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 3 Oct 2013 13:56:09 +0100
From: Djalal Harouni <tixxdz@...ndz.org>
To: Ingo Molnar <mingo@...nel.org>
Cc: Andy Lutomirski <luto@...capital.net>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Kees Cook <keescook@...omium.org>,
	Al Viro <viro@...iv.linux.org.uk>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	"Serge E. Hallyn" <serge.hallyn@...ntu.com>,
	Cyrill Gorcunov <gorcunov@...nvz.org>,
	David Rientjes <rientjes@...gle.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Linux FS Devel <linux-fsdevel@...r.kernel.org>,
	kernel-hardening@...ts.openwall.com,
	Djalal Harouni <tixxdz@...il.com>
Subject: Re: [PATCH v2 0/9] procfs: protect /proc/<pid>/* files with
 file->f_cred

On Thu, Oct 03, 2013 at 08:22:56AM +0200, Ingo Molnar wrote:
> 
> * Djalal Harouni <tixxdz@...ndz.org> wrote:
> 
> >  * You can't do it for /proc/*/stat otherwise you will break userspace
> >   "ps"..., ps must access /proc/1/stat etc... so the proposed solution
> >   will work without any side effect.
> 
> The thing is, returning -EINVAL is not the only way to reject access to 
> privileged information!

> In the /proc/1/stat case a compatibility quirk can solve the problem: 
> create a special 'dummy' process inode for invalid accesses and give it to 
> ps, with all fields present but zero.
Hmm, we already return zero for the fields that must be protected.
Already done.
Not all fields need to be zero ?  If so, yes it could be done as you
propose and avoid the 'if permitted' test each time... but we don't want
to do it


> >   And for /proc/*/maps you will perhaps break glibc under certain
> >   situations... so just hold it for the moment and test it
> >   later. There have been reports in the past about it.
> 
> Same deal: just create a dummy compat-quirk maps inode with constant, zero 
> information contents to placate old user-space:
> 
>  00000000-00000000 ---p 00000000 00:00 0
> 
> [ Or whatever line is needed to minimally not break old userspace. ]
> 
> But don't leak privileged information!
> 
> ( Maybe add a CONFIG_PROC_FS_COMPAT_QUIRKS Kconfig option, default-y for 
>   now, that new/sane userspace can turn off. )
Yes, that could work, but I'm not sure (it depends on what glibc is
doing and what info it needs)

With the right permission checks, and glibc tests, this will be nice!

> Thanks,
> 
> 	Ingo

-- 
Djalal Harouni
http://opendz.org

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.