[RFC PATCH 1/2] proc: force dcache drop on unauthorized access

[RFC PATCH 1/2] proc: force dcache drop on unauthorized access

[Vasiliy Kulikov]

The patch "proc: fix races against execve() of /proc/PID/fd**" is still
a partial fix for a setxid problem.  link(2) is a yet another way to
identify whether a specific fd is opened by a privileged process.  By
calling link(2) against /proc/PID/fd/* an attacker may identify whether
the fd number is valid for PID by analysing link(2) return code.

Both getattr() and link() can be used by the attacker iff the dentry is
present in the dcache.  In this case ->lookup() is not called and the
only way to check ptrace permissions is either operation handler or
->revalidate().  The easiest solution to prevent any unauthorized access
to /proc/PID/fd*/ files is to force the dentry drop on each unauthorized
access attempt.

If an attacker keeps opened fd of /proc/PID/fd/ and dcache contains
a specific dentry for some /proc/PID/fd/XXX, any future attemp to use the
dentry by the attacker would lead to the dentry drop as a result of a
failed ptrace check in ->revalidate().  Then the attacker cannot spawn a
dentry for the specific fd number because of ptrace check in ->lookup().

The dentry drop can be still observed by an attacker by analysing
information from /proc/slabinfo, which is addressed in the successive
patch.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 fs/proc/base.c |   42 ++++++------------------------------------
 1 files changed, 6 insertions(+), 36 deletions(-)

--
diff --git a/fs/proc/base.c b/fs/proc/base.c
index d44c701..8b72293 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1665,46 +1665,12 @@ out:
 	return error;
 }
 
-static int proc_pid_fd_link_getattr(struct vfsmount *mnt, struct dentry *dentry,
-		struct kstat *stat)
-{
-	struct inode *inode = dentry->d_inode;
-	struct task_struct *task = get_proc_task(inode);
-	int rc;
-
-	if (task == NULL)
-		return -ESRCH;
-
-	rc = -EACCES;
-	if (lock_trace(task))
-		goto out_task;
-
-	generic_fillattr(inode, stat);
-	unlock_trace(task);
-	rc = 0;
-out_task:
-	put_task_struct(task);
-	return rc;
-}
-
 static const struct inode_operations proc_pid_link_inode_operations = {
 	.readlink	= proc_pid_readlink,
 	.follow_link	= proc_pid_follow_link,
 	.setattr	= proc_setattr,
 };
 
-static const struct inode_operations proc_fdinfo_link_inode_operations = {
-	.setattr	= proc_setattr,
-	.getattr	= proc_pid_fd_link_getattr,
-};
-
-static const struct inode_operations proc_fd_link_inode_operations = {
-	.readlink	= proc_pid_readlink,
-	.follow_link	= proc_pid_follow_link,
-	.setattr	= proc_setattr,
-	.getattr	= proc_pid_fd_link_getattr,
-};
-
 
 /* building an inode */
 
@@ -2013,6 +1979,11 @@ static int tid_fd_revalidate(struct dentry *dentry, struct nameidata *nd)
 	task = get_proc_task(inode);
 	fd = proc_fd(inode);
 
+	if (!ptrace_may_access(task, PTRACE_MODE_READ)) {
+		put_task_struct(task);
+		task = NULL;
+	}
+
 	if (task) {
 		files = get_files_struct(task);
 		if (files) {
@@ -2085,7 +2056,7 @@ static struct dentry *proc_fd_instantiate(struct inode *dir,
 	spin_unlock(&files->file_lock);
 	put_files_struct(files);
 
-	inode->i_op = &proc_fd_link_inode_operations;
+	inode->i_op = &proc_pid_link_inode_operations;
 	inode->i_size = 64;
 	ei->op.proc_get_link = proc_fd_link;
 	d_set_d_op(dentry, &tid_fd_dentry_operations);
@@ -2267,7 +2238,6 @@ static struct dentry *proc_fdinfo_instantiate(struct inode *dir,
 	ei->fd = fd;
 	inode->i_mode = S_IFREG | S_IRUSR;
 	inode->i_fop = &proc_fdinfo_file_operations;
-	inode->i_op = &proc_fdinfo_link_inode_operations;
 	d_set_d_op(dentry, &tid_fd_dentry_operations);
 	d_add(dentry, inode);
 	/* Close the race of the process dying before we return the dentry */

[RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Vasiliy Kulikov]

Historically /proc/slabinfo has 0444 permissions and is accessible to
the world.  slabinfo contains rather private information related both to
the kernel and userspace tasks.  Depending on the situation, it might
reveal either private information per se or information useful to make
another targeted attack.  Some examples of what can be learned by
reading/watching for /proc/slabinfo entries:

1) dentry (and different *inode*) number might reveal other processes fs
activity.  The number of dentry "active objects" doesn't strictly show
file count opened/touched by a process, however, there is a good
correlation between them.  The patch "proc: force dcache drop on
unauthorized access" relies on the privacy of dentry count.

2) different inode entries might reveal the same information as (1), but
these are more fine granted counters.  If a filesystem is mounted in a
private mount point (or even a private namespace) and fs type differs from
other mounted fs types, fs activity in this mount point/namespace is
revealed.  If there is a single ecryptfs mount point, the whole fs
activity of a single user is revealed.  Number of files in ecryptfs
mount point is a private information per se.

3) fuse_* reveals number of files / fs activity of a user in a user
private mount point.  It is approx. the same severity as ecryptfs
infoleak in (2).

4) sysfs_dir_cache similar to (2) reveals devices' addition/removal,
which can be otherwise hidden by "chmod 0700 /sys/".  With 0444 slabinfo
the precise number of sysfs files is known to the world.

5) buffer_head might reveal some kernel activity.  With other
information leaks an attacker might identify what specific kernel
routines generate buffer_head activity.

6) *kmalloc* infoleaks are very situational.  Attacker should watch for
the specific kmalloc size entry and filter the noise related to the unrelated
kernel activity.  If an attacker has relatively silent victim system, he
might get rather precise counters.

Additional information sources might significantly increase the slabinfo
infoleak benefits.  E.g. if an attacker knows that the processes
activity on the system is very low (only core daemons like syslog and
cron), he may run setxid binaries / trigger local daemon activity /
trigger network services activity / await sporadic cron jobs activity
/ etc. and get rather precise counters for fs and network activity of
these privileged tasks, which is unknown otherwise.


Also hiding slabinfo is a one step to complicate exploitation of kernel
heap overflows (and possibly, other bugs).  The related discussion:

http://thread.gmane.org/gmane.linux.kernel/1108378


World readable slabinfo simplifies kernel developers' job of debugging
kernel bugs (e.g. memleaks), but I believe it does more harm than
benefits.  For most users 0444 slabinfo is an unreasonable attack vector.

Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 mm/slab.c |    3 ++-
 mm/slub.c |    2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

