All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vasiliy Kulikov <segoon@openwall.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, Al Viro <viro@zeniv.linux.org.uk>,
	Stephen Wilson <wilsons@start.ca>,
	Alexey Dobriyan <adobriyan@gmail.com>,
	security@kernel.org, kernel-hardening@lists.openwall.com
Subject: [kernel-hardening] Re: [PATCH] proc: restrict access to /proc/$PID/{sched,schedstat}
Date: Wed, 9 Nov 2011 13:06:34 +0400	[thread overview]
Message-ID: <20111109090634.GA3418@albatros> (raw)
In-Reply-To: <20111108151752.b2e37741.akpm@linux-foundation.org>

On Tue, Nov 08, 2011 at 15:17 -0800, Andrew Morton wrote:
> > On Sat, Nov 05, 2011 at 14:48 +0400, Vasiliy Kulikov wrote:
> > > /proc/$PID/{sched,schedstat} contain debugging scheduler counters, which
> > > should not be world readable.  They may be used to gather private information
> > > about processes' activity.  E.g. it can be used to count the number of
> > > characters typed in gksu dialog:
> > > 
> > > http://www.openwall.com/lists/oss-security/2011/11/05/3
> > > 
> > > This infoleak is similar to io (1d1221f375c) and stat's eip/esp (f83ce3e6b02d)
> > > infoleaks.  Probably other 0644/0444 procfs files are vulnerable to
> > > similar infoleaks.
> 
> Grumble.
> 
> The obvious issue with this patch is its non-back-compatibility.  What
> existing code will break, in what manner and what is the seriousness of
> the breakage?
> 
> You *know* this is the main issue, yet you didn't address it at all! 
> You just leave the issue out there for other people to work out, and to
> ask the obvious questions.
> 
> This happens over and over and I'm getting rather tired of the charade.
> 
> So I'm going to ignore this patch and I ask that you and other security
> people never do this again.
> 
> If you're going to submit a patch which you know will change kernel
> interfaces in a non-backward-compatible fashion then don't just pretend
> that it didn't happen!  Please provide us with a complete description
> of the breakage and at least some analysis of the downstream
> implications of the change.  So that we are better able to decide
> whether the security improvements justify the disruption.

I'm sorry it looked like I didn't test the patch, but I really didn't
face to any breakage (top, ps, gnome monitor).  The actual problem is
that the patch is still incomplete - all proc monitoring tools watch for
/proc/$PID/stat file content changes, not /proc/$PID/sched.
/proc/$PID/stat contains the same sched information, which I've missed.
Restricting "stat" does break these tools.

/proc/$PID/stat already has "fake" fields like KSTK_EIP() and KSTK_ESP().
We can continue to do such sort of force fields zeroing, which doesn't
break ABI.


Thanks,

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

WARNING: multiple messages have this Message-ID (diff)
From: Vasiliy Kulikov <segoon@openwall.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, Al Viro <viro@zeniv.linux.org.uk>,
	Stephen Wilson <wilsons@start.ca>,
	Alexey Dobriyan <adobriyan@gmail.com>,
	security@kernel.org, kernel-hardening@lists.openwall.com
Subject: Re: [PATCH] proc: restrict access to /proc/$PID/{sched,schedstat}
Date: Wed, 9 Nov 2011 13:06:34 +0400	[thread overview]
Message-ID: <20111109090634.GA3418@albatros> (raw)
In-Reply-To: <20111108151752.b2e37741.akpm@linux-foundation.org>

On Tue, Nov 08, 2011 at 15:17 -0800, Andrew Morton wrote:
> > On Sat, Nov 05, 2011 at 14:48 +0400, Vasiliy Kulikov wrote:
> > > /proc/$PID/{sched,schedstat} contain debugging scheduler counters, which
> > > should not be world readable.  They may be used to gather private information
> > > about processes' activity.  E.g. it can be used to count the number of
> > > characters typed in gksu dialog:
> > > 
> > > http://www.openwall.com/lists/oss-security/2011/11/05/3
> > > 
> > > This infoleak is similar to io (1d1221f375c) and stat's eip/esp (f83ce3e6b02d)
> > > infoleaks.  Probably other 0644/0444 procfs files are vulnerable to
> > > similar infoleaks.
> 
> Grumble.
> 
> The obvious issue with this patch is its non-back-compatibility.  What
> existing code will break, in what manner and what is the seriousness of
> the breakage?
> 
> You *know* this is the main issue, yet you didn't address it at all! 
> You just leave the issue out there for other people to work out, and to
> ask the obvious questions.
> 
> This happens over and over and I'm getting rather tired of the charade.
> 
> So I'm going to ignore this patch and I ask that you and other security
> people never do this again.
> 
> If you're going to submit a patch which you know will change kernel
> interfaces in a non-backward-compatible fashion then don't just pretend
> that it didn't happen!  Please provide us with a complete description
> of the breakage and at least some analysis of the downstream
> implications of the change.  So that we are better able to decide
> whether the security improvements justify the disruption.

I'm sorry it looked like I didn't test the patch, but I really didn't
face to any breakage (top, ps, gnome monitor).  The actual problem is
that the patch is still incomplete - all proc monitoring tools watch for
/proc/$PID/stat file content changes, not /proc/$PID/sched.
/proc/$PID/stat contains the same sched information, which I've missed.
Restricting "stat" does break these tools.

/proc/$PID/stat already has "fake" fields like KSTK_EIP() and KSTK_ESP().
We can continue to do such sort of force fields zeroing, which doesn't
break ABI.


Thanks,

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

  reply	other threads:[~2011-11-09  9:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20111105104828.GA3489@albatros>
2011-11-08 11:59 ` [PATCH] proc: restrict access to /proc/$PID/{sched,schedstat} Vasiliy Kulikov
2011-11-08 23:17   ` Andrew Morton
2011-11-09  9:06     ` Vasiliy Kulikov [this message]
2011-11-09  9:06       ` Vasiliy Kulikov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111109090634.GA3418@albatros \
    --to=segoon@openwall.com \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=security@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=wilsons@start.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.