proc: avoid information leaks to non-privileged processes
diff mbox series

Message ID 20090504125114.5e391564@chukar
State New, archived
Headers show
Series
  • proc: avoid information leaks to non-privileged processes
Related show

Commit Message

Jake Edge May 4, 2009, 6:51 p.m. UTC
This is essentially v2 of "[PATCH] proc: avoid leaking eip, esp, or
wchan to non-privileged processes", adding some of Eric Biederman's
suggestions as well as the start_stack change (only give out that
address if the process is ptrace()-able).  This has been tested with ps
and top without any ill effects being seen.

proc: avoid information leaks to non-privileged processes

By using the same test as is used for /proc/pid/maps and /proc/pid/smaps,
only allow processes that can ptrace() a given process to see information
that might be used to bypass address space layout randomization (ASLR).
These include eip, esp, wchan, and start_stack in /proc/pid/stat as well
as the non-symbolic output from /proc/pid/wchan.

ASLR can be bypassed by sampling eip as shown by the proof-of-concept code
at http://code.google.com/p/fuzzyaslr/  As part of a presentation
(http://www.cr0.org/paper/to-jt-linux-alsr-leak.pdf) esp and wchan were also
noted as possibly usable information leaks as well.  The start_stack address
also leaks potentially useful information.

Cc: Stable Team <stable@kernel.org>
Signed-off-by: Jake Edge <jake@lwn.net>
---
 fs/proc/array.c |   13 +++++++++----
 fs/proc/base.c  |    5 ++++-
 2 files changed, 13 insertions(+), 5 deletions(-)

Comments

Linus Torvalds May 4, 2009, 7 p.m. UTC | #1
On Mon, 4 May 2009, Jake Edge wrote:
>
> This is essentially v2 of "[PATCH] proc: avoid leaking eip, esp, or
> wchan to non-privileged processes", adding some of Eric Biederman's
> suggestions as well as the start_stack change (only give out that
> address if the process is ptrace()-able).  This has been tested with ps
> and top without any ill effects being seen.

Looks sane to me. Anybody objects?

		Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Arjan van de Ven May 4, 2009, 7:51 p.m. UTC | #2
On Mon, 4 May 2009 12:00:12 -0700 (PDT)
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> 
> 
> On Mon, 4 May 2009, Jake Edge wrote:
> >
> > This is essentially v2 of "[PATCH] proc: avoid leaking eip, esp, or
> > wchan to non-privileged processes", adding some of Eric Biederman's
> > suggestions as well as the start_stack change (only give out that
> > address if the process is ptrace()-able).  This has been tested
> > with ps and top without any ill effects being seen.
> 
> Looks sane to me. Anybody objects?
> 

Acked-by: Arjan van de Ven <arjan@linux.intel.com>
Eric W. Biederman May 4, 2009, 8:20 p.m. UTC | #3
Arjan van de Ven <arjan@infradead.org> writes:

> On Mon, 4 May 2009 12:00:12 -0700 (PDT)
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>> 
>> 
>> On Mon, 4 May 2009, Jake Edge wrote:
>> >
>> > This is essentially v2 of "[PATCH] proc: avoid leaking eip, esp, or
>> > wchan to non-privileged processes", adding some of Eric Biederman's
>> > suggestions as well as the start_stack change (only give out that
>> > address if the process is ptrace()-able).  This has been tested
>> > with ps and top without any ill effects being seen.
>> 
>> Looks sane to me. Anybody objects?
>> 
>
> Acked-by: Arjan van de Ven <arjan@linux.intel.com>

Looks sane here.

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Linus Torvalds May 4, 2009, 10:24 p.m. UTC | #4
On Mon, 4 May 2009, Eric W. Biederman wrote:

> Arjan van de Ven <arjan@infradead.org> writes:
> 
> > On Mon, 4 May 2009 12:00:12 -0700 (PDT)
> > Linus Torvalds <torvalds@linux-foundation.org> wrote:
> >
> >> 
> >> 
> >> On Mon, 4 May 2009, Jake Edge wrote:
> >> >
> >> > This is essentially v2 of "[PATCH] proc: avoid leaking eip, esp, or
> >> > wchan to non-privileged processes", adding some of Eric Biederman's
> >> > suggestions as well as the start_stack change (only give out that
> >> > address if the process is ptrace()-able).  This has been tested
> >> > with ps and top without any ill effects being seen.
> >> 
> >> Looks sane to me. Anybody objects?
> >> 
> >
> > Acked-by: Arjan van de Ven <arjan@linux.intel.com>
> 
> Looks sane here.
> 
> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

Ok, applied.

Also, does anybody have any commentary or opinion on the patch by Matt 
Mackall to use stronger random numbers than "get_random_int()". I wonder 
what the performance impact of that is - "get_random_int()" is very cheap 
by design, and many users may consider calling "get_random_bytes()" to be 
overkill and a potential performance issue.

Quite frankly, the way "get_random_bytes()" works now (it does a _full_ 
sha thing every time), I think it's insane overkill. But I do have to 
admit that our current "get_random_int()" is insane _underkill_.

I'd like to improve the latter without going to quie the extreme that 
matt's patch did.

		Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Arjan van de Ven May 4, 2009, 11:26 p.m. UTC | #5
On Mon, 4 May 2009 15:24:15 -0700 (PDT)
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> 
> 
> On Mon, 4 May 2009, Eric W. Biederman wrote:
> 
> > Arjan van de Ven <arjan@infradead.org> writes:
> > 
> > > On Mon, 4 May 2009 12:00:12 -0700 (PDT)
> > > Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > >
> > >> 
> > >> 
> > >> On Mon, 4 May 2009, Jake Edge wrote:
> > >> >
> > >> > This is essentially v2 of "[PATCH] proc: avoid leaking eip,
> > >> > esp, or wchan to non-privileged processes", adding some of
> > >> > Eric Biederman's suggestions as well as the start_stack change
> > >> > (only give out that address if the process is ptrace()-able).
> > >> > This has been tested with ps and top without any ill effects
> > >> > being seen.
> > >> 
> > >> Looks sane to me. Anybody objects?
> > >> 
> > >
> > > Acked-by: Arjan van de Ven <arjan@linux.intel.com>
> > 
> > Looks sane here.
> > 
> > Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
> 
> Ok, applied.
> 
> Also, does anybody have any commentary or opinion on the patch by
> Matt Mackall to use stronger random numbers than "get_random_int()".
> I wonder what the performance impact of that is - "get_random_int()"
> is very cheap by design, and many users may consider calling
> "get_random_bytes()" to be overkill and a potential performance issue.
> 
> Quite frankly, the way "get_random_bytes()" works now (it does a
> _full_ sha thing every time), I think it's insane overkill. But I do
> have to admit that our current "get_random_int()" is insane
> _underkill_.
> 
> I'd like to improve the latter without going to quie the extreme that 
> matt's patch did.

doing something simple as hashing in the tsc will already help;
while some people are nervous about the predictability of the tsc,
in practice there's likely enough bits of unpredictability there
to be worth the very low price of 50 cycles or less....
Linus Torvalds May 4, 2009, 11:54 p.m. UTC | #6
On Mon, 4 May 2009, Linus Torvalds wrote:
> 
> Quite frankly, the way "get_random_bytes()" works now (it does a _full_ 
> sha thing every time), I think it's insane overkill. But I do have to 
> admit that our current "get_random_int()" is insane _underkill_.

Actually, I don't think "get_random_int()" is underkill per se (it does 
that half md4 transform to try to hide the source of the data), but the 
data itself is simply not modified at all, and the buffers aren't updated 
in between rounds.

In fact "secure_ip_id()" (which it uses) explicityl does that private 
hash[] array so that the mixing that "half_md4_transform()" does do will 
_not_ be saved for the next round - so the next round will always start 
from the same keyptr "secret" state.

I think.

If that wasn't the case, and we actually kept mixing up the end result 
back into the next iteration, I suspect the current "get_random_int()" 
wouldn't be _nearly_ as bad as it is now. 

Or maybe I'm missing some part of the transform, and we do mix the values 
back as we do that "get_random_int()". I just don't see it. And if I'm 
right, then I think _that_ is the real weakness of our current 
get_random_int().

			Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Matt Mackall May 5, 2009, 5:50 a.m. UTC | #7