--
diff --git a/mm/slab.c b/mm/slab.c
index 6d90a09..560ffd0 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -4584,7 +4584,8 @@ static const struct file_operations proc_slabstats_operations = {
 
 static int __init slab_proc_init(void)
 {
-	proc_create("slabinfo",S_IWUSR|S_IRUGO,NULL,&proc_slabinfo_operations);
+	proc_create("slabinfo", S_IWUSR | S_IRUSR, NULL,
+		    &proc_slabinfo_operations);
 #ifdef CONFIG_DEBUG_SLAB_LEAK
 	proc_create("slab_allocators", 0, NULL, &proc_slabstats_operations);
 #endif
diff --git a/mm/slub.c b/mm/slub.c
index 9f662d7..f440fc7 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -5257,7 +5257,7 @@ static const struct file_operations proc_slabinfo_operations = {
 
 static int __init slab_proc_init(void)
 {
-	proc_create("slabinfo", S_IRUGO, NULL, &proc_slabinfo_operations);
+	proc_create("slabinfo", S_IRUSR, NULL, &proc_slabinfo_operations);
 	return 0;
 }
 module_init(slab_proc_init);

Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Cyrill Gorcunov]

On Sat, Sep 10, 2011 at 08:41:34PM +0400, Vasiliy Kulikov wrote:
> Historically /proc/slabinfo has 0444 permissions and is accessible to
> the world.  slabinfo contains rather private information related both to
> the kernel and userspace tasks.  Depending on the situation, it might
> reveal either private information per se or information useful to make
> another targeted attack.  Some examples of what can be learned by
> reading/watching for /proc/slabinfo entries:
> 
...

Since this file is controversy point, probably its permissions might be
configurable via setup option?

	Cyrill

Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Vasiliy Kulikov]

On Mon, Sep 12, 2011 at 19:06 +0400, Cyrill Gorcunov wrote:
> On Sat, Sep 10, 2011 at 08:41:34PM +0400, Vasiliy Kulikov wrote:
> > Historically /proc/slabinfo has 0444 permissions and is accessible to
> > the world.  slabinfo contains rather private information related both to
> > the kernel and userspace tasks.  Depending on the situation, it might
> > reveal either private information per se or information useful to make
> > another targeted attack.  Some examples of what can be learned by
> > reading/watching for /proc/slabinfo entries:
> > 
> ...
> 
> Since this file is controversy point, probably its permissions might be
> configurable via setup option?

We could create DEBUG_PROC (of cource, defaults to =n) to keep current
relaxed procfs permissions, but I don't think a configure option is needed
for a thing which is trivially done via "chmod" in init scripts.

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Vasiliy Kulikov]

(cc'ed all members of the previous discussion as currently lists might
not work as they should)

On Sat, Sep 10, 2011 at 20:41 +0400, Vasiliy Kulikov wrote:
> Historically /proc/slabinfo has 0444 permissions and is accessible to
> the world.  slabinfo contains rather private information related both to
> the kernel and userspace tasks.  Depending on the situation, it might
> reveal either private information per se or information useful to make
> another targeted attack.  Some examples of what can be learned by
> reading/watching for /proc/slabinfo entries:
> 
> 1) dentry (and different *inode*) number might reveal other processes fs
> activity.  The number of dentry "active objects" doesn't strictly show
> file count opened/touched by a process, however, there is a good
> correlation between them.  The patch "proc: force dcache drop on
> unauthorized access" relies on the privacy of dentry count.
> 
> 2) different inode entries might reveal the same information as (1), but
> these are more fine granted counters.  If a filesystem is mounted in a
> private mount point (or even a private namespace) and fs type differs from
> other mounted fs types, fs activity in this mount point/namespace is
> revealed.  If there is a single ecryptfs mount point, the whole fs
> activity of a single user is revealed.  Number of files in ecryptfs
> mount point is a private information per se.
> 
> 3) fuse_* reveals number of files / fs activity of a user in a user
> private mount point.  It is approx. the same severity as ecryptfs
> infoleak in (2).
> 
> 4) sysfs_dir_cache similar to (2) reveals devices' addition/removal,
> which can be otherwise hidden by "chmod 0700 /sys/".  With 0444 slabinfo
> the precise number of sysfs files is known to the world.
> 
> 5) buffer_head might reveal some kernel activity.  With other
> information leaks an attacker might identify what specific kernel
> routines generate buffer_head activity.
> 
> 6) *kmalloc* infoleaks are very situational.  Attacker should watch for
> the specific kmalloc size entry and filter the noise related to the unrelated
> kernel activity.  If an attacker has relatively silent victim system, he
> might get rather precise counters.
> 
> Additional information sources might significantly increase the slabinfo
> infoleak benefits.  E.g. if an attacker knows that the processes
> activity on the system is very low (only core daemons like syslog and
> cron), he may run setxid binaries / trigger local daemon activity /
> trigger network services activity / await sporadic cron jobs activity
> / etc. and get rather precise counters for fs and network activity of
> these privileged tasks, which is unknown otherwise.
> 
> 
> Also hiding slabinfo is a one step to complicate exploitation of kernel
> heap overflows (and possibly, other bugs).  The related discussion:
> 
> http://thread.gmane.org/gmane.linux.kernel/1108378
> 
> 
> World readable slabinfo simplifies kernel developers' job of debugging
> kernel bugs (e.g. memleaks), but I believe it does more harm than
> benefits.  For most users 0444 slabinfo is an unreasonable attack vector.

Please tell if anybody has complains about the restriction - whether it
forces someone besides kernel developers to do "chmod/chgrp".  But if
someone want to debug the kernel, it shouldn't significantly influence
on common users, especially it shouldn't create security issues.

Thanks!

> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
> ---
>  mm/slab.c |    3 ++-
>  mm/slub.c |    2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> --
> diff --git a/mm/slab.c b/mm/slab.c
> index 6d90a09..560ffd0 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -4584,7 +4584,8 @@ static const struct file_operations proc_slabstats_operations = {
>  
>  static int __init slab_proc_init(void)
>  {
> -	proc_create("slabinfo",S_IWUSR|S_IRUGO,NULL,&proc_slabinfo_operations);
> +	proc_create("slabinfo", S_IWUSR | S_IRUSR, NULL,
> +		    &proc_slabinfo_operations);
>  #ifdef CONFIG_DEBUG_SLAB_LEAK
>  	proc_create("slab_allocators", 0, NULL, &proc_slabstats_operations);
>  #endif
> diff --git a/mm/slub.c b/mm/slub.c
> index 9f662d7..f440fc7 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -5257,7 +5257,7 @@ static const struct file_operations proc_slabinfo_operations = {
>  
>  static int __init slab_proc_init(void)
>  {
> -	proc_create("slabinfo", S_IRUGO, NULL, &proc_slabinfo_operations);
> +	proc_create("slabinfo", S_IRUSR, NULL, &proc_slabinfo_operations);
>  	return 0;
>  }
>  module_init(slab_proc_init);

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Dave Hansen]

On Wed, 2011-09-14 at 17:16 +0400, Vasiliy Kulikov wrote:
> > World readable slabinfo simplifies kernel developers' job of debugging
> > kernel bugs (e.g. memleaks), but I believe it does more harm than
> > benefits.  For most users 0444 slabinfo is an unreasonable attack vector.
> 
> Please tell if anybody has complains about the restriction - whether it
> forces someone besides kernel developers to do "chmod/chgrp".  But if
> someone want to debug the kernel, it shouldn't significantly influence
> on common users, especially it shouldn't create security issues. 

Ubuntu ships today with a /etc/init/mounted-proc.conf that does:

	chmod 0400 "${MOUNTPOINT}"/slabinfo

After cursing Kees's name a few times, I commented it out and it hasn't
bothered me again.  

I expect that the folks that really care about this (and their distros)
will probably have a similar mechanism.  I guess the sword cuts both
ways in this case: it obviously _works_ to have the distros do it, but
it was a one-time inconvenience for me to override that.

In other words, I dunno.  If we do this in the kernel, can we at least
do something like CONFIG_INSECURE to both track these kinds of things
and make it easy to get them out of a developer's way?

-- Dave

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Vasiliy Kulikov]

Hi Dave,

On Wed, Sep 14, 2011 at 08:18 -0700, Dave Hansen wrote:
> On Wed, 2011-09-14 at 17:16 +0400, Vasiliy Kulikov wrote:
> > > World readable slabinfo simplifies kernel developers' job of debugging
> > > kernel bugs (e.g. memleaks), but I believe it does more harm than
> > > benefits.  For most users 0444 slabinfo is an unreasonable attack vector.
> > 
> > Please tell if anybody has complains about the restriction - whether it
> > forces someone besides kernel developers to do "chmod/chgrp".  But if
> > someone want to debug the kernel, it shouldn't significantly influence
> > on common users, especially it shouldn't create security issues. 
> 
> Ubuntu ships today with a /etc/init/mounted-proc.conf that does:
> 
> 	chmod 0400 "${MOUNTPOINT}"/slabinfo
> 
> After cursing Kees's name a few times, I commented it out and it hasn't
> bothered me again.  

Another way is chgrp slabinfo to some "admin" group which are privileged
in this sense and add your user to this group.  But please, sane and
secure defaults!

> I expect that the folks that really care about this (and their distros)
> will probably have a similar mechanism.  I guess the sword cuts both
> ways in this case: it obviously _works_ to have the distros do it, but
> it was a one-time inconvenience for me to override that.
> 
> In other words, I dunno.  If we do this in the kernel, can we at least
> do something like CONFIG_INSECURE to both track these kinds of things
> and make it easy to get them out of a developer's way?

What do you think about adding your user to the slabinfo's group or
chmod it - quite the opposite Ubuntu currently does?  I think it is more
generic (e.g. you may chmod 0444 to allow all users to get debug
information or just 0440 and chgrp admin to allow only trusted users to
do it) and your local policy doesn't touch the kernel.

Thanks,

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Vasiliy Kulikov]

(cc'ed Dave back, sorry for the noise)

On Wed, Sep 14, 2011 at 19:42 +0400, Vasiliy Kulikov wrote:
> Hi Dave,
> 
> On Wed, Sep 14, 2011 at 08:18 -0700, Dave Hansen wrote:
> > On Wed, 2011-09-14 at 17:16 +0400, Vasiliy Kulikov wrote:
> > > > World readable slabinfo simplifies kernel developers' job of debugging
> > > > kernel bugs (e.g. memleaks), but I believe it does more harm than
> > > > benefits.  For most users 0444 slabinfo is an unreasonable attack vector.
> > > 
> > > Please tell if anybody has complains about the restriction - whether it
> > > forces someone besides kernel developers to do "chmod/chgrp".  But if
> > > someone want to debug the kernel, it shouldn't significantly influence
> > > on common users, especially it shouldn't create security issues. 
> > 
> > Ubuntu ships today with a /etc/init/mounted-proc.conf that does:
> > 
> > 	chmod 0400 "${MOUNTPOINT}"/slabinfo
> > 
> > After cursing Kees's name a few times, I commented it out and it hasn't
> > bothered me again.  
> 
> Another way is chgrp slabinfo to some "admin" group which are privileged
> in this sense and add your user to this group.  But please, sane and
> secure defaults!
> 
> > I expect that the folks that really care about this (and their distros)
> > will probably have a similar mechanism.  I guess the sword cuts both
> > ways in this case: it obviously _works_ to have the distros do it, but
> > it was a one-time inconvenience for me to override that.
> > 
> > In other words, I dunno.  If we do this in the kernel, can we at least
> > do something like CONFIG_INSECURE to both track these kinds of things
> > and make it easy to get them out of a developer's way?
> 
> What do you think about adding your user to the slabinfo's group or
> chmod it - quite the opposite Ubuntu currently does?  I think it is more
> generic (e.g. you may chmod 0444 to allow all users to get debug
> information or just 0440 and chgrp admin to allow only trusted users to
> do it) and your local policy doesn't touch the kernel.
> 
> Thanks,
> 
> -- 
> Vasiliy Kulikov
> http://www.openwall.com - bringing security into open computing environments

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Dave Hansen]

On Wed, 2011-09-14 at 19:42 +0400, Vasiliy Kulikov wrote:
> > In other words, I dunno.  If we do this in the kernel, can we at least
> > do something like CONFIG_INSECURE to both track these kinds of things
> > and make it easy to get them out of a developer's way?
> 
> What do you think about adding your user to the slabinfo's group or
> chmod it - quite the opposite Ubuntu currently does?  I think it is more
> generic (e.g. you may chmod 0444 to allow all users to get debug
> information or just 0440 and chgrp admin to allow only trusted users to
> do it) and your local policy doesn't touch the kernel.

That obviously _works_.  I'd be happy to ack your patch.  As I said,
it's pretty minimally painful, even to folks who care about slabinfo
like me.

-- Dave

Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Dave Hansen]