On Mon, May 04, 2009 at 03:24:15PM -0700, Linus Torvalds wrote:
> 
> 
> On Mon, 4 May 2009, Eric W. Biederman wrote:
> 
> > Arjan van de Ven <arjan@infradead.org> writes:
> > 
> > > On Mon, 4 May 2009 12:00:12 -0700 (PDT)
> > > Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > >
> > >> 
> > >> 
> > >> On Mon, 4 May 2009, Jake Edge wrote:
> > >> >
> > >> > This is essentially v2 of "[PATCH] proc: avoid leaking eip, esp, or
> > >> > wchan to non-privileged processes", adding some of Eric Biederman's
> > >> > suggestions as well as the start_stack change (only give out that
> > >> > address if the process is ptrace()-able).  This has been tested
> > >> > with ps and top without any ill effects being seen.
> > >> 
> > >> Looks sane to me. Anybody objects?
> > >> 
> > >
> > > Acked-by: Arjan van de Ven <arjan@linux.intel.com>
> > 
> > Looks sane here.
> > 
> > Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
> 
> Ok, applied.
> 
> Also, does anybody have any commentary or opinion on the patch by Matt 
> Mackall to use stronger random numbers than "get_random_int()". I wonder 
> what the performance impact of that is - "get_random_int()" is very cheap 
> by design, and many users may consider calling "get_random_bytes()" to be 
> overkill and a potential performance issue.
> 
> Quite frankly, the way "get_random_bytes()" works now (it does a _full_ 
> sha thing every time), I think it's insane overkill. But I do have to 
> admit that our current "get_random_int()" is insane _underkill_.
> 
> I'd like to improve the latter without going to quie the extreme that 
> matt's patch did.

ASLR aside, get_random_u32 is the right interface (strong,
well-defined type) for random.c to export and get_random_int (weak,
ambiguous type) is the wrong one.

As to what's the appropriate sort of RNG for ASLR to use, finding a
balance between too strong and too weak is tricky. I'd rather move
things to a known safe point and back it off to acceptable performance
from there if anyone complains. So let's do my patch now.

Looking forward:

A faster-but-weakened RNG for ASLR (and similar purposes) will still
need to be strong in the cryptographic sense. Which probably means
having secret state (per cpu?) that changes at every call via a strong
cipher or hash (though lighter than the full RNG). Alternately, we use
a weak primitive (eg halfmd4) with per-task secrets. Not really keen
on the latter as a user may expose outputs across task boundaries that
allow backtracking attacks against the ASLR. In other words, the
latter requires disciplined users, while the former doesn't.
tip-bot for Ingo Molnar May 5, 2009, 6:31 a.m. UTC | #8
* Matt Mackall <mpm@selenic.com> wrote:

> As to what's the appropriate sort of RNG for ASLR to use, finding 
> a balance between too strong and too weak is tricky. [...]

In exec-shield i mixed 'easily accessible and fast' semi-random 
state to the get_random_int() result: xor-ed the cycle counter, the 
pid and a kernel address to it. That strengthened the result in a 
pretty practical way (without strengthening the theoretical 
randomless - each of those items are considered guessable) and does 
so without weakening the entropy of the random pool.

As usual, it got objected to and removed during upstream review so 
the upstream code stands on a single foot only - which is an 
obviously bad idea.

The thing is, it's very hard to argue for (and prove) security 
related complexity on an objective basis. ASLR was met with quite 
some upstream hostility, so it did not really get merged upstream, 
it barely managed to limp upstream.

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Eric W. Biederman May 5, 2009, 7:51 a.m. UTC | #9
Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Mon, 4 May 2009, Linus Torvalds wrote:
>> 
>> Quite frankly, the way "get_random_bytes()" works now (it does a _full_ 
>> sha thing every time), I think it's insane overkill. But I do have to 
>> admit that our current "get_random_int()" is insane _underkill_.
>
> Actually, I don't think "get_random_int()" is underkill per se (it does 
> that half md4 transform to try to hide the source of the data), but the 
> data itself is simply not modified at all, and the buffers aren't updated 
> in between rounds.
>
> In fact "secure_ip_id()" (which it uses) explicityl does that private 
> hash[] array so that the mixing that "half_md4_transform()" does do will 
> _not_ be saved for the next round - so the next round will always start 
> from the same keyptr "secret" state.
>
> I think.
>
> If that wasn't the case, and we actually kept mixing up the end result 
> back into the next iteration, I suspect the current "get_random_int()" 
> wouldn't be _nearly_ as bad as it is now. 
>
> Or maybe I'm missing some part of the transform, and we do mix the values 
> back as we do that "get_random_int()". I just don't see it. And if I'm 
> right, then I think _that_ is the real weakness of our current 
> get_random_int().

Yes, not mixing the result back (which would give us some kind of
pseudo random number generator) is the problem.

secure_ip_id, looks to be a very different kind of thing.  A seed
that is reused periodically.  Ultimately those values do change.

For the state we are mixing back into I expect we want it to be
per cpu so we don't need locks and avoid  cache line ping pongs
when we mix the state back.

I haven't seen Matts patch and couldn't find it when I did a quick
look so I don't have any idea there.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Eric W. Biederman May 5, 2009, 8:14 a.m. UTC | #10
Ingo Molnar <mingo@elte.hu> writes:

> * Matt Mackall <mpm@selenic.com> wrote:
>
>> As to what's the appropriate sort of RNG for ASLR to use, finding 
>> a balance between too strong and too weak is tricky. [...]
>
> In exec-shield i mixed 'easily accessible and fast' semi-random 
> state to the get_random_int() result: xor-ed the cycle counter, the 
> pid and a kernel address to it. That strengthened the result in a 
> pretty practical way (without strengthening the theoretical 
> randomless - each of those items are considered guessable) and does 
> so without weakening the entropy of the random pool.

The trouble is, that thinking completely misses the problem, and I
expect that is why we have a problem.  Throwing a bunch of possibly
truly random values into the pot for luck is nice.  But you didn't
throw in a pseudo random number generator.  An unpredictable sequence
that is guaranteed to change from one invocation to the next.

In a very practical sense a pseudo random generator is completely
sufficient.  Throwing in a few truly random numbers guards against
attacks on the random number generator.

What we have now is a hash over an a value that changes every 5 minutes
and some well known values.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Andi Kleen May 5, 2009, 8:58 a.m. UTC | #11
Matt Mackall <mpm@selenic.com> writes:
>
> Looking forward:
>
> A faster-but-weakened RNG for ASLR (and similar purposes) 

We really need it for the user space interface too, right now
recent glibc drains your entropy pool on every exec, and worse
recent kernels drain it now even with old glibc too. So any
system which doesn't have a active high frequency random number
source (which is most systems) doesn't have much real entropy
left for the applications that really need it.

-Andi (who always thought it was a bad idea to let ASLR weaken
your SSL/SSH session keys)
Linus Torvalds May 5, 2009, 3:17 p.m. UTC | #12
On Tue, 5 May 2009, Eric W. Biederman wrote:
> 
> Yes, not mixing the result back (which would give us some kind of
> pseudo random number generator) is the problem.

Guys, so how about this?

It's a really simple patch that basically just open-codes the current 
"secure_ip_id()" call, but when open-coding it we now use a _static_ 
hashing area, so that it gets updated every time.

And to make sure somebody can't just start from the same original seed of 
all-zeroes, and then do the "half_md4_transform()" over and over until 
they get the same sequence as the kernel has, each iteration also mixes in 
the same old "current->pid + jiffies" we used - so we should now have a 
regular strong pseudo-number generator, but we also have one that doesn't 
have a single seed.

Note: the "pid + jiffies" is just meant to be a tiny tiny bit of noise. It 
has no real meaning. It could be anything. I just picked the previous 
seed, it's just that now we keep the state in between calls and that will 
feed into the next result, and that should make all the difference.

I made that hash be a per-cpu data just to avoid cache-line ping-pong: 
having multiple CPU's write to the same data would be fine for randomness, 
and add yet another layer of chaos to it, but since get_random_int() is 
supposed to be a fast interface I did it that way instead. I considered 
using "__raw_get_cpu_var()" to avoid any preemption overhead while still 
getting the hash be _mostly_ ping-pong free, but in the end good taste won 
out.

I have _not_ tested this. Somebody obviously should, and I'm not checking 
it in without ack's and somebody testing it.

		Linus
---
 drivers/char/random.c |   19 ++++++++++++-------
 1 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index f824ef8..9f8fd5b 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1665,15 +1665,20 @@ EXPORT_SYMBOL(secure_dccp_sequence_number);
  * value is not cryptographically secure but for several uses the cost of
  * depleting entropy is too high
  */
+DEFINE_PER_CPU(__u32 [4], get_random_int_hash);
 unsigned int get_random_int(void)
 {
-	/*
-	 * Use IP's RNG. It suits our purpose perfectly: it re-keys itself
-	 * every second, from the entropy pool (and thus creates a limited
-	 * drain on it), and uses halfMD4Transform within the second. We
-	 * also mix it with jiffies and the PID:
-	 */
-	return secure_ip_id((__force __be32)(current->pid + jiffies));
+	struct keydata *keyptr;
+	__u32 *hash = get_cpu_var(get_random_int_hash);
+	int ret;
+
+	keyptr = get_keyptr();
+	hash[0] += current->pid + jiffies;
+
+	ret = half_md4_transform(hash, keyptr->secret);
+	put_cpu_var(get_random_int_hash);
+
+	return ret;
 }
 
 /*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Linus Torvalds May 5, 2009, 3:35 p.m. UTC | #13
On Tue, 5 May 2009, Linus Torvalds wrote:
> 
> Note: the "pid + jiffies" is just meant to be a tiny tiny bit of noise. It 
> has no real meaning. It could be anything. I just picked the previous 
> seed, it's just that now we keep the state in between calls and that will 
> feed into the next result, and that should make all the difference.

Actually, thinking about it, we could/should probably just remove that 
tiny bit of noise.

After all, we get _real_ noise from the "keyptr->secret" thing. It's not 
updated all the time, but it's certainly updated often enough that nobody 
will ever see anything remotely guessable, I suspect.

Not that the "pid+jiffies" should hurt either, of course. It just doesn't 
really look meaningful, and only exists as a historical oddity that 
relates to the previous implementation of get_random_int().

			Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Matt Mackall May 5, 2009, 4:10 p.m. UTC | #14
On Tue, May 05, 2009 at 08:17:43AM -0700, Linus Torvalds wrote:
> 
> 
> On Tue, 5 May 2009, Eric W. Biederman wrote:
> > 
> > Yes, not mixing the result back (which would give us some kind of
> > pseudo random number generator) is the problem.
> 
> Guys, so how about this?
> 
> It's a really simple patch that basically just open-codes the current 
> "secure_ip_id()" call, but when open-coding it we now use a _static_ 
> hashing area, so that it gets updated every time.
> 
> And to make sure somebody can't just start from the same original seed of 
> all-zeroes, and then do the "half_md4_transform()" over and over until 
> they get the same sequence as the kernel has, each iteration also mixes in 
> the same old "current->pid + jiffies" we used - so we should now have a 
> regular strong pseudo-number generator, but we also have one that doesn't 
> have a single seed.
> 
> Note: the "pid + jiffies" is just meant to be a tiny tiny bit of noise. It 
> has no real meaning. It could be anything. I just picked the previous 
> seed, it's just that now we keep the state in between calls and that will 
> feed into the next result, and that should make all the difference.
> 
> I made that hash be a per-cpu data just to avoid cache-line ping-pong: 
> having multiple CPU's write to the same data would be fine for randomness, 
> and add yet another layer of chaos to it, but since get_random_int() is 
> supposed to be a fast interface I did it that way instead. I considered 
> using "__raw_get_cpu_var()" to avoid any preemption overhead while still 
> getting the hash be _mostly_ ping-pong free, but in the end good taste won 
> out.

It's an ok model, but I'd be much happier if we used a decent
hash function. I'm not a cryptanalyst, so I can't rattle off all the
known attacks against this hash function, but it can no longer be
considered any kind of cryptographic primitive. I would not be at all
surprised if it was possible to recover the entire secret state by
observing some small set of RNG outputs and then use that to backtrack
the RNG to reveal the layout of a recently launched process.

(Sending off a couple queries to some folks who are good at breaking
such things to confirm my hunches.)

Also, the current function name must go. It is seriously misleading.
get_random_u32 please.
Matt Mackall May 5, 2009, 4:18 p.m. UTC | #15
On Tue, May 05, 2009 at 08:35:35AM -0700, Linus Torvalds wrote:
> 
> 
> On Tue, 5 May 2009, Linus Torvalds wrote:
> > 
> > Note: the "pid + jiffies" is just meant to be a tiny tiny bit of noise. It 
> > has no real meaning. It could be anything. I just picked the previous 
> > seed, it's just that now we keep the state in between calls and that will 
> > feed into the next result, and that should make all the difference.
> 
> Actually, thinking about it, we could/should probably just remove that 
> tiny bit of noise.
> 
> After all, we get _real_ noise from the "keyptr->secret" thing. It's not 
> updated all the time, but it's certainly updated often enough that nobody 
> will ever see anything remotely guessable, I suspect.
> 
> Not that the "pid+jiffies" should hurt either, of course. It just doesn't 
> really look meaningful, and only exists as a historical oddity that 
> relates to the previous implementation of get_random_int().

I think it can only do good here. Recursively applied functions are
vulnerable to falling into greatly reduced state spaces (see 'strange
attractors') and adding any old crap can perturb it out of those
spaces. A good hash function should be resistant to this, but MD4 and
half_MD4 are not good hash functions.
tip-bot for Ingo Molnar May 5, 2009, 7:52 p.m. UTC | #16
* Eric W. Biederman <ebiederm@xmission.com> wrote:

> Ingo Molnar <mingo@elte.hu> writes:
> 
> > * Matt Mackall <mpm@selenic.com> wrote:
> >
> >> As to what's the appropriate sort of RNG for ASLR to use, finding 
> >> a balance between too strong and too weak is tricky. [...]
> >
> > In exec-shield i mixed 'easily accessible and fast' semi-random 
> > state to the get_random_int() result: xor-ed the cycle counter, the 
> > pid and a kernel address to it. That strengthened the result in a 
> > pretty practical way (without strengthening the theoretical 
> > randomless - each of those items are considered guessable) and does 
> > so without weakening the entropy of the random pool.
> 
> The trouble is, that thinking completely misses the problem, and I 
> expect that is why we have a problem.  Throwing a bunch of 
> possibly truly random values into the pot for luck is nice.  But 
> you didn't throw in a pseudo random number generator.  An 
> unpredictable sequence that is guaranteed to change from one 
> invocation to the next.

Alas, i did - it got 'reviewed' out of existence ;)

I still have the backups, here's the original exec-shield RNG:

+/*
+ * Get a random word:
+ */
+static inline unsigned int get_random_int(void)
+{
+       unsigned int val = 0;
+
+       if (!exec_shield_randomize)
+               return 0;
+
+#ifdef CONFIG_X86_HAS_TSC
+       rdtscl(val);
+#endif
+       val += current->pid + jiffies + (int)&val;
+
+       /*
+        * Use IP's RNG. It suits our purpose perfectly: it re-keys itself
+        * every second, from the entropy pool (and thus creates a limited
+        * drain on it), and uses halfMD4Transform within the second. We
+        * also spice it with the TSC (if available), jiffies, PID and the
+        * stack address:
+        */
+       return secure_ip_id(val);
+}

I thought that basing it on the networking PRNG is proper design: 
the networking folks have showed it time and again that they care 
about the PRNG not being brute-forceable easily, while still staying 
fast enough.

I added the TSC and a few other pseudo-random details to increase 
complexity of any brute-force attack. (in the hope of rendering it 
less practical, at minimal cost)

> In a very practical sense a pseudo random generator is completely 
> sufficient.  Throwing in a few truly random numbers guards against 
> attacks on the random number generator.
> 
> What we have now is a hash over an a value that changes every 5 
> minutes and some well known values.

Yes.

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Matt Mackall May 5, 2009, 8:22 p.m. UTC | #17
On Tue, May 05, 2009 at 09:52:46PM +0200, Ingo Molnar wrote:
> 
> * Eric W. Biederman <ebiederm@xmission.com> wrote:
> 
> > Ingo Molnar <mingo@elte.hu> writes:
> > 
> > > * Matt Mackall <mpm@selenic.com> wrote:
> > >
> > >> As to what's the appropriate sort of RNG for ASLR to use, finding 
> > >> a balance between too strong and too weak is tricky. [...]
> > >
> > > In exec-shield i mixed 'easily accessible and fast' semi-random 
> > > state to the get_random_int() result: xor-ed the cycle counter, the 
> > > pid and a kernel address to it. That strengthened the result in a 
> > > pretty practical way (without strengthening the theoretical 
> > > randomless - each of those items are considered guessable) and does 
> > > so without weakening the entropy of the random pool.
> > 
> > The trouble is, that thinking completely misses the problem, and I 
> > expect that is why we have a problem.  Throwing a bunch of 
> > possibly truly random values into the pot for luck is nice.  But 
> > you didn't throw in a pseudo random number generator.  An 
> > unpredictable sequence that is guaranteed to change from one 
> > invocation to the next.
> 
> Alas, i did - it got 'reviewed' out of existence ;)
> 
> I still have the backups, here's the original exec-shield RNG:
> 
> +/*
> + * Get a random word:
> + */
> +static inline unsigned int get_random_int(void)
> +{
> +       unsigned int val = 0;
> +
> +       if (!exec_shield_randomize)
> +               return 0;
> +
> +#ifdef CONFIG_X86_HAS_TSC
> +       rdtscl(val);
> +#endif
> +       val += current->pid + jiffies + (int)&val;
> +
> +       /*
> +        * Use IP's RNG. It suits our purpose perfectly: it re-keys itself
> +        * every second, from the entropy pool (and thus creates a limited
> +        * drain on it), and uses halfMD4Transform within the second. We
> +        * also spice it with the TSC (if available), jiffies, PID and the
> +        * stack address:
> +        */
> +       return secure_ip_id(val);
> +}