On Sat, 2011-09-10 at 20:41 +0400, Vasiliy Kulikov wrote:
> @@ -4584,7 +4584,8 @@ static const struct file_operations proc_slabstats_operations = {
>  
>  static int __init slab_proc_init(void)
>  {
> -       proc_create("slabinfo",S_IWUSR|S_IRUGO,NULL,&proc_slabinfo_operations);
> +       proc_create("slabinfo", S_IWUSR | S_IRUSR, NULL,
> +                   &proc_slabinfo_operations);
>  #ifdef CONFIG_DEBUG_SLAB_LEAK
>         proc_create("slab_allocators", 0, NULL, &proc_sla 

If you respin this, please don't muck with the whitespace.  Otherwise,
I'm fine with this.  Distros are already starting to do this anyway in
userspace.

Reviewed-by: Dave Hansen <dave@linux.vnet.ibm.com>

-- Dave

Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Vasiliy Kulikov]

On Wed, Sep 14, 2011 at 11:41 -0700, Dave Hansen wrote:
> On Sat, 2011-09-10 at 20:41 +0400, Vasiliy Kulikov wrote:
> > @@ -4584,7 +4584,8 @@ static const struct file_operations proc_slabstats_operations = {
> >  
> >  static int __init slab_proc_init(void)
> >  {
> > -       proc_create("slabinfo",S_IWUSR|S_IRUGO,NULL,&proc_slabinfo_operations);
> > +       proc_create("slabinfo", S_IWUSR | S_IRUSR, NULL,
> > +                   &proc_slabinfo_operations);
> >  #ifdef CONFIG_DEBUG_SLAB_LEAK
> >         proc_create("slab_allocators", 0, NULL, &proc_sla 
> 
> If you respin this, please don't muck with the whitespace.

OK, I was just removing checkpatch warnings.

>  Otherwise,
> I'm fine with this.  Distros are already starting to do this anyway in
> userspace.
> 
> Reviewed-by: Dave Hansen <dave@linux.vnet.ibm.com>

Thank you!

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Kees Cook]

Hi Vasiliy,

On Sat, Sep 10, 2011 at 08:41:34PM +0400, Vasiliy Kulikov wrote:
> Historically /proc/slabinfo has 0444 permissions and is accessible to
> the world.  slabinfo contains rather private information related both to
> the kernel and userspace tasks.  Depending on the situation, it might
> reveal either private information per se or information useful to make
> another targeted attack.  Some examples of what can be learned by
> reading/watching for /proc/slabinfo entries:
> ...
> World readable slabinfo simplifies kernel developers' job of debugging
> kernel bugs (e.g. memleaks), but I believe it does more harm than
> benefits.  For most users 0444 slabinfo is an unreasonable attack vector.
> 
> Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>

Haven't had any mass complaints about the 0400 in Ubuntu (sorry Dave!), so
I'm obviously for it.

Reviewed-by: Kees Cook <kees@ubuntu.com>

-Kees

-- 
Kees Cook
Ubuntu Security Team

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Vasiliy Kulikov]

Hi Andrew,

On Wed, Sep 14, 2011 at 12:27 -0700, Kees Cook wrote:
> On Sat, Sep 10, 2011 at 08:41:34PM +0400, Vasiliy Kulikov wrote:
> > Historically /proc/slabinfo has 0444 permissions and is accessible to
> > the world.  slabinfo contains rather private information related both to
> > the kernel and userspace tasks.  Depending on the situation, it might
> > reveal either private information per se or information useful to make
> > another targeted attack.  Some examples of what can be learned by
> > reading/watching for /proc/slabinfo entries:
> > ...
> > World readable slabinfo simplifies kernel developers' job of debugging
> > kernel bugs (e.g. memleaks), but I believe it does more harm than
> > benefits.  For most users 0444 slabinfo is an unreasonable attack vector.
> > 
> > Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
> 
> Haven't had any mass complaints about the 0400 in Ubuntu (sorry Dave!), so
> I'm obviously for it.
> 
> Reviewed-by: Kees Cook <kees@ubuntu.com>

Looks like the members of the previous slabinfo discussion don't object
against the patch now and it got two other Reviewed-by responses.  Can
you merge it as-is or should I probably convince someone else?

Thanks,

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Christoph Lameter]

On Sun, 18 Sep 2011, Vasiliy Kulikov wrote:

> > Reviewed-by: Kees Cook <kees@ubuntu.com>
>
> Looks like the members of the previous slabinfo discussion don't object
> against the patch now and it got two other Reviewed-by responses.  Can
> you merge it as-is or should I probably convince someone else?

The patch is incomplete. If you want to restrict access to this
information then /sys/kernel/slab needs to have a similar
restriction.

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Pekka Enberg]

> On Wed, Sep 14, 2011 at 12:27 -0700, Kees Cook wrote:
>> On Sat, Sep 10, 2011 at 08:41:34PM +0400, Vasiliy Kulikov wrote:
>> > Historically /proc/slabinfo has 0444 permissions and is accessible to
>> > the world.  slabinfo contains rather private information related both to
>> > the kernel and userspace tasks.  Depending on the situation, it might
>> > reveal either private information per se or information useful to make
>> > another targeted attack.  Some examples of what can be learned by
>> > reading/watching for /proc/slabinfo entries:
>> > ...
>> > World readable slabinfo simplifies kernel developers' job of debugging
>> > kernel bugs (e.g. memleaks), but I believe it does more harm than
>> > benefits.  For most users 0444 slabinfo is an unreasonable attack vector.
>> >
>> > Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>

On Sun, Sep 18, 2011 at 8:05 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
>> Haven't had any mass complaints about the 0400 in Ubuntu (sorry Dave!), so
>> I'm obviously for it.
>>
>> Reviewed-by: Kees Cook <kees@ubuntu.com>
>
> Looks like the members of the previous slabinfo discussion don't object
> against the patch now and it got two other Reviewed-by responses.  Can
> you merge it as-is or should I probably convince someone else?

We discussed this in March (google for 'Make /proc/slabinfo 0400') and
concluded that it's not worth it doesn't really protect from anything
and causes harm to developers.

                        Pekka

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Vasiliy Kulikov]

Hello Pekka,

On Mon, Sep 19, 2011 at 17:30 +0300, Pekka Enberg wrote:
> > On Wed, Sep 14, 2011 at 12:27 -0700, Kees Cook wrote:
> >> On Sat, Sep 10, 2011 at 08:41:34PM +0400, Vasiliy Kulikov wrote:
> >> > Historically /proc/slabinfo has 0444 permissions and is accessible to
> >> > the world.  slabinfo contains rather private information related both to
> >> > the kernel and userspace tasks.  Depending on the situation, it might
> >> > reveal either private information per se or information useful to make
> >> > another targeted attack.  Some examples of what can be learned by
> >> > reading/watching for /proc/slabinfo entries:
> >> > ...
> >> > World readable slabinfo simplifies kernel developers' job of debugging
> >> > kernel bugs (e.g. memleaks), but I believe it does more harm than
> >> > benefits.  For most users 0444 slabinfo is an unreasonable attack vector.
> >> >
> >> > Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
> 
> On Sun, Sep 18, 2011 at 8:05 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
> >> Haven't had any mass complaints about the 0400 in Ubuntu (sorry Dave!), so
> >> I'm obviously for it.
> >>
> >> Reviewed-by: Kees Cook <kees@ubuntu.com>
> >
> > Looks like the members of the previous slabinfo discussion don't object
> > against the patch now and it got two other Reviewed-by responses.  Can
> > you merge it as-is or should I probably convince someone else?
> 
> We discussed this in March (google for 'Make /proc/slabinfo 0400')

Sure, I've read it and included the link in the patch description :)

> and
> concluded that it's not worth it doesn't really protect from anything

Closing only slabinfo doesn't add any significant protection against
kernel heap exploits per se, no objections here.  

But as said in the desciption, the reason for this patch is not protecting
against exploitation heap bugs.  It is a source of infoleaks of kernel
and userspace activity, which should be forbidden to non-root users.

> and causes harm to developers.

One note: only to _kernel_ developers.  It means it is a strictly
debugging feature, which shouldn't be enabled in the production systems.

Thanks,

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Pekka Enberg]

Hi Vasiliy,

On Mon, Sep 19, 2011 at 5:46 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
>> and
>> concluded that it's not worth it doesn't really protect from anything
>
> Closing only slabinfo doesn't add any significant protection against
> kernel heap exploits per se, no objections here.
>
> But as said in the desciption, the reason for this patch is not protecting
> against exploitation heap bugs.  It is a source of infoleaks of kernel
> and userspace activity, which should be forbidden to non-root users.

Last time we discussed this, the 'extra protection' didn't seem to be
significant enough to justify disabling an useful kernel debugging
interface by default.

What's different about the patch now?

>> and causes harm to developers.
>
> One note: only to _kernel_ developers.  It means it is a strictly
> debugging feature, which shouldn't be enabled in the production systems.

It's pretty much _the_ interface for debugging kernel memory leaks in
production systems and we ask users for it along with /proc/meminfo
when debugging many memory management related issues. When we
temporarily dropped /proc/slabinfo with the introduction of SLUB, people
complained pretty loudly.

I'd be willing to consider this patch if it's a config option that's not enabled
by default; otherwise you need to find someone else to merge the patch.
You can add some nasty warnings to the Kconfig text to scare the users
into enabling it. ;-)

                         Pekka

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Vasiliy Kulikov]

On Mon, Sep 19, 2011 at 18:13 +0300, Pekka Enberg wrote:
> On Mon, Sep 19, 2011 at 5:46 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
> >> and
> >> concluded that it's not worth it doesn't really protect from anything
> >
> > Closing only slabinfo doesn't add any significant protection against
> > kernel heap exploits per se, no objections here.
> >
> > But as said in the desciption, the reason for this patch is not protecting
> > against exploitation heap bugs.  It is a source of infoleaks of kernel
> > and userspace activity, which should be forbidden to non-root users.
> 
> Last time we discussed this, the 'extra protection' didn't seem to be
> significant enough to justify disabling an useful kernel debugging
> interface by default.
> 
> What's different about the patch now?

The exploitation you're talking about is an exploitation of kernel heap
bugs.  Dan's previous "make slabinfo 0400" patch tried to complicate
attacker's life by hiding information about how many free object are
left in the slab.  With this information an attacker may compute how he
should spray the slab to position slab object to increase his chances of
overwriting specific memory areas - pointers, etc.