Ingo, what are you on about? On every architecture but X86 with TSC
this is identical to the broken code.

TSC only helps matters slightly - the timescales involved in process
creation are very short and we can probably brute-force attack it with
a useful probability of success. ie:

a) record TSC
b) fork target process
c) record TSC
d) guess TSC value 
e) attempt attack
f) repeat
Eric W. Biederman May 5, 2009, 9:20 p.m. UTC | #18
Matt Mackall <mpm@selenic.com> writes:

> On Tue, May 05, 2009 at 09:52:46PM +0200, Ingo Molnar wrote:
>> 
>> * Eric W. Biederman <ebiederm@xmission.com> wrote:
>> 
>> > Ingo Molnar <mingo@elte.hu> writes:
>> > 
>> > > * Matt Mackall <mpm@selenic.com> wrote:
>> > >
>> > >> As to what's the appropriate sort of RNG for ASLR to use, finding 
>> > >> a balance between too strong and too weak is tricky. [...]
>> > >
>> > > In exec-shield i mixed 'easily accessible and fast' semi-random 
>> > > state to the get_random_int() result: xor-ed the cycle counter, the 
>> > > pid and a kernel address to it. That strengthened the result in a 
>> > > pretty practical way (without strengthening the theoretical 
>> > > randomless - each of those items are considered guessable) and does 
>> > > so without weakening the entropy of the random pool.
>> > 
>> > The trouble is, that thinking completely misses the problem, and I 
>> > expect that is why we have a problem.  Throwing a bunch of 
>> > possibly truly random values into the pot for luck is nice.  But 
>> > you didn't throw in a pseudo random number generator.  An 
>> > unpredictable sequence that is guaranteed to change from one 
>> > invocation to the next.
>> 
>> Alas, i did - it got 'reviewed' out of existence ;)
>> 
>> I still have the backups, here's the original exec-shield RNG:
>> 
>> +/*
>> + * Get a random word:
>> + */
>> +static inline unsigned int get_random_int(void)
>> +{
>> +       unsigned int val = 0;
>> +
>> +       if (!exec_shield_randomize)
>> +               return 0;
>> +
>> +#ifdef CONFIG_X86_HAS_TSC
>> +       rdtscl(val);
>> +#endif
>> +       val += current->pid + jiffies + (int)&val;
>> +
>> +       /*
>> +        * Use IP's RNG. It suits our purpose perfectly: it re-keys itself
>> +        * every second, from the entropy pool (and thus creates a limited
>> +        * drain on it), and uses halfMD4Transform within the second. We
>> +        * also spice it with the TSC (if available), jiffies, PID and the
>> +        * stack address:
>> +        */
>> +       return secure_ip_id(val);
>> +}
>
> Ingo, what are you on about? On every architecture but X86 with TSC
> this is identical to the broken code.

Well it has the val = (int)&val bit. 

However you are quite right the original get_random_int does not have
any state that persists from one call to the next.  Ingo you failed to
copy that from the way ip uses secure_ip_id.

Which ultimately means get_random_int has never had a pseudo random
number generator in it.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
tip-bot for Ingo Molnar May 6, 2009, 10:30 a.m. UTC | #19
* Matt Mackall <mpm@selenic.com> wrote:

> On Tue, May 05, 2009 at 09:52:46PM +0200, Ingo Molnar wrote:
> > 
> > * Eric W. Biederman <ebiederm@xmission.com> wrote:
> > 
> > > Ingo Molnar <mingo@elte.hu> writes:
> > > 
> > > > * Matt Mackall <mpm@selenic.com> wrote:
> > > >
> > > >> As to what's the appropriate sort of RNG for ASLR to use, finding 
> > > >> a balance between too strong and too weak is tricky. [...]
> > > >
> > > > In exec-shield i mixed 'easily accessible and fast' semi-random 
> > > > state to the get_random_int() result: xor-ed the cycle counter, the 
> > > > pid and a kernel address to it. That strengthened the result in a 
> > > > pretty practical way (without strengthening the theoretical 
> > > > randomless - each of those items are considered guessable) and does 
> > > > so without weakening the entropy of the random pool.
> > > 
> > > The trouble is, that thinking completely misses the problem, and I 
> > > expect that is why we have a problem.  Throwing a bunch of 
> > > possibly truly random values into the pot for luck is nice.  But 
> > > you didn't throw in a pseudo random number generator.  An 
> > > unpredictable sequence that is guaranteed to change from one 
> > > invocation to the next.
> > 
> > Alas, i did - it got 'reviewed' out of existence ;)
> > 
> > I still have the backups, here's the original exec-shield RNG:
> > 
> > +/*
> > + * Get a random word:
> > + */
> > +static inline unsigned int get_random_int(void)
> > +{
> > +       unsigned int val = 0;
> > +
> > +       if (!exec_shield_randomize)
> > +               return 0;
> > +
> > +#ifdef CONFIG_X86_HAS_TSC
> > +       rdtscl(val);
> > +#endif
> > +       val += current->pid + jiffies + (int)&val;
> > +
> > +       /*
> > +        * Use IP's RNG. It suits our purpose perfectly: it re-keys itself
> > +        * every second, from the entropy pool (and thus creates a limited
> > +        * drain on it), and uses halfMD4Transform within the second. We
> > +        * also spice it with the TSC (if available), jiffies, PID and the
> > +        * stack address:
> > +        */
> > +       return secure_ip_id(val);
> > +}
> 
> Ingo, what are you on about? On every architecture but X86 with 
> TSC this is identical to the broken code.

Note that this was the exec-shield arch/*86/mm/mmap.c code.

(Also, obviously "only" covering 95% of the Linux systems has its 
use as well. Most other architectures have their own cycle counters 
as well.)

> TSC only helps matters slightly - the timescales involved in 
> process creation are very short and we can probably brute-force 
> attack it with a useful probability of success. ie:
> 
> a) record TSC
> b) fork target process
> c) record TSC
> d) guess TSC value 
> e) attempt attack
> f) repeat

Try that one day and see how much jitter there is in that sequence, 
even on a completely quiescent system.

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
tip-bot for Ingo Molnar May 6, 2009, 10:33 a.m. UTC | #20
* Eric W. Biederman <ebiederm@xmission.com> wrote:

> Matt Mackall <mpm@selenic.com> writes:
> 
> > On Tue, May 05, 2009 at 09:52:46PM +0200, Ingo Molnar wrote:
> >> 
> >> * Eric W. Biederman <ebiederm@xmission.com> wrote:
> >> 
> >> > Ingo Molnar <mingo@elte.hu> writes:
> >> > 
> >> > > * Matt Mackall <mpm@selenic.com> wrote:
> >> > >
> >> > >> As to what's the appropriate sort of RNG for ASLR to use, finding 
> >> > >> a balance between too strong and too weak is tricky. [...]
> >> > >
> >> > > In exec-shield i mixed 'easily accessible and fast' semi-random 
> >> > > state to the get_random_int() result: xor-ed the cycle counter, the 
> >> > > pid and a kernel address to it. That strengthened the result in a 
> >> > > pretty practical way (without strengthening the theoretical 
> >> > > randomless - each of those items are considered guessable) and does 
> >> > > so without weakening the entropy of the random pool.
> >> > 
> >> > The trouble is, that thinking completely misses the problem, and I 
> >> > expect that is why we have a problem.  Throwing a bunch of 
> >> > possibly truly random values into the pot for luck is nice.  But 
> >> > you didn't throw in a pseudo random number generator.  An 
> >> > unpredictable sequence that is guaranteed to change from one 
> >> > invocation to the next.
> >> 
> >> Alas, i did - it got 'reviewed' out of existence ;)
> >> 
> >> I still have the backups, here's the original exec-shield RNG:
> >> 
> >> +/*
> >> + * Get a random word:
> >> + */
> >> +static inline unsigned int get_random_int(void)
> >> +{
> >> +       unsigned int val = 0;
> >> +
> >> +       if (!exec_shield_randomize)
> >> +               return 0;
> >> +
> >> +#ifdef CONFIG_X86_HAS_TSC
> >> +       rdtscl(val);
> >> +#endif
> >> +       val += current->pid + jiffies + (int)&val;
> >> +
> >> +       /*
> >> +        * Use IP's RNG. It suits our purpose perfectly: it re-keys itself
> >> +        * every second, from the entropy pool (and thus creates a limited
> >> +        * drain on it), and uses halfMD4Transform within the second. We
> >> +        * also spice it with the TSC (if available), jiffies, PID and the
> >> +        * stack address:
> >> +        */
> >> +       return secure_ip_id(val);
> >> +}
> >
> > Ingo, what are you on about? On every architecture but X86 with TSC
> > this is identical to the broken code.
> 
> Well it has the val = (int)&val bit. 
> 
> However you are quite right the original get_random_int does not 
> have any state that persists from one call to the next.  Ingo you 
> failed to copy that from the way ip uses secure_ip_id.
> 
> Which ultimately means get_random_int has never had a pseudo 
> random number generator in it.