I don't speak about how much/whether closing slabinfo complicates this
task, though.  My idea is orthogonal to the Dan's idea.  I claim that
with 0444 slabinfo any user may get information about in-system activity
that he shouldn't learn.  In short, one may learn precisely when other
user reads directory contents, opens files, how much files there are in
the specific _private_ directory, how much files _private_ ecryptfs or
fuse mount point contains, etc.  This breaks user's assumption that
the number of files in a private directory is a private information.
There are a bit more thoughts in the patch description.


> >> and causes harm to developers.
> >
> > One note: only to _kernel_ developers.  It means it is a strictly
> > debugging feature, which shouldn't be enabled in the production systems.
> 
> It's pretty much _the_ interface for debugging kernel memory leaks in
> production systems and we ask users for it along with /proc/meminfo
> when debugging many memory management related issues. When we
> temporarily dropped /proc/slabinfo with the introduction of SLUB, people
> complained pretty loudly.

Could you point to the discussion, please?  I cannot find the patch for
0400 slabinfo even in the linux-history repository.


> I'd be willing to consider this patch if it's a config option that's not enabled
> by default; otherwise you need to find someone else to merge the patch.
> You can add some nasty warnings to the Kconfig text to scare the users
> into enabling it. ;-)

How do you see this CONFIG_ option?  CONFIG_PROCFS_COMPAT_MODES (or _PERMS),
defaults to Y?  If we find more procfs files with dangerous permissions,
we may move it under "ifndef CONFIG_PROCFS_COMPAT_PERMS".

Thanks,

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Pekka Enberg]

On Mon, Sep 19, 2011 at 6:57 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
> On Mon, Sep 19, 2011 at 18:13 +0300, Pekka Enberg wrote:
>> On Mon, Sep 19, 2011 at 5:46 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
>> >> and
>> >> concluded that it's not worth it doesn't really protect from anything
>> >
>> > Closing only slabinfo doesn't add any significant protection against
>> > kernel heap exploits per se, no objections here.
>> >
>> > But as said in the desciption, the reason for this patch is not protecting
>> > against exploitation heap bugs.  It is a source of infoleaks of kernel
>> > and userspace activity, which should be forbidden to non-root users.
>>
>> Last time we discussed this, the 'extra protection' didn't seem to be
>> significant enough to justify disabling an useful kernel debugging
>> interface by default.
>>
>> What's different about the patch now?
>
> The exploitation you're talking about is an exploitation of kernel heap
> bugs.  Dan's previous "make slabinfo 0400" patch tried to complicate
> attacker's life by hiding information about how many free object are
> left in the slab.  With this information an attacker may compute how he
> should spray the slab to position slab object to increase his chances of
> overwriting specific memory areas - pointers, etc.
>
> I don't speak about how much/whether closing slabinfo complicates this
> task, though.  My idea is orthogonal to the Dan's idea.  I claim that
> with 0444 slabinfo any user may get information about in-system activity
> that he shouldn't learn.  In short, one may learn precisely when other
> user reads directory contents, opens files, how much files there are in
> the specific _private_ directory, how much files _private_ ecryptfs or
> fuse mount point contains, etc.  This breaks user's assumption that
> the number of files in a private directory is a private information.
> There are a bit more thoughts in the patch description.

Yes, I read your patch description and I think it's convincing enough
to warrant a config option but not changing the default.

However, if the encryptfs and infoleaks really are serious enough to
hide /proc/slabinfo, I think you should consider switching over to
kmalloc() instead of kmem_cache_alloc() to make sure nobody can
gain access to the information. Or add a SLAB_ANON flag for
kmem_cache_create() that forces SLUB cache merging to existing
caches.

>> >> and causes harm to developers.
>> >
>> > One note: only to _kernel_ developers.  It means it is a strictly
>> > debugging feature, which shouldn't be enabled in the production systems.
>>
>> It's pretty much _the_ interface for debugging kernel memory leaks in
>> production systems and we ask users for it along with /proc/meminfo
>> when debugging many memory management related issues. When we
>> temporarily dropped /proc/slabinfo with the introduction of SLUB, people
>> complained pretty loudly.
>
> Could you point to the discussion, please?  I cannot find the patch for
> 0400 slabinfo even in the linux-history repository.

We dropped the whole file for SLUB:

http://lwn.net/Articles/263337/

[ I didn't find the original discussion that motivated the above
  patch but it should be somewhere in LKML archives around
  that time. ]

Making it root-only will have pretty much the same kind of
out-of-the-box behavior.

>> I'd be willing to consider this patch if it's a config option that's not enabled
>> by default; otherwise you need to find someone else to merge the patch.
>> You can add some nasty warnings to the Kconfig text to scare the users
>> into enabling it. ;-)
>
> How do you see this CONFIG_ option?  CONFIG_PROCFS_COMPAT_MODES (or _PERMS),
> defaults to Y?  If we find more procfs files with dangerous permissions,
> we may move it under "ifndef CONFIG_PROCFS_COMPAT_PERMS".

I guess CONFIG_RESTRICT_PROCFS type of thing makes most sense
since the problem is not only about SLAB. If you want to make it slab-only
config option, I'm fine with that too.

Please note that you need to restrict sysfs files for SLUB as well.

                         Pekka

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Vasiliy Kulikov]

On Mon, Sep 19, 2011 at 19:11 +0300, Pekka Enberg wrote:
> >> What's different about the patch now?
> >
> > The exploitation you're talking about is an exploitation of kernel heap
> > bugs.  Dan's previous "make slabinfo 0400" patch tried to complicate
> > attacker's life by hiding information about how many free object are
> > left in the slab.  With this information an attacker may compute how he
> > should spray the slab to position slab object to increase his chances of
> > overwriting specific memory areas - pointers, etc.
> >
> > I don't speak about how much/whether closing slabinfo complicates this
> > task, though.  My idea is orthogonal to the Dan's idea.  I claim that
> > with 0444 slabinfo any user may get information about in-system activity
> > that he shouldn't learn.  In short, one may learn precisely when other
> > user reads directory contents, opens files, how much files there are in
> > the specific _private_ directory, how much files _private_ ecryptfs or
> > fuse mount point contains, etc.  This breaks user's assumption that
> > the number of files in a private directory is a private information.
> > There are a bit more thoughts in the patch description.
> 
> Yes, I read your patch description and I think it's convincing enough
> to warrant a config option but not changing the default.
> 
> However, if the encryptfs and infoleaks really are serious enough to
> hide /proc/slabinfo, I think you should consider switching over to
> kmalloc() instead of kmem_cache_alloc() to make sure nobody can
> gain access to the information.

kmalloc() is still visible in slabinfo as kmalloc-128 or so.


> >> > One note: only to _kernel_ developers.  It means it is a strictly
> >> > debugging feature, which shouldn't be enabled in the production systems.
> >>
> >> It's pretty much _the_ interface for debugging kernel memory leaks in
> >> production systems and we ask users for it along with /proc/meminfo
> >> when debugging many memory management related issues. When we
> >> temporarily dropped /proc/slabinfo with the introduction of SLUB, people
> >> complained pretty loudly.
> >
> > Could you point to the discussion, please?  I cannot find the patch for
> > 0400 slabinfo even in the linux-history repository.
> 
> We dropped the whole file for SLUB:
> 
> http://lwn.net/Articles/263337/

Ah, I've misunderstood you.


> [ I didn't find the original discussion that motivated the above
>   patch but it should be somewhere in LKML archives around
>   that time. ]
> 
> Making it root-only will have pretty much the same kind of
> out-of-the-box behavior.
> 
> >> I'd be willing to consider this patch if it's a config option that's not enabled
> >> by default; otherwise you need to find someone else to merge the patch.
> >> You can add some nasty warnings to the Kconfig text to scare the users
> >> into enabling it. ;-)
> >
> > How do you see this CONFIG_ option?  CONFIG_PROCFS_COMPAT_MODES (or _PERMS),
> > defaults to Y?  If we find more procfs files with dangerous permissions,
> > we may move it under "ifndef CONFIG_PROCFS_COMPAT_PERMS".
> 
> I guess CONFIG_RESTRICT_PROCFS type of thing makes most sense
> since the problem is not only about SLAB. If you want to make it slab-only
> config option, I'm fine with that too.

OK, then I'll prepare a patch with a configure option, if no other
objections.

> Please note that you need to restrict sysfs files for SLUB as well.

Sure.

Thank you for the comments!

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Pekka Enberg]

On Mon, Sep 19, 2011 at 7:18 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
>> However, if the encryptfs and infoleaks really are serious enough to
>> hide /proc/slabinfo, I think you should consider switching over to
>> kmalloc() instead of kmem_cache_alloc() to make sure nobody can
>> gain access to the information.
>
> kmalloc() is still visible in slabinfo as kmalloc-128 or so.

Yes, but there's no way for users to know where the allocations came from
if you mix them up with other kmalloc-128 call-sites. That way the number
of private files will stay private to the user, no? Doesn't that give you even
better protection against the infoleak?

                                Pekka

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Vasiliy Kulikov]

On Mon, Sep 19, 2011 at 20:31 +0300, Pekka Enberg wrote:
> On Mon, Sep 19, 2011 at 7:18 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
> >> However, if the encryptfs and infoleaks really are serious enough to
> >> hide /proc/slabinfo, I think you should consider switching over to
> >> kmalloc() instead of kmem_cache_alloc() to make sure nobody can
> >> gain access to the information.
> >
> > kmalloc() is still visible in slabinfo as kmalloc-128 or so.
> 
> Yes, but there's no way for users to know where the allocations came from
> if you mix them up with other kmalloc-128 call-sites. That way the number
> of private files will stay private to the user, no? Doesn't that give you even
> better protection against the infoleak?

No, what it gives us is an obscurity, not a protection.  I'm sure it
highly depends on the specific situation whether an attacker is able to
identify whether the call is from e.g. ecryptfs or from VFS.  Also the
correlation between the number in slabinfo and the real private actions
still exists.

Thanks,

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Christoph Lameter]

On Mon, 19 Sep 2011, Vasiliy Kulikov wrote:

> > > kmalloc() is still visible in slabinfo as kmalloc-128 or so.
> >
> > Yes, but there's no way for users to know where the allocations came from
> > if you mix them up with other kmalloc-128 call-sites. That way the number
> > of private files will stay private to the user, no? Doesn't that give you even
> > better protection against the infoleak?
>
> No, what it gives us is an obscurity, not a protection.  I'm sure it
> highly depends on the specific situation whether an attacker is able to
> identify whether the call is from e.g. ecryptfs or from VFS.  Also the
> correlation between the number in slabinfo and the real private actions
> still exists.

IMHO a restriction of access to slab statistics is reasonable in a
hardened environment. Make it dependent on CONFIG_SECURITY or some such
thing?

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Pekka Enberg]

On Mon, Sep 19, 2011 at 8:35 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
>> Yes, but there's no way for users to know where the allocations came from
>> if you mix them up with other kmalloc-128 call-sites. That way the number
>> of private files will stay private to the user, no? Doesn't that give you even
>> better protection against the infoleak?
>
> No, what it gives us is an obscurity, not a protection.  I'm sure it
> highly depends on the specific situation whether an attacker is able to
> identify whether the call is from e.g. ecryptfs or from VFS.  Also the
> correlation between the number in slabinfo and the real private actions
> still exists.

How is the attacker able to identify that we kmalloc()'d from ecryptfs or
VFS based on non-root /proc/slabinfo when the slab allocator itself does
not have that sort of information if you mix up the allocations? Isn't this
much stronger protection especially if you combine that with /proc/slabinfo
restriction?

                                  Pekka

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Vasiliy Kulikov]

On Mon, Sep 19, 2011 at 20:51 +0300, Pekka Enberg wrote:
> On Mon, Sep 19, 2011 at 8:35 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
> >> Yes, but there's no way for users to know where the allocations came from
> >> if you mix them up with other kmalloc-128 call-sites. That way the number
> >> of private files will stay private to the user, no? Doesn't that give you even
> >> better protection against the infoleak?
> >
> > No, what it gives us is an obscurity, not a protection.  I'm sure it
> > highly depends on the specific situation whether an attacker is able to
> > identify whether the call is from e.g. ecryptfs or from VFS.  Also the
> > correlation between the number in slabinfo and the real private actions
> > still exists.
> 
> How is the attacker able to identify that we kmalloc()'d from ecryptfs or
> VFS based on non-root /proc/slabinfo when the slab allocator itself does
> not have that sort of information if you mix up the allocations?

How can you _guarantee_ that they mix?

> Isn't this
> much stronger protection especially if you combine that with /proc/slabinfo
> restriction?

I don't see any reason to change allocators if we close slabinfo.

Thanks,

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Dave Hansen]

On Mon, 2011-09-19 at 20:51 +0300, Pekka Enberg wrote:
> How is the attacker able to identify that we kmalloc()'d from ecryptfs or
> VFS based on non-root /proc/slabinfo when the slab allocator itself does
> not have that sort of information if you mix up the allocations? Isn't this
> much stronger protection especially if you combine that with /proc/slabinfo
> restriction? 

Mixing it up just adds noise.  It makes the attack somewhat more
difficult, but it still leaves open the possibility that the attacker
can filter out the noise somehow.

-- Dave

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Pekka Enberg]

On Mon, 2011-09-19 at 20:51 +0300, Pekka Enberg wrote:
>> How is the attacker able to identify that we kmalloc()'d from ecryptfs or
>> VFS based on non-root /proc/slabinfo when the slab allocator itself does
>> not have that sort of information if you mix up the allocations? Isn't this
>> much stronger protection especially if you combine that with /proc/slabinfo
>> restriction?

On Mon, Sep 19, 2011 at 9:03 PM, Dave Hansen <dave@linux.vnet.ibm.com> wrote:
> Mixing it up just adds noise.  It makes the attack somewhat more
> difficult, but it still leaves open the possibility that the attacker
> can filter out the noise somehow.

So that would mean the attacker has somewhat fine-grained control over
kernel memory allocations, no? Can they use /proc/meminfo to deduce
the same kind of information? Should we close that down too?

                         Pekka

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Pekka Enberg]

On Mon, Sep 19, 2011 at 8:58 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
>> Isn't this
>> much stronger protection especially if you combine that with /proc/slabinfo
>> restriction?
>
> I don't see any reason to change allocators if we close slabinfo.

OK, so what about /proc/meminfo, sysfs, 'perf kmem', and other kernel interfaces
through which you can get direct or indirect information about kernel memory
allocations?

                                Pekka

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Vasiliy Kulikov]

On Mon, Sep 19, 2011 at 21:46 +0300, Pekka Enberg wrote:
> On Mon, Sep 19, 2011 at 8:58 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
> >> Isn't this
> >> much stronger protection especially if you combine that with /proc/slabinfo
> >> restriction?
> >
> > I don't see any reason to change allocators if we close slabinfo.
> 
> OK, so what about /proc/meminfo, sysfs, 'perf kmem', and other kernel interfaces
> through which you can get direct or indirect information about kernel memory
> allocations?

Oh, we also have perf...  Given these are separate interfaces, I think
slab oriented restriction makes more sense.

So, now we have:

/proc/slabinfo
/sys/kernel/slab
/proc/meminfo
'perf kmem' - not sure what specific files should be guarded


Is there another way to get directly or indirectly the information about
slabs?


Thanks,

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Linus Torvalds]

On Mon, Sep 19, 2011 at 11:46 AM, Pekka Enberg <penberg@cs.helsinki.fi> wrote:
>
> OK, so what about /proc/meminfo, sysfs, 'perf kmem', and other kernel interfaces
> through which you can get direct or indirect information about kernel memory
> allocations?

Pekka, there are degrees of badness.

Also, quite frankly, your argument that /proc/slabinfo is so important
for kernel debugging is bogus. Every time I've complained about the
fact that the thing is useless AND ACTIVELY MISLEADING because it
mixes up all the slabs (so big numbers for "vm_area_struct" might
actually be about some other slab entirely, and *has* been, to the
point of people wasting time), the answer has been "whatever".

You can't have it both ways just to argue for the status quo.

Considering how useless /proc/slabinfo actually is today - exactly
because of the misleading mixing - I suspect the right thing to do is
to make it root-only.

Having some aggregate number in /proc/meminfo would probably be fine.

And yes, we probably should avoid giving page-level granularity in
/proc/meminfo too. Do it in megabytes instead. None of the information
there is really relevant at a page level, everybody just wants rough
aggregates.

                   Linus

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Pekka Enberg]

Hi Linus,

On Mon, Sep 19, 2011 at 9:55 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> Also, quite frankly, your argument that /proc/slabinfo is so important
> for kernel debugging is bogus. Every time I've complained about the
> fact that the thing is useless AND ACTIVELY MISLEADING because it
> mixes up all the slabs (so big numbers for "vm_area_struct" might
> actually be about some other slab entirely, and *has* been, to the
> point of people wasting time), the answer has been "whatever".
>
> You can't have it both ways just to argue for the status quo.

Well, sure. I was actually planning to rip out SLUB merging completely
because it makes /proc/slabinfo so useless but never got around doing
that. Mixing up allocations makes heap exploits harder but there's no
agreement on how much more difficult (i.e. if it matters at all for brute
force attacks).

So dunno what's the right thing to do here. Every time I discuss the
issues with 'security folks' I'm left with more questions than answers...
Everybody seems to be more interested in closing down kernel ABIs
rather than making kernel memory allocations more robust against
attacks.

But anyway, if you feel about this strongly feel free to pick up
Vasiliy's patch. I think my suggestion of introducing a
CONFIG_RESTRICT_PROCFS makes most sense because:

  - When we've mucked around with /proc/slabinfo in the past, we have
    broken setups. (That might be less relevant now.)

  - /proc/slabinfo is not the only source where you can get information
    on kernel memory allocations (we have one in sysfs and perf kmem).

On Mon, Sep 19, 2011 at 9:55 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> Considering how useless /proc/slabinfo actually is today - exactly
> because of the misleading mixing - I suspect the right thing to do is
> to make it root-only.
>
> Having some aggregate number in /proc/meminfo would probably be fine.
>
> And yes, we probably should avoid giving page-level granularity in
> /proc/meminfo too. Do it in megabytes instead. None of the information
> there is really relevant at a page level, everybody just wants rough
> aggregates.

We have this in /proc/meminfo:

Slab:              20012 kB

Or did you mean something even more specific?

                        Pekka

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Pekka Enberg]

On Mon, Sep 19, 2011 at 9:55 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
> Oh, we also have perf...  Given these are separate interfaces, I think
> slab oriented restriction makes more sense.
>
> So, now we have:
>
> /proc/slabinfo
> /sys/kernel/slab
> /proc/meminfo
> 'perf kmem' - not sure what specific files should be guarded

I don't think you can close down 'perf kmem' per se. You need to make
sure the attacker is not able to use perf tracing (of which 'perf kmem' is
a subset).

                       Pekka

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Pekka Enberg]

On Mon, Sep 19, 2011 at 9:55 PM, Vasiliy Kulikov <segoon@openwall.com> wrote:
> Is there another way to get directly or indirectly the information about
> slabs?

I didn't check closely but there seems to be some filesystem specific
stats under /proc at least. No idea how useful those are as attack
vectors.

I also suspect there are per-module knobs under /proc and /sys that
can be used to get indirect information. Many subsystems have
implemented their own slab wrappers in the past and they keep popping
up every now and then.

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Pekka Enberg]

On Mon, Sep 19, 2011 at 10:18 PM, Pekka Enberg <penberg@cs.helsinki.fi> wrote:
>> Having some aggregate number in /proc/meminfo would probably be fine.
>>
>> And yes, we probably should avoid giving page-level granularity in
>> /proc/meminfo too. Do it in megabytes instead. None of the information
>> there is really relevant at a page level, everybody just wants rough
>> aggregates.
>
> We have this in /proc/meminfo:
>
> Slab:              20012 kB
>
> Or did you mean something even more specific?

Oh, sorry, I completely misread what you wrote above. Sure, we can
round the numbers into megabytes without breaking the ABI.

                        Pekka

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 452 bytes --]

On Mon, 19 Sep 2011 18:46:58 +0400, Vasiliy Kulikov said:

> One note: only to _kernel_ developers.  It means it is a strictly
> debugging feature, which shouldn't be enabled in the production systems.

Until somebody at vendor support says "What does 'cat /proc/slabinfo' say?"

Anybody who thinks that debugging tools should be totally disabled on
"production" systems probably hasn't spent enough time actually
running production systems.

[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Alan Cox]

On Mon, 19 Sep 2011 15:45:07 -0400
Valdis.Kletnieks@vt.edu wrote:

> On Mon, 19 Sep 2011 18:46:58 +0400, Vasiliy Kulikov said:
> 
> > One note: only to _kernel_ developers.  It means it is a strictly
> > debugging feature, which shouldn't be enabled in the production
> > systems.
> 
> Until somebody at vendor support says "What does 'cat /proc/slabinfo'
> say?"
> 
> Anybody who thinks that debugging tools should be totally disabled on
> "production" systems probably hasn't spent enough time actually
> running production systems.

Agreed - far better it is simply set root only. At that point if you
can read it you've bypassed DAC anyway so either you already control
the box or something like SELinux or SMACK is in the way in which case
*it* can manage extra policy just fine.

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 424 bytes --]

On Mon, 19 Sep 2011 12:51:10 CDT, Christoph Lameter said:

> IMHO a restriction of access to slab statistics is reasonable in a
> hardened environment. Make it dependent on CONFIG_SECURITY or some such
> thing?

Probably need to invent a separate Kconfig variable - CONFIG_SECURITY
is probably a way-too-big hammer for this nail. I can see lots of systems
that want to enable that, but won't want to tighten access to slab.

[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Christoph Lameter]

On Mon, 19 Sep 2011, Valdis.Kletnieks@vt.edu wrote:

> On Mon, 19 Sep 2011 12:51:10 CDT, Christoph Lameter said:
>
> > IMHO a restriction of access to slab statistics is reasonable in a
> > hardened environment. Make it dependent on CONFIG_SECURITY or some such
> > thing?
>
> Probably need to invent a separate Kconfig variable - CONFIG_SECURITY
> is probably a way-too-big hammer for this nail. I can see lots of systems
> that want to enable that, but won't want to tighten access to slab.

There is already CONFIG_SECURITY_DMESG_RESTRICT. Generalize that setting
to include all sorts of other kernel statistics?

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1336 bytes --]