Indeed, good point. But, practical randomness was still saved by the 
other layers. Which what it is really about: adding different 
sources of semi-random values really increases attack complexity, 
and is cheap.

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Matt Mackall May 6, 2009, 4:25 p.m. UTC | #21
On Wed, May 06, 2009 at 12:30:34PM +0200, Ingo Molnar wrote:
> (Also, obviously "only" covering 95% of the Linux systems has its 
> use as well. Most other architectures have their own cycle counters 
> as well.)

X86 might be 95% of desktop. But it's a small fraction of Linux
systems once you count cell phones, video players, TVs, cameras, GPS
devices, cars, routers, etc. almost none of which are x86-based. In
fact, just Linux cell phones (with about an 8% share of a 1.2billion
devices per year market) dwarf Linux desktops (maybe 5% of a
200m/y market).
Linus Torvalds May 6, 2009, 4:48 p.m. UTC | #22
On Wed, 6 May 2009, Matt Mackall wrote:

> On Wed, May 06, 2009 at 12:30:34PM +0200, Ingo Molnar wrote:
> > (Also, obviously "only" covering 95% of the Linux systems has its 
> > use as well. Most other architectures have their own cycle counters 
> > as well.)
> 
> X86 might be 95% of desktop. But it's a small fraction of Linux
> systems once you count cell phones, video players, TVs, cameras, GPS
> devices, cars, routers, etc. almost none of which are x86-based. In
> fact, just Linux cell phones (with about an 8% share of a 1.2billion
> devices per year market) dwarf Linux desktops (maybe 5% of a
> 200m/y market).

Matt, are you willing to ack my suggested patch which adds history to the 
mix? Did somebody test that? I have this memory of there being an 
"exploit" program to show the non-randomness of the values, but I can't 
recall details, and would really want to get a second opinion from 
somebody who cares about PRNG's.

			Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Matt Mackall May 6, 2009, 5:57 p.m. UTC | #23
On Wed, May 06, 2009 at 09:48:20AM -0700, Linus Torvalds wrote:
> 
> Matt, are you willing to ack my suggested patch which adds history to the 
> mix? Did somebody test that? I have this memory of there being an 
> "exploit" program to show the non-randomness of the values, but I can't 
> recall details, and would really want to get a second opinion from 
> somebody who cares about PRNG's.

I still don't like it. I bounced it off some folks on the adversarial
side of things and they didn't think it looked strong enough either.
Full MD5 collisions can be generated about as fast as they can be
checked, which makes _reduced strength_ MD4 not much better than an
LFSR in terms of attack potential. So I suggest we either:

a) take my original patch 
b) respin your patch using at least SHA1 rather than halfMD4 and
changing the name to get_random_u32

If you'd prefer (b), I'll do the legwork.
tip-bot for Ingo Molnar May 6, 2009, 8:25 p.m. UTC | #24
* Matt Mackall <mpm@selenic.com> wrote:

> On Wed, May 06, 2009 at 12:30:34PM +0200, Ingo Molnar wrote:
> > (Also, obviously "only" covering 95% of the Linux systems has its 
> > use as well. Most other architectures have their own cycle counters 
> > as well.)
> 
> X86 might be 95% of desktop. But it's a small fraction of Linux 
> systems once you count cell phones, video players, TVs, cameras, 
> GPS devices, cars, routers, etc. almost none of which are 
> x86-based. In fact, just Linux cell phones (with about an 8% share 
> of a 1.2billion devices per year market) dwarf Linux desktops 
> (maybe 5% of a 200m/y market).

Firstly, the cycle counter is just one out of several layers there. 
So it's a hyperbole to suggest that i'm somehow not caring about 
architectures that dont have a cycle counter. I'm simply making use 
of a cheaply accessed and fast-changing variable on hw that has it.

Also, are those systems really going to be attacked locally, 
brute-forcing a PRNG? Servers and desktops are the more likely 
targets. They both have the necessary computing power to run 
statistical analysis locally fast enough and have an actual value to 
be attacked.

And, even if we ignored those factors, ad argumendo, you would still 
be wrong IMHO: what matters really in this context isnt even any 
artificial 'share' metric - but the people willing to improve and 
fix the upstream kernel, and the reasons why they do that, and the 
platforms they use.

And amongst our contributors and testers, willing to run and improve 
the latest upstream kernel, x86 has a larger than 95% share. Look at 
kerneloops.org stats. Or bugzilla. Or lkml.

If embedded matters that much, make it show up as a real factor on 
those upstream forums. Lets call this Ingo's Law: if something 
doesnt improve the upstream kernel, directly or indirectly, it does 
not exist ;-)

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Matt Mackall May 6, 2009, 8:52 p.m. UTC | #25
On Wed, May 06, 2009 at 10:25:17PM +0200, Ingo Molnar wrote:
> 
> * Matt Mackall <mpm@selenic.com> wrote:
> 
> > On Wed, May 06, 2009 at 12:30:34PM +0200, Ingo Molnar wrote:
> > > (Also, obviously "only" covering 95% of the Linux systems has its 
> > > use as well. Most other architectures have their own cycle counters 
> > > as well.)
> > 
> > X86 might be 95% of desktop. But it's a small fraction of Linux 
> > systems once you count cell phones, video players, TVs, cameras, 
> > GPS devices, cars, routers, etc. almost none of which are 
> > x86-based. In fact, just Linux cell phones (with about an 8% share 
> > of a 1.2billion devices per year market) dwarf Linux desktops 
> > (maybe 5% of a 200m/y market).
> 
> Firstly, the cycle counter is just one out of several layers there. 
> So it's a hyperbole to suggest that i'm somehow not caring about 
> architectures that dont have a cycle counter. I'm simply making use 
> of a cheaply accessed and fast-changing variable on hw that has it.

Whatever, I've never argued against TSC being beneficial. But it sure
as hell is not sufficient. Your original claim that this attack was
not possible in your original code: still bogus.
 
> Also, are those systems really going to be attacked locally, 
> brute-forcing a PRNG? 

Yes[1], even though my point was mostly to shoot down your bogus
statistic for reasons unrelated to this discussion. If you want to
make a new claim that '95% of Linux systems interesting to Ingo are
x86', I won't argue with that.

[1] 95% of security holes are caused by developer failures of imagination.
Matt Mackall May 7, 2009, 12:50 a.m. UTC | #26
On Wed, May 06, 2009 at 12:57:17PM -0500, Matt Mackall wrote:
> On Wed, May 06, 2009 at 09:48:20AM -0700, Linus Torvalds wrote:
> > 
> > Matt, are you willing to ack my suggested patch which adds history to the 
> > mix? Did somebody test that? I have this memory of there being an 
> > "exploit" program to show the non-randomness of the values, but I can't 
> > recall details, and would really want to get a second opinion from 
> > somebody who cares about PRNG's.
> 
> I still don't like it. I bounced it off some folks on the adversarial
> side of things and they didn't think it looked strong enough either.
> Full MD5 collisions can be generated about as fast as they can be
> checked, which makes _reduced strength_ MD4 not much better than an
> LFSR in terms of attack potential. So I suggest we either:
> 
> a) take my original patch 
> b) respin your patch using at least SHA1 rather than halfMD4 and
> changing the name to get_random_u32
> 
> If you'd prefer (b), I'll do the legwork.

I've done some basic benchmarks on the primitives here in userspace:

halfMD4 get_random_int: about .326us per call or 12.2MB/s
sha1 get_random_int: about .660us per call or 6.1MB/s
dd /dev/urandom: 3.6MB/s

So I think the SHA1 solution is quite competitive on the performance
front with far fewer concerns about its strength. I'll spin a proper
patch tomorrow.
tip-bot for Ingo Molnar May 7, 2009, 3:02 p.m. UTC | #27
* Matt Mackall <mpm@selenic.com> wrote:

> On Wed, May 06, 2009 at 12:57:17PM -0500, Matt Mackall wrote:
> > On Wed, May 06, 2009 at 09:48:20AM -0700, Linus Torvalds wrote:
> > > 
> > > Matt, are you willing to ack my suggested patch which adds history to the 
> > > mix? Did somebody test that? I have this memory of there being an 
> > > "exploit" program to show the non-randomness of the values, but I can't 
> > > recall details, and would really want to get a second opinion from 
> > > somebody who cares about PRNG's.
> > 
> > I still don't like it. I bounced it off some folks on the adversarial
> > side of things and they didn't think it looked strong enough either.
> > Full MD5 collisions can be generated about as fast as they can be
> > checked, which makes _reduced strength_ MD4 not much better than an
> > LFSR in terms of attack potential. So I suggest we either:
> > 
> > a) take my original patch 
> > b) respin your patch using at least SHA1 rather than halfMD4 and
> > changing the name to get_random_u32
> > 
> > If you'd prefer (b), I'll do the legwork.
> 
> I've done some basic benchmarks on the primitives here in userspace:
> 
> halfMD4 get_random_int: about .326us per call or 12.2MB/s
> sha1 get_random_int: about .660us per call or 6.1MB/s
> dd /dev/urandom: 3.6MB/s
> 
> So I think the SHA1 solution is quite competitive on the 
> performance front with far fewer concerns about its strength. I'll 
> spin a proper patch tomorrow.

Hm, i'm not happy about that at all - that's like a ~2000 cycles 
cost, probably fully cached! Do you have cache-cold numbers as well?

We have:

 aldebaran:~/l> ./lat_proc fork
 Process fork+exit: 61.7865 microseconds

So what you are talking about is about 1% of our fork performance! 
And fork is a pretty fat operation - it could be much worse for 
something ligther.

As i mentioned it in the previous mail, i'd _really_ like to hear 
your thread model and attack vector description. Does this overhead 
justify the threat? Your change will only result in get_random_int() 
not being considered fast anymore.

So unless a strong reason to do otherwise, i'd really prefer Linus's 
modified patch, the one i tested and sent out yesterday. (attached 
below again)

	Ingo

----- Forwarded message from Ingo Molnar <mingo@elte.hu> -----

Date: Wed, 6 May 2009 22:09:54 +0200
From: Ingo Molnar <mingo@elte.hu>
To: Linus Torvalds <torvalds@linux-foundation.org>
Subject: [patch] random: make get_random_int() more random
Cc: Matt Mackall <mpm@selenic.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Arjan van de Ven <arjan@infradead.org>, Jake Edge <jake@lwn.net>,
	security@kernel.org,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	James Morris <jmorris@namei.org>,
	linux-security-module@vger.kernel.org,
	Eric Paris <eparis@redhat.com>, Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Roland McGrath <roland@redhat.com>, mingo@redhat.com,
	Andrew Morton <akpm@linux-foundation.org>, Greg KH <greg@kroah.com>,
	Dave Jones <davej@redhat.com>


* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> On Wed, 6 May 2009, Matt Mackall wrote:
> 
> > On Wed, May 06, 2009 at 12:30:34PM +0200, Ingo Molnar wrote:
>
> > > (Also, obviously "only" covering 95% of the Linux systems has 
> > > its use as well. Most other architectures have their own cycle 
> > > counters as well.)
> > 
> > X86 might be 95% of desktop. But it's a small fraction of Linux 
> > systems once you count cell phones, video players, TVs, cameras, 
> > GPS devices, cars, routers, etc. almost none of which are 
> > x86-based. In fact, just Linux cell phones (with about an 8% 
> > share of a 1.2billion devices per year market) dwarf Linux 
> > desktops (maybe 5% of a 200m/y market).
> 
> Matt, are you willing to ack my suggested patch which adds history 
> to the mix? Did somebody test that? I have this memory of there 
> being an "exploit" program to show the non-randomness of the 
> values, but I can't recall details, and would really want to get a 
> second opinion from somebody who cares about PRNG's.

I tested it, and besides booting up fine, i also tested the 
get_random_int() randomness. I did this by adding this quick 
trace_printk() line:

   trace_printk("get_random_int(): %08x\n", get_random_int());

to sys_prctl() and triggered sys_prctl() in a loop, which gave a 
list of get_random_int() numbers:

# tracer: nop
#
#           TASK-PID    CPU#    TIMESTAMP  FUNCTION
#              | |	 |          |         |
           <...>-6288  [000]   618.151323: sys_prctl: get_random_int(): 2e927f66
           <...>-6290  [000]   618.152924: sys_prctl: get_random_int(): d210df1f
           <...>-6293  [000]   618.155326: sys_prctl: get_random_int(): 753ad860
           <...>-6295  [000]   618.156939: sys_prctl: get_random_int(): c74d935f
           <...>-6298  [000]   618.159334: sys_prctl: get_random_int(): bb4e7597
           <...>-6300  [000]   618.160936: sys_prctl: get_random_int(): b0119885
           <...>-6303  [000]   618.163331: sys_prctl: get_random_int(): 093f5c70

Full list attached.

I then wrote a quick script to write those numbers out into a 
continous binary file (result also attached).

I then ran the FIPS randomness test over the first 20,000 bits [2.5K 
data], which it passed:

rngtest: bits received from input: 20064
rngtest: bits sent to output: 20000
rngtest: FIPS 140-2 successes: 1
rngtest: FIPS 140-2 failures: 0
rngtest: FIPS 140-2(2001-10-10) Monobit: 0
rngtest: FIPS 140-2(2001-10-10) Poker: 0
rngtest: FIPS 140-2(2001-10-10) Runs: 0
rngtest: FIPS 140-2(2001-10-10) Long run: 0
rngtest: FIPS 140-2(2001-10-10) Continuous run: 0
rngtest: input channel speed: (min=3.104; avg=3.104; max=3.104)Gibits/s
rngtest: FIPS tests speed: (min=110.892; avg=110.892; max=110.892)Mibits/s
rngtest: output channel speed: (min=544.957; avg=544.957; max=544.957)Mibits/s
rngtest: Program run time: 294 microseconds

So it looks good enough - that's a sample of 800+ pseudo-random 
integers.

I also modified your patch to include two more random sources, 
get_cycles() [which all 22 architectures define - albeit not all 
have the hw to actually do fine-grained cycle counts - so for some 
it's always-zero or a low-res return value], plus a kernel stack 
address.

The relevant line is:

+	hash[0] += current->pid + jiffies + get_cycles() + (int)(long)&ret;

The argument is that the more layers we have here, the harder it 
becomes to _reliably_ attack a given system. Works-100%-sure is an 
important prize for certain types of attackers - and with the cycle 
counter, jiffies, PID and a kernel address all mixed in that becomes 
quite hard to achieve.

I tested this too - it also results in good random numbers. Find the 
patch below.

	Ingo


--------------->
Subject: random: make get_random_int() more random
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Tue, 5 May 2009 08:17:43 -0700 (PDT)

It's a really simple patch that basically just open-codes the current
"secure_ip_id()" call, but when open-coding it we now use a _static_
hashing area, so that it gets updated every time.

And to make sure somebody can't just start from the same original seed of
all-zeroes, and then do the "half_md4_transform()" over and over until
they get the same sequence as the kernel has, each iteration also mixes in
the same old "current->pid + jiffies" we used - so we should now have a
regular strong pseudo-number generator, but we also have one that doesn't
have a single seed.

Note: the "pid + jiffies" is just meant to be a tiny tiny bit of noise. It
has no real meaning. It could be anything. I just picked the previous
seed, it's just that now we keep the state in between calls and that will
feed into the next result, and that should make all the difference.

I made that hash be a per-cpu data just to avoid cache-line ping-pong:
having multiple CPU's write to the same data would be fine for randomness,
and add yet another layer of chaos to it, but since get_random_int() is
supposed to be a fast interface I did it that way instead. I considered
using "__raw_get_cpu_var()" to avoid any preemption overhead while still
getting the hash be _mostly_ ping-pong free, but in the end good taste won
out.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/char/random.c |   19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

Index: linux/drivers/char/random.c
===================================================================
--- linux.orig/drivers/char/random.c
+++ linux/drivers/char/random.c
@@ -1665,15 +1665,20 @@ EXPORT_SYMBOL(secure_dccp_sequence_numbe
  * value is not cryptographically secure but for several uses the cost of
  * depleting entropy is too high
  */
+DEFINE_PER_CPU(__u32 [4], get_random_int_hash);
 unsigned int get_random_int(void)
 {
-	/*
-	 * Use IP's RNG. It suits our purpose perfectly: it re-keys itself
-	 * every second, from the entropy pool (and thus creates a limited
-	 * drain on it), and uses halfMD4Transform within the second. We
-	 * also mix it with jiffies and the PID:
-	 */
-	return secure_ip_id((__force __be32)(current->pid + jiffies));
+	struct keydata *keyptr;
+	__u32 *hash = get_cpu_var(get_random_int_hash);
+	int ret;
+
+	keyptr = get_keyptr();
+	hash[0] += current->pid + jiffies + get_cycles() + (int)(long)&ret;
+
+	ret = half_md4_transform(hash, keyptr->secret);
+	put_cpu_var(get_random_int_hash);
+
+	return ret;
 }
 
 /*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Florian Weimer May 7, 2009, 3:16 p.m. UTC | #28
* Matt Mackall:

> On Wed, May 06, 2009 at 09:48:20AM -0700, Linus Torvalds wrote:
>> 
>> Matt, are you willing to ack my suggested patch which adds history to the 
>> mix? Did somebody test that? I have this memory of there being an 
>> "exploit" program to show the non-randomness of the values, but I can't 
>> recall details, and would really want to get a second opinion from 
>> somebody who cares about PRNG's.
>
> I still don't like it. I bounced it off some folks on the adversarial
> side of things and they didn't think it looked strong enough either.
> Full MD5 collisions can be generated about as fast as they can be
> checked, which makes _reduced strength_ MD4 not much better than an
> LFSR in terms of attack potential.

Well, with periodic reseeding, even that shouldn't be a problem.  You
don't need collision resistance at all, so those MD5 attacks don't
tell you anything about the difficulty of state recovery/prediction
attacks on your variant.  (The trouble with hash functions is that
they haven't got any secrets to work from.  With seeded PRNGs, this is
obviously different.)

On the other hand, most people who need a quick, unpredictable source
of randomness seem to use RC4 with a random key initialized from a
more costly source.  (If you're paranoid, you should discard the first
few hundred bytes.)  The nice thing is that you can use a well-tested
primitive, unchanged, so it's easier to avoid nasty suprises.

Oh, and you should really, really ditch that Tausworthe generator (in
lib/random32.c).
Matt Mackall May 7, 2009, 4:55 p.m. UTC | #29
On Thu, May 07, 2009 at 05:16:27PM +0200, Florian Weimer wrote:
> * Matt Mackall:
> 
> > On Wed, May 06, 2009 at 09:48:20AM -0700, Linus Torvalds wrote:
> >> 
> >> Matt, are you willing to ack my suggested patch which adds history to the 
> >> mix? Did somebody test that? I have this memory of there being an 
> >> "exploit" program to show the non-randomness of the values, but I can't 
> >> recall details, and would really want to get a second opinion from 
> >> somebody who cares about PRNG's.
> >
> > I still don't like it. I bounced it off some folks on the adversarial
> > side of things and they didn't think it looked strong enough either.
> > Full MD5 collisions can be generated about as fast as they can be
> > checked, which makes _reduced strength_ MD4 not much better than an
> > LFSR in terms of attack potential.
> 
> Well, with periodic reseeding, even that shouldn't be a problem.  You
> don't need collision resistance at all, so those MD5 attacks don't
> tell you anything about the difficulty of state recovery/prediction
> attacks on your variant.

It's *not* MD5. It's a reduced-round MD4. And MD4 is already many
orders of magnitude weaker than MD5. It's so weak in fact that
collisions can be generated in O(1)[1]. Hard to get much weaker than
that, except by, say, using something like our reduced-round variant.

It's not much of a stretch of the imagination to think that such an
amazingly weak hash might reveal our hidden state quite rapidly,
especially when used in a feedback mode.

[1] http://eprint.iacr.org/2005/151.pdf

We have a better hash function handy, and it's only takes twice as long.

> On the other hand, most people who need a quick, unpredictable source
> of randomness seem to use RC4 with a random key initialized from a
> more costly source.

Using a stream cipher is a fine idea. Ted and I have recently
discussed adding this as a layer to the stock RNG. We haven't used it
historically because of a) export restrictions and b) unsuitability of
the cryptoapi interface.

> Oh, and you should really, really ditch that Tausworthe generator (in
> lib/random32.c).

I'm not responsible for that particular bad idea.
Linus Torvalds May 7, 2009, 5:53 p.m. UTC | #30
On Thu, 7 May 2009, Matt Mackall wrote:
> 
> We have a better hash function handy, and it's only takes twice as long.

Matt, I really don't like your notion of "only twice as long".

I mean, really. In the kernel, we tend to never even talk about how many 
_times_ slower something is. We talk about cycles or small percentages.

The fact is, the current "get_random_int()" is a joke, and will return the 
same value over and over again for long stretches of time. I mean, really. 
Even people who don't care a lot would expect more than _that_ out of a 
PRNG.

And quite frankly, a lot of the users of get_random_int() probably use it 
not as some crypto function, but as a replacement for not having to write 
their own copy of some standard PRNG linear congruential generator.

I mean, really. The virtual address randomization was never meant to be 
"cryptographically secure" in that sense. Dammit, look at the code: it 
only takes something like 8 bits of the results _anyway_.

In other words, YOUR WHOLE ARGUMENT IS TOTALLY INSANE. You talk about 
"cryptographically secure hashes" for some 8-bit value. Listen to 
yourself. At that point, any cryptographer will just ridicule you. There's 
no point in trying to break the randomness, because you'll be much better 
off just trying a lot of different values.

So Matt, get with the program already. Don't ignore the performance 
argument by saying "it's only twice as slow". Admit it - that's just 
idiotic.

If somebody _really_ wants true randomness, teach them to use 
"get_random_bytes()" by all means. 

			Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Matt Mackall May 7, 2009, 6:14 p.m. UTC | #31
On Thu, May 07, 2009 at 05:02:31PM +0200, Ingo Molnar wrote:
> 
> * Matt Mackall <mpm@selenic.com> wrote:
> 
> > On Wed, May 06, 2009 at 12:57:17PM -0500, Matt Mackall wrote:
> > > On Wed, May 06, 2009 at 09:48:20AM -0700, Linus Torvalds wrote:
> > > > 
> > > > Matt, are you willing to ack my suggested patch which adds history to the 
> > > > mix? Did somebody test that? I have this memory of there being an 
> > > > "exploit" program to show the non-randomness of the values, but I can't 
> > > > recall details, and would really want to get a second opinion from 
> > > > somebody who cares about PRNG's.
> > > 
> > > I still don't like it. I bounced it off some folks on the adversarial
> > > side of things and they didn't think it looked strong enough either.
> > > Full MD5 collisions can be generated about as fast as they can be
> > > checked, which makes _reduced strength_ MD4 not much better than an
> > > LFSR in terms of attack potential. So I suggest we either:
> > > 
> > > a) take my original patch 
> > > b) respin your patch using at least SHA1 rather than halfMD4 and
> > > changing the name to get_random_u32
> > > 
> > > If you'd prefer (b), I'll do the legwork.
> > 
> > I've done some basic benchmarks on the primitives here in userspace:
> > 
> > halfMD4 get_random_int: about .326us per call or 12.2MB/s
> > sha1 get_random_int: about .660us per call or 6.1MB/s
> > dd /dev/urandom: 3.6MB/s
> > 
> > So I think the SHA1 solution is quite competitive on the 
> > performance front with far fewer concerns about its strength. I'll 
> > spin a proper patch tomorrow.
> 
> Hm, i'm not happy about that at all - that's like a ~2000 cycles 
> cost, probably fully cached! Do you have cache-cold numbers as well?

No, but you're free to benchmark my forthcoming patch. Given that I
haven't heard of anyone complaining about performance regressions in
exec() from the introduction of ASLR (nor can I find any on Google),
I've assumed doubling the RNG overhead would continue to remain in the
noise.

>  aldebaran:~/l> ./lat_proc fork
>  Process fork+exit: 61.7865 microseconds

Uh, what? There's no exec() involved in fork+exit, so hopefully ASLR
doesn't decide to make an appearance.

> As i mentioned it in the previous mail, i'd _really_ like to hear 
> your thread model and attack vector description. Does this overhead 
> justify the threat? Your change will only result in get_random_int() 
> not being considered fast anymore.

My threat model is that someone more clever and with a lot more
expertise attacking systems than either you or me will be able to
leverage the extreme weakness of this hash (O(1) attacks against the
*full* version!) into an attack that incrementally exposes the hidden
RNG state. I've asked a couple such people whether they think that's
likely, and they've said yes.

In fact, it's been known for over a decade that reduced-round MD4 such
as ours is *not one way* and that preimages (aka our hidden state) can
be found in less than an hour on a *mid-90s era PC*:

http://citeseer.ist.psu.edu/old/182935.html

Combine that with greatly improved techniques for attacking hashes in
the MD4 family in the last five years and you're probably looking at
less than a second of CPU time. Combine that with the fact that we're
using the hash in a feedback mode, and things only get easier.

On the question of 'what if we add in the TSC?', I'd first say (a) we
can't and shouldn't assume a TSC is available, though we certainly
should use it if it is. Second I'd say that there are numerous timing
attacks that have been done that suggest that guessing the TSC can be
done with useful probability. For instance, see the branch prediction
attacks against AES and RSA.
tip-bot for Ingo Molnar May 7, 2009, 6:21 p.m. UTC | #32
* Matt Mackall <mpm@selenic.com> wrote:

> >  aldebaran:~/l> ./lat_proc fork
> >  Process fork+exit: 61.7865 microseconds
> 
> Uh, what? There's no exec() involved in fork+exit, so hopefully 
> ASLR doesn't decide to make an appearance.

We use it to seed the per task stackprotector secret. Look for 
get_random_int() in kernel/fork.c.

(
  Now, if get_random_int() slows down we could certainly water that 
  down and have a system-wide secret and some easy and fast 
  long-cycle permutation code to make it not so easy to figure out 
  the core secret from a kernel crash signature.

  [ Alas, that might be worth doing in any case - to not have 
    get_random_int() in the clone() / pthread_create() fastpath. ]

  We really need a design decision there - if get_random_int() is 
  supposed to be a mathematically safe hash, ignoring the physics of 
  the world, then we need a separate get_random_int_fast() API or 
  so. All current users of get_random_int() will evaporate as well. 
)

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
tip-bot for Ingo Molnar May 7, 2009, 6:41 p.m. UTC | #33
* Matt Mackall <mpm@selenic.com> wrote:

> > As i mentioned it in the previous mail, i'd _really_ like to 
> > hear your thread model and attack vector description. Does this 
> > overhead justify the threat? Your change will only result in 
> > get_random_int() not being considered fast anymore.
> 
> My threat model is that someone more clever and with a lot more 
> expertise attacking systems than either you or me will be able to 
> leverage the extreme weakness of this hash (O(1) attacks against 
> the *full* version!) into an attack that incrementally exposes the 
> hidden RNG state. I've asked a couple such people whether they 
> think that's likely, and they've said yes.

My question was whether the variant laced with the cycle counter 
could be exposable.

I'd say that's almost impossible physically, in all but the most 
trivial cases. Yes, you can do it with a static PRNG and a static 
pool, but neither pool is static in reality (irqs are coming in all 
the time) nor is the cycle counter static. The cycle counter will 
mix in things like cache miss delays.

You need a really, really fast probe and a really quiet system to 
pull off a statistical attack like that. In all other realistic 
cases you have to play catch-up with the current state of the pool, 
trying to guess it. And even if you have a reasonably good idea 
about it, there's no _guarantee_ that your guess about the pool's 
state is when a critical context you'd like to attack seeds its ASLR 
or whatever other secret state.

> In fact, it's been known for over a decade that reduced-round MD4 
> such as ours is *not one way* and that preimages (aka our hidden 
> state) can be found in less than an hour on a *mid-90s era PC*:
> 
> http://citeseer.ist.psu.edu/old/182935.html

But that is completely inapposite to the PRNG i posted. Getting to 
preimage in a static MD4 hash where you have a reasonable number of 
probes right after the unknown result is possible.

Eliminating cycle counter noise out of a _static_ secret permutated 
via a PRNG is probably also possible, if the probe can be done 
faster than the natural noise level of the cycle counter is.

_Combining_ the two and attacking _that_ (combined with the addition 
of jiffies and a kernel stack address) - seems close to impossible 
to me. You'd have to guess the precise cycle counter value at the 
point of the hashing, and your samples wont help with that. They 
might give an estimation, but that's really what ASLR is about: the 
moment an attack isnt 100% sure, other measures (such as crash 
monitoring) can save the day.

The really serious attackers avoid uncertainty like the plague. 

It's the same reason why three letter agencies rather drop slam-dunk 
court cases than expose their sources and methods. A failed attack 
against a system exposes the attack and since the attack failed, 
there's no way to erase traces of the attack.

Furthermore, ASLR is mostly about changing the dynamics of worm 
attacks via the network. If an attack has only a 10% chance to 
succeed, that might break the propagation dynamics of a worm. So 
just a handful of bits are enough there.

> Combine that with greatly improved techniques for attacking hashes 
> in the MD4 family in the last five years and you're probably 
> looking at less than a second of CPU time. Combine that with the 
> fact that we're using the hash in a feedback mode, and things only 
> get easier.
> 
> On the question of 'what if we add in the TSC?', I'd first say (a) 
> we can't and shouldn't assume a TSC is available, though we 
> certainly should use it if it is. Second I'd say that there are 
> numerous timing attacks that have been done that suggest that 
> guessing the TSC can be done with useful probability. For 
> instance, see the branch prediction attacks against AES and RSA.

That is something completely different again, and i'm not sure 
whether you are trolling here or not...

Branch prediction attacks against AES and RSA are _completely 
different_ and the TSC connection is only there in name. They are 
about using timing as a _sample_ of the secret state and its effects 
on runtime.

Here the TSC is taken once, and mixed. _This_ TSC is not taken ever 
again, nor saved. There's no statistical method to recover it! You 
might know it up to a certain precision if you are lucky and can run 
right after the call, but a couple of bits of true randomness will 
be there for sure. In most cases more than a coupleof bits - an exec 
takes 200-300 usecs and you have to run on that same CPU - so to get 
a sample you have to go back almost 1 million TSC cycles ... That's 
20 bits of TSC space.

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Matt Mackall May 7, 2009, 6:42 p.m. UTC | #34
On Thu, May 07, 2009 at 10:53:49AM -0700, Linus Torvalds wrote:
> So Matt, get with the program already. Don't ignore the performance 
> argument by saying "it's only twice as slow". Admit it - that's just 
> idiotic.

If you want to take it out of random.c and put it in pinhead-rng.c, be
my guest. I'm not going to put my Acked-by on it.
Matt Mackall May 7, 2009, 7:24 p.m. UTC | #35
On Thu, May 07, 2009 at 08:41:36PM +0200, Ingo Molnar wrote:
> 
> * Matt Mackall <mpm@selenic.com> wrote:
> 
> > > As i mentioned it in the previous mail, i'd _really_ like to 
> > > hear your thread model and attack vector description. Does this 
> > > overhead justify the threat? Your change will only result in 
> > > get_random_int() not being considered fast anymore.
> > 
> > My threat model is that someone more clever and with a lot more 
> > expertise attacking systems than either you or me will be able to 
> > leverage the extreme weakness of this hash (O(1) attacks against 
> > the *full* version!) into an attack that incrementally exposes the 
> > hidden RNG state. I've asked a couple such people whether they 
> > think that's likely, and they've said yes.
> 
> My question was whether the variant laced with the cycle counter 
> could be exposable.

In my world, some machines don't have TSCs, so I think this is the
wrong question to be asking.

Patch
diff mbox series

diff --git a/fs/proc/array.c b/fs/proc/array.c
index 7e4877d..725a650 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -80,6 +80,7 @@ 
 #include <linux/delayacct.h>
 #include <linux/seq_file.h>
 #include <linux/pid_namespace.h>
+#include <linux/ptrace.h>
 #include <linux/tracehook.h>
 
 #include <asm/pgtable.h>
@@ -352,6 +353,7 @@  static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 	char state;
 	pid_t ppid = 0, pgid = -1, sid = -1;
 	int num_threads = 0;
+	int permitted;
 	struct mm_struct *mm;
 	unsigned long long start_time;
 	unsigned long cmin_flt = 0, cmaj_flt = 0;
@@ -364,11 +366,14 @@  static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 
 	state = *get_task_state(task);
 	vsize = eip = esp = 0;
+	permitted = ptrace_may_access(task, PTRACE_MODE_READ);
 	mm = get_task_mm(task);
 	if (mm) {
 		vsize = task_vsize(mm);
-		eip = KSTK_EIP(task);
-		esp = KSTK_ESP(task);
+		if (permitted) {
+			eip = KSTK_EIP(task);
+			esp = KSTK_ESP(task);
+		}
 	}
 
 	get_task_comm(tcomm, task);
@@ -424,7 +429,7 @@  static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 		unlock_task_sighand(task, &flags);
 	}
 
-	if (!whole || num_threads < 2)
+	if (permitted && (!whole || num_threads < 2))
 		wchan = get_wchan(task);
 	if (!whole) {
 		min_flt = task->min_flt;
@@ -476,7 +481,7 @@  static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 		rsslim,
 		mm ? mm->start_code : 0,
 		mm ? mm->end_code : 0,
-		mm ? mm->start_stack : 0,
+		(permitted && mm) ? mm->start_stack : 0,
 		esp,
 		eip,
 		/* The signal information here is obsolete.
diff --git a/fs/proc/base.c b/fs/proc/base.c
index aa763ab..fb45615 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -322,7 +322,10 @@  static int proc_pid_wchan(struct task_struct *task, char *buffer)
 	wchan = get_wchan(task);
 
 	if (lookup_symbol_name(wchan, symname) < 0)
-		return sprintf(buffer, "%lu", wchan);
+		if (!ptrace_may_access(task, PTRACE_MODE_READ))
+			return 0;
+		else
+			return sprintf(buffer, "%lu", wchan);
 	else
 		return sprintf(buffer, "%s", symname);
 }