On Mon, 19 Sep 2011 15:02:34 CDT, Christoph Lameter said:
> On Mon, 19 Sep 2011, Valdis.Kletnieks@vt.edu wrote:
> 
> > On Mon, 19 Sep 2011 12:51:10 CDT, Christoph Lameter said:
> >
> > > IMHO a restriction of access to slab statistics is reasonable in a
> > > hardened environment. Make it dependent on CONFIG_SECURITY or some such
> > > thing?
> >
> > Probably need to invent a separate Kconfig variable - CONFIG_SECURITY
> > is probably a way-too-big hammer for this nail. I can see lots of systems
> > that want to enable that, but won't want to tighten access to slab.
> 
> There is already CONFIG_SECURITY_DMESG_RESTRICT. Generalize that setting
> to include all sorts of other kernel statistics?

The question becomes "What are the chances that a given site either will or
will not agree with using the same setting for all things aggregated under
the same option".   I could probably make a good case for a  flag
called CONFIG_SECURITY_INFOLEAK_RESTRICT that would be a switch
for "standard perms or root only" for all info-leaking files under /proc and
/sys.

The problem is that dmesg has *other* info-leakage issues - but it's not
internal kernel state, it's stuff like ipfilter messages and potentially
Selinux messages if auditd isn't running, and of late things like systemd and
crond like to stick crap in there as well.


[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[David Rientjes]

On Mon, 19 Sep 2011, Pekka Enberg wrote:

> Well, sure. I was actually planning to rip out SLUB merging completely
> because it makes /proc/slabinfo so useless but never got around doing
> that.

Ripping out cache merging entirely for the benefit of an interface seems 
like overkill, it actually allows the allocator to return cache-hot 
objects that has a small but measurable impact on performance for some 
networking loads.  It would probably be better to increase awareness of 
slabinfo -a and the use of slub_nomerge on the command line when debugging 
issues.  The most complete solution would be to move everything out of 
struct kmem_cache except what is necessary for slabinfo and then point to 
the actual cache data structure that could be shared by merged caches.  
That's not hard to do, but would add an increment to both the alloc and 
free fastpaths and require some surgery throughout all of the slub code to 
understand the new data structures and that would be a pretty big patch.

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Vasiliy Kulikov]

(I have to increase the CC list by CC'ing Greg and perf guys.  The
motivation for restricting slabinfo is here:

http://www.openwall.com/lists/kernel-hardening/2011/09/10/4)

> Anybody who thinks that debugging tools should be totally disabled on
> "production" systems probably hasn't spent enough time actually
> running production systems.

On Mon, Sep 19, 2011 at 20:55 +0100, Alan Cox wrote:
> Agreed - far better it is simply set root only.

Sorry, I've poorly worded my statement.  Of course I mean root-only
slabinfo, not totally disable it.


So, where are we now?

Linus, Alan, Kees, and Dave are about to simply restrict slabinfo (and
probably similar interfaces) to root.  Pekka is OK too.

Christoph and Valdis are about to create new CONFIG_ option to be able
to restrict the access to slabinfo/etc., but with old relaxed
permissions.


IMO having a compatibility with the old tools using slabinfo/meminfo as
non-root for good things is great, but probably a seamless upgrade
doesn't worth it in this case.  Instead, we can provide a transition
instructions to continue to use non-root monitoring tools:

    groupadd meminfo
    usermod -a -G meminfo $MONITOR_USER

And add these lines to the init scripts between mount /proc and running
monitoring daemon, for Ubuntu's upstart it should fit in mountall.conf:

    chmod g+r /proc/slabinfo /proc/meminfo
    chgrp meminfo /proc/slabinfo /proc/meminfo

Then the daemon may watch for memleaks again.  It requires some actions
from the sysadmin, but no changes to the daemons and it helps to keep
sane kernel defaults.

Are we OK with this solution?


Now about other sources of infoleaks.  Pekka has noticed that the same
information about slabs is accessible from sysfs and 'perf kmem':

http://www.openwall.com/lists/kernel-hardening/2011/09/19/23

For sysfs the solution is relatively simple - the same __ATTR(..., 0600,
...) for SLAB_ATTR() (all __ATTR* in mm/*.c?) with the same chmod in
init scripts.


For the perf the situation differs.  AFAICS (please correct me if I'm
wrong) all performance events are recorded and passed to userspace
without any security checks.  IOW, every event happened in the current
process' context is signalled to the current task if requested.  It
contains k*alloc/kfree, which are the subject of this thread, and
probably much more infoleaks which simply nobody cared about yet.
Probably it's time to develop some rules to identify what events are
safe to signal to user and what are dangerous/private to the
system/other users (probably, potentially)?  I'm not very familiar with
perf events and there might be already such mechanisms, but they're just
not used for memory things, so this is a question to the perf guys.


Other sources of similar infoleaks are fs-specific and IMO we should fix
them after these more "generic" infoleaks (slabinfo, meminfo, sysfs,
perf).


Thanks,

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments

Re: [kernel-hardening] Re: [RFC PATCH 2/2] mm: restrict access to /proc/slabinfo

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 799 bytes --]

On Wed, 21 Sep 2011 21:05:27 +0400, Vasiliy Kulikov said:
> Sorry, I've poorly worded my statement.  Of course I mean root-only
> slabinfo, not totally disable it.

Oh, that I can live with.. ;)

> Linus, Alan, Kees, and Dave are about to simply restrict slabinfo (and
> probably similar interfaces) to root.  Pekka is OK too.
> 
> Christoph and Valdis are about to create new CONFIG_ option to be able
> to restrict the access to slabinfo/etc., but with old relaxed
> permissions.

I'm OK with a decision to just make the files mode 400 and be done with it,
since I can always stick a chmod in the startup scripts if it's *really* a problem.

Just that *if* we add a CONFIG_ option, it shouldn't be slabinfo-specific, but
should cover the *other* identified info-leakers in /proc and /sys as well.

[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

Re: [RFC PATCH 1/2] proc: force dcache drop on unauthorized access

[Vasiliy Kulikov]

Hello Andrew,

On Sat, Sep 10, 2011 at 20:40 +0400, Vasiliy Kulikov wrote:
> The patch "proc: fix races against execve() of /proc/PID/fd**" is still
> a partial fix for a setxid problem.  link(2) is a yet another way to
> identify whether a specific fd is opened by a privileged process.  By
> calling link(2) against /proc/PID/fd/* an attacker may identify whether
> the fd number is valid for PID by analysing link(2) return code.
> 
> Both getattr() and link() can be used by the attacker iff the dentry is
> present in the dcache.  In this case ->lookup() is not called and the
> only way to check ptrace permissions is either operation handler or
> ->revalidate().  The easiest solution to prevent any unauthorized access
> to /proc/PID/fd*/ files is to force the dentry drop on each unauthorized
> access attempt.
> 
> If an attacker keeps opened fd of /proc/PID/fd/ and dcache contains
> a specific dentry for some /proc/PID/fd/XXX, any future attemp to use the
> dentry by the attacker would lead to the dentry drop as a result of a
> failed ptrace check in ->revalidate().  Then the attacker cannot spawn a
> dentry for the specific fd number because of ptrace check in ->lookup().
> 
> The dentry drop can be still observed by an attacker by analysing
> information from /proc/slabinfo, which is addressed in the successive
> patch.

After (almost) everybody agreed that closing slabinfo and similar is
a good thing, should I resend these 2 patches (/proc/PID/fd and
slabinfo) with sysfs addition or you'll take these patches and I'll send
sysfs patch after them?

Thanks,

-- 
Vasiliy Kulikov
http://www.openwall.com - bringing security into open computing environments