Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[Dan Rosenberg]

> Actually, this is not even a joke.
> 
> Take a look at how we track what sockets a user wants dumped via
> the inet_diag netlink facility, the socket pointer is used as
> the identification cookie.
> 
> I'm sure we'll now get some more security theatre about how we
> have to undo that too.
> 
> More and more I see this whole idea as extremely rediculious.
> 
> If I can write to or read kernel memory, I can look up the
> sockets, inodes, and whatever else we're currently exposing
> the addresses of.  Even without a symbol table, which is
> readily available, I can easily find the ksymtab and find the
> inode and socket hash table addresses there.
> 
> This whole exercise is closing the barn door after the horses have
> already escaped, and it's causing all kinds of inconveniences
> that we really have no need for.

Of course if you can both write and read to kernel memory, this is a
pointless exercise, but those are not the conditions I am trying to
defend against.  Proactive security assumes that individual bugs will
continue to be found, and takes steps to prevent exploitation of classes
of bugs on a broader scale.  In this case, the goal is for it to be
difficult to exploit arbitrary write bugs WITHOUT having a separate
arbitrary read bug.  Several other steps are being taken in this
direction - there is open discussion about the merits of hiding symbol
information, address space randomization, marking various likely targets
as read-only, and restricting access to debugging information.  However,
none of this really accomplishes much if addresses are still exposed
via /proc.

I'm sorry that you see all this as "security theatre".  It's clear that
there's too much resistance to this effort for it to ever succeed, so
I'm ceasing attempts to get this patch series through.

-Dan

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[Dan Rosenberg]

I am willing to do whatever is necessary to make these changes acceptable.  Are you completely set on not removing these addresses from the output?

-Dan

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[David Miller]

From: "Dan Rosenberg" <drosenberg-PiUznwcHFHrqlBn2x/YWAg@public.gmane.org>
Date: Fri, 12 Nov 2010 02:15:25 +0000

> I am willing to do whatever is necessary to make these changes
> acceptable.  Are you completely set on not removing these addresses
> from the output?

I want whatever you replace it with to be equivalent for
object tracking purposes.

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[Dan Rosenberg]

> I want whatever you replace it with to be equivalent for
> object tracking purposes.

In nearly all of the cases I fixed, the socket inode is already
provided, which serves as a perfectly good unique identifier.  Would you
prefer I include that information twice?

-Dan

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[David Miller]

From: Dan Rosenberg <drosenberg-PiUznwcHFHrqlBn2x/YWAg@public.gmane.org>
Date: Thu, 11 Nov 2010 21:34:29 -0500

> 
>> I want whatever you replace it with to be equivalent for
>> object tracking purposes.
> 
> In nearly all of the cases I fixed, the socket inode is already
> provided, which serves as a perfectly good unique identifier.  Would you
> prefer I include that information twice?

The problem is that the socket inode is not available in a certain
subclass of cases, so the transformation is not equivalent.

Why not attack this at the heart of where your concern is, and hack
the %p format handling to do whatever it is you like instead of
patching code all over the tree?

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[Dan Rosenberg]

> > 
> >> I want whatever you replace it with to be equivalent for
> >> object tracking purposes.
> > 
> > In nearly all of the cases I fixed, the socket inode is already
> > provided, which serves as a perfectly good unique identifier.  Would you
> > prefer I include that information twice?
> 
> The problem is that the socket inode is not available in a certain
> subclass of cases, so the transformation is not equivalent.
> 
> Why not attack this at the heart of where your concern is, and hack
> the %p format handling to do whatever it is you like instead of
> patching code all over the tree?

This has already been suggested, and I agree it is a much better
approach.  If I take this approach, and find some suitable substitute
for those cases where the socket inode is not available, will you
consider these changes?

-Dan

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[David Miller]

From: Dan Rosenberg <drosenberg-PiUznwcHFHrqlBn2x/YWAg@public.gmane.org>
Date: Thu, 11 Nov 2010 21:51:04 -0500

> This has already been suggested, and I agree it is a much better
> approach.  If I take this approach, and find some suitable substitute
> for those cases where the socket inode is not available, will you
> consider these changes?

I will consider an approach where the keys reported allow object
tracking equally as the actual pointers allow for right now.

I've said that this is my criteria about 3 times now.

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[Eric Dumazet]

Le jeudi 11 novembre 2010 à 21:34 -0500, Dan Rosenberg a écrit :
> > I want whatever you replace it with to be equivalent for
> > object tracking purposes.
> 
> In nearly all of the cases I fixed, the socket inode is already
> provided, which serves as a perfectly good unique identifier.  Would you
> prefer I include that information twice?

Oh well. Please read this answer carefuly.

Some facts to feed your next patch. I am very pleased you changed your
mind and that we keep useful information in kernel log.

1) Inode numbers are not guaranteed to be unique. Its a 32bit seq
number, and we dont check another socket inode use the same inode number
(after 2^32 allocations it can happens)

2) /proc/net/ files can deliver same "line" of information several
times, because of their implementation.

3) Because of SLAB_DESTROY_BY_RCU, same 'kernel socket pointer' can be
seen several times in /proc/net/tcp & /proc/net/udp, but really on
different "sockets"

4) Some good applications use both the socket pointer and inode number
(tuple) to filter out the [2] problem. Dont break them, please ?
Anything that might break an application must be at the very least
tunable.

In my opinion, a good thing would be :

- Use a special printf() format , aka "secure pointer", as Thomas
suggested.

- Make sure you print different opaque values for two different kernel
pointers. This is mandatory.

- Make sure the NULL pointer stay as a NULL pointer to not let the
hostile user know your secret, and to ease debugging stuff.

- Have security experts advice to chose a nice crypto function, maybe
jenkin hash. Not too slow would be nice.


static unsigned long securize_kpointers_rnd;

At boot time, stick a random value in this variable.
(Maybe make sure the 5 low order bits are 0)

unsigned long opacify_kptr(unsigned long ptr)
{
	if (ptr == 0)
		return ptr;
	if (capable(CAP_NET_ADMIN))
		return ptr;

	return some_crypto_hash(ptr, &securize_kpointers_rnd);
}

At least, use a central point, so that we can easily add/change the
logic if needed.

Please provide this patch in kernel/printk.c for initial review, then
once everybody is OK, you can send one patch for net tree.

No need to send 10 patches if we dont agree on the general principle.

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[Dan Rosenberg]

> 
> 1) Inode numbers are not guaranteed to be unique. Its a 32bit seq
> number, and we dont check another socket inode use the same inode number
> (after 2^32 allocations it can happens)
> 

Ok...this is a bit far-fetched, but I see your point.

> 2) /proc/net/ files can deliver same "line" of information several
> times, because of their implementation.
> 
> 3) Because of SLAB_DESTROY_BY_RCU, same 'kernel socket pointer' can be
> seen several times in /proc/net/tcp & /proc/net/udp, but really on
> different "sockets"
> 
> 4) Some good applications use both the socket pointer and inode number
> (tuple) to filter out the [2] problem. Dont break them, please ?
> Anything that might break an application must be at the very least
> tunable.
> 
> In my opinion, a good thing would be :
> 
> - Use a special printf() format , aka "secure pointer", as Thomas
> suggested.
> 

I am happy to write this, but just to be sure, you're sure we're ok with
a printf() format that cannot be used in interrupt context?  %pS and %ps
are taken, so I'm thinking %pH ("hidden").

> - Make sure you print different opaque values for two different kernel
> pointers. This is mandatory.
> 
> - Make sure the NULL pointer stay as a NULL pointer to not let the
> hostile user know your secret, and to ease debugging stuff.
> 

Alright, this is fine by me.

> - Have security experts advice to chose a nice crypto function, maybe
> jenkin hash. Not too slow would be nice.
> 
> 
> static unsigned long securize_kpointers_rnd;
> 
> At boot time, stick a random value in this variable.
> (Maybe make sure the 5 low order bits are 0)
> 

I don't really like the approach of relying on a global secret value
that's used repeatedly.  Sure, it's better than not obfuscating the
pointers at all, but it seems like it will be difficult to prevent its
value from being inferred or discovered.

> unsigned long opacify_kptr(unsigned long ptr)
> {
> 	if (ptr == 0)
> 		return ptr;
> 	if (capable(CAP_NET_ADMIN))
> 		return ptr;
> 
> 	return some_crypto_hash(ptr, &securize_kpointers_rnd);
> }
> 

I question the use of CAP_NET_ADMIN.  Sure, that makes sense in this
context, but wouldn't it be useful to be able to use this format
specifier outside of net?  In fact, I'm not sure that CAP_SYS_ADMIN is
appropriate either - perhaps just going off current_euid()?

> At least, use a central point, so that we can easily add/change the
> logic if needed.
> 
> Please provide this patch in kernel/printk.c for initial review, then
> once everybody is OK, you can send one patch for net tree.
> 

Do you mean lib/vsprintf.c?

> No need to send 10 patches if we dont agree on the general principle.

Agreed.

-Dan

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[Stephen Hemminger]

On Fri, 12 Nov 2010 08:23:30 +0100
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Le jeudi 11 novembre 2010 à 21:34 -0500, Dan Rosenberg a écrit :
> > > I want whatever you replace it with to be equivalent for
> > > object tracking purposes.
> > 
> > In nearly all of the cases I fixed, the socket inode is already
> > provided, which serves as a perfectly good unique identifier.  Would you
> > prefer I include that information twice?
> 
> Oh well. Please read this answer carefuly.
> 
> Some facts to feed your next patch. I am very pleased you changed your
> mind and that we keep useful information in kernel log.
> 
> 1) Inode numbers are not guaranteed to be unique. Its a 32bit seq
> number, and we dont check another socket inode use the same inode number
> (after 2^32 allocations it can happens)
> 
> 2) /proc/net/ files can deliver same "line" of information several
> times, because of their implementation.
> 
> 3) Because of SLAB_DESTROY_BY_RCU, same 'kernel socket pointer' can be
> seen several times in /proc/net/tcp & /proc/net/udp, but really on
> different "sockets"
> 
> 4) Some good applications use both the socket pointer and inode number
> (tuple) to filter out the [2] problem. Dont break them, please ?
> Anything that might break an application must be at the very least
> tunable.
> 
> In my opinion, a good thing would be :
> 
> - Use a special printf() format , aka "secure pointer", as Thomas
> suggested.
> 
> - Make sure you print different opaque values for two different kernel
> pointers. This is mandatory.
> 
> - Make sure the NULL pointer stay as a NULL pointer to not let the
> hostile user know your secret, and to ease debugging stuff.
> 
> - Have security experts advice to chose a nice crypto function, maybe
> jenkin hash. Not too slow would be nice.
> 
> 
> static unsigned long securize_kpointers_rnd;
> 
> At boot time, stick a random value in this variable.
> (Maybe make sure the 5 low order bits are 0)
> 
> unsigned long opacify_kptr(unsigned long ptr)
> {
> 	if (ptr == 0)
> 		return ptr;
> 	if (capable(CAP_NET_ADMIN))
> 		return ptr;
> 
> 	return some_crypto_hash(ptr, &securize_kpointers_rnd);
> }
> 
> At least, use a central point, so that we can easily add/change the
> logic if needed.
> 
> Please provide this patch in kernel/printk.c for initial review, then
> once everybody is OK, you can send one patch for net tree.
> 
> No need to send 10 patches if we dont agree on the general principle.

Also, the whole idea needs to be under a config option, so only
the paranoid idiots turn it on.


-- 

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[Dan Rosenberg]

> 
> Also, the whole idea needs to be under a config option, so only
> the paranoid idiots turn it on.

If that's what's necessary to get it accepted, I'm willing to do that.
But when a solution does not negatively impact usability or performance
and improves security, even in a small way, why should it not be enabled
by default?  Of course it's my responsibility to first propose a
solution that is acceptable from a usability/debugging standpoint, but
assuming that can be achieved, I don't really see what the problem is.
There's a difference between being a "paranoid idiot" and wanting to
protect users from unnecessary exposure.

-Dan

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[Stephen Hemminger]

On Fri, 12 Nov 2010 12:24:42 -0500
Dan Rosenberg <drosenberg@vsecurity.com> wrote:

> 
> > 
> > Also, the whole idea needs to be under a config option, so only
> > the paranoid idiots turn it on.
> 
> If that's what's necessary to get it accepted, I'm willing to do that.
> But when a solution does not negatively impact usability or performance
> and improves security, even in a small way, why should it not be enabled
> by default?  Of course it's my responsibility to first propose a
> solution that is acceptable from a usability/debugging standpoint, but
> assuming that can be achieved, I don't really see what the problem is.
> There's a difference between being a "paranoid idiot" and wanting to
> protect users from unnecessary exposure.

See earlier discussion about automatically running crypto tests on boot
which caused Linus to flame. This is more intrusive, and is not something
most developers would want; but it might make sense in production
environment.

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[Alexey Dobriyan]

On Fri, Nov 12, 2010 at 08:33:15AM -0800, Stephen Hemminger wrote:
> Also, the whole idea needs to be under a config option, so only
> the paranoid idiots turn it on.

Would be fun if something will break because ffff8800bcd498c0
will become something else. :-)

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[David Miller]

From: Alexey Dobriyan <adobriyan@gmail.com>
Date: Fri, 12 Nov 2010 22:18:50 +0200

> On Fri, Nov 12, 2010 at 08:33:15AM -0800, Stephen Hemminger wrote:
>> Also, the whole idea needs to be under a config option, so only
>> the paranoid idiots turn it on.
> 
> Would be fun if something will break because ffff8800bcd498c0
> will become something else. :-)

Actually, this is not even a joke.

Take a look at how we track what sockets a user wants dumped via
the inet_diag netlink facility, the socket pointer is used as
the identification cookie.

I'm sure we'll now get some more security theatre about how we
have to undo that too.

More and more I see this whole idea as extremely rediculious.

If I can write to or read kernel memory, I can look up the
sockets, inodes, and whatever else we're currently exposing
the addresses of.  Even without a symbol table, which is
readily available, I can easily find the ksymtab and find the
inode and socket hash table addresses there.

This whole exercise is closing the barn door after the horses have
already escaped, and it's causing all kinds of inconveniences
that we really have no need for.

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[Alexey Dobriyan]

On Fri, Nov 12, 2010 at 12:37:38PM -0800, David Miller wrote:
> From: Alexey Dobriyan <adobriyan@gmail.com>
> Date: Fri, 12 Nov 2010 22:18:50 +0200
> 
> > On Fri, Nov 12, 2010 at 08:33:15AM -0800, Stephen Hemminger wrote:
> >> Also, the whole idea needs to be under a config option, so only
> >> the paranoid idiots turn it on.
> > 
> > Would be fun if something will break because ffff8800bcd498c0
> > will become something else. :-)
> 
> Actually, this is not even a joke.
> 
> Take a look at how we track what sockets a user wants dumped via
> the inet_diag netlink facility, the socket pointer is used as
> the identification cookie.

I think we should not expose kernel pointers in future interfaces,
but leave existing ones alone.

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[David Miller]

From: Alexey Dobriyan <adobriyan@gmail.com>
Date: Sat, 13 Nov 2010 00:40:27 +0200

> I think we should not expose kernel pointers in future interfaces,
> but leave existing ones alone.

That seems like the most reasonable and pragmatic position I've seen
thus far :-)

[PATCH 4/10] Fix leaking of kernel heap addresses in net/

[Dan Rosenberg]

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 7e41e2c..53dbdd0 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1973,25 +1973,49 @@ static void get_openreq6(struct seq_file *seq,
 	if (ttd < 0)
 		ttd = 0;
 
-	seq_printf(seq,
-		   "%4d: %08X%08X%08X%08X:%04X %08X%08X%08X%08X:%04X "
-		   "%02X %08X:%08X %02X:%08lX %08X %5d %8d %d %d %p\n",
-		   i,
-		   src->s6_addr32[0], src->s6_addr32[1],
-		   src->s6_addr32[2], src->s6_addr32[3],
-		   ntohs(inet_rsk(req)->loc_port),
-		   dest->s6_addr32[0], dest->s6_addr32[1],
-		   dest->s6_addr32[2], dest->s6_addr32[3],
-		   ntohs(inet_rsk(req)->rmt_port),
-		   TCP_SYN_RECV,
-		   0,0, /* could print option size, but that is af dependent. */
-		   1,   /* timers active (only the expire timer) */
-		   jiffies_to_clock_t(ttd),
-		   req->retrans,
-		   uid,
-		   0,  /* non standard timer */
-		   0, /* open_requests have no inode */
-		   0, req);
+	/* Only expose kernel addresses to privileged readers */
+	if (capable(CAP_NET_ADMIN))
+		seq_printf(seq,
+			"%4d: %08X%08X%08X%08X:%04X %08X%08X%08X%08X:%04X "
+			"%02X %08X:%08X %02X:%08lX %08X %5d %8d %d %d %p\n",
+			i,
+			src->s6_addr32[0], src->s6_addr32[1],
+			src->s6_addr32[2], src->s6_addr32[3],
+			ntohs(inet_rsk(req)->loc_port),
+			dest->s6_addr32[0], dest->s6_addr32[1],
+			dest->s6_addr32[2], dest->s6_addr32[3],
+			ntohs(inet_rsk(req)->rmt_port),
+			TCP_SYN_RECV,
+			0, 0, /* could print option size,
+				 but that is af dependent. */
+			1,    /* timers active (only the expire timer) */
+			jiffies_to_clock_t(ttd),
+			req->retrans,
+			uid,
+			0,  /* non standard timer */
+			0, /* open_requests have no inode */
+			0, req);
+	else
+		seq_printf(seq,
+			"%4d: %08X%08X%08X%08X:%04X %08X%08X%08X%08X:%04X "
+			"%02X %08X:%08X %02X:%08lX %08X %5d %8d %d %d %d\n",
+			i,
+			src->s6_addr32[0], src->s6_addr32[1],
+			src->s6_addr32[2], src->s6_addr32[3],
+			ntohs(inet_rsk(req)->loc_port),
+			dest->s6_addr32[0], dest->s6_addr32[1],
+			dest->s6_addr32[2], dest->s6_addr32[3],
+			ntohs(inet_rsk(req)->rmt_port),
+			TCP_SYN_RECV,
+			0, 0, /* could print option size,
+				 but that is af dependent. */
+			1,    /* timers active (only the expire timer) */
+			jiffies_to_clock_t(ttd),
+			req->retrans,
+			uid,
+			0,  /* non standard timer */
+			0, /* open_requests have no inode */
+			0, 0);
 }
 
 static void get_tcp6_sock(struct seq_file *seq, struct sock *sp, int i)
@@ -2024,30 +2048,57 @@ static void get_tcp6_sock(struct seq_file *seq, struct sock *sp, int i)
 		timer_expires = jiffies;
 	}
 
-	seq_printf(seq,
-		   "%4d: %08X%08X%08X%08X:%04X %08X%08X%08X%08X:%04X "
-		   "%02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %lu %lu %u %u %d\n",
-		   i,
-		   src->s6_addr32[0], src->s6_addr32[1],
-		   src->s6_addr32[2], src->s6_addr32[3], srcp,
-		   dest->s6_addr32[0], dest->s6_addr32[1],
-		   dest->s6_addr32[2], dest->s6_addr32[3], destp,
-		   sp->sk_state,
-		   tp->write_seq-tp->snd_una,
-		   (sp->sk_state == TCP_LISTEN) ? sp->sk_ack_backlog : (tp->rcv_nxt - tp->copied_seq),
-		   timer_active,
-		   jiffies_to_clock_t(timer_expires - jiffies),
-		   icsk->icsk_retransmits,
-		   sock_i_uid(sp),
-		   icsk->icsk_probes_out,
-		   sock_i_ino(sp),
-		   atomic_read(&sp->sk_refcnt), sp,
-		   jiffies_to_clock_t(icsk->icsk_rto),
-		   jiffies_to_clock_t(icsk->icsk_ack.ato),
-		   (icsk->icsk_ack.quick << 1 ) | icsk->icsk_ack.pingpong,
-		   tp->snd_cwnd,
-		   tcp_in_initial_slowstart(tp) ? -1 : tp->snd_ssthresh
-		   );
+	/* Only expose kernel addresses to privileged readers */
+	if (capable(CAP_NET_ADMIN))
+		seq_printf(seq,
+			"%4d: %08X%08X%08X%08X:%04X %08X%08X%08X%08X:%04X "
+			"%02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %lu %lu %u %u %d\n",
+			i,
+			src->s6_addr32[0], src->s6_addr32[1],
+			src->s6_addr32[2], src->s6_addr32[3], srcp,
+			dest->s6_addr32[0], dest->s6_addr32[1],
+			dest->s6_addr32[2], dest->s6_addr32[3], destp,
+			sp->sk_state,
+			tp->write_seq-tp->snd_una,
+			(sp->sk_state == TCP_LISTEN) ?
+			sp->sk_ack_backlog : (tp->rcv_nxt - tp->copied_seq),
+			timer_active,
+			jiffies_to_clock_t(timer_expires - jiffies),
+			icsk->icsk_retransmits,
+			sock_i_uid(sp),
+			icsk->icsk_probes_out,
+			sock_i_ino(sp),
+			atomic_read(&sp->sk_refcnt), sp,
+			jiffies_to_clock_t(icsk->icsk_rto),
+			jiffies_to_clock_t(icsk->icsk_ack.ato),
+			(icsk->icsk_ack.quick << 1) | icsk->icsk_ack.pingpong,
+			tp->snd_cwnd,
+			tcp_in_initial_slowstart(tp) ? -1 : tp->snd_ssthresh);
+	else
+		seq_printf(seq,
+			"%4d: %08X%08X%08X%08X:%04X %08X%08X%08X%08X:%04X "
+			"%02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %d %lu %lu %u %u %d\n",
+			i,
+			src->s6_addr32[0], src->s6_addr32[1],
+			src->s6_addr32[2], src->s6_addr32[3], srcp,
+			dest->s6_addr32[0], dest->s6_addr32[1],
+			dest->s6_addr32[2], dest->s6_addr32[3], destp,
+			sp->sk_state,
+			tp->write_seq-tp->snd_una,
+			(sp->sk_state == TCP_LISTEN) ?
+			sp->sk_ack_backlog : (tp->rcv_nxt - tp->copied_seq),
+			timer_active,
+			jiffies_to_clock_t(timer_expires - jiffies),
+			icsk->icsk_retransmits,
+			sock_i_uid(sp),
+			icsk->icsk_probes_out,
+			sock_i_ino(sp),
+			atomic_read(&sp->sk_refcnt), 0,
+			jiffies_to_clock_t(icsk->icsk_rto),
+			jiffies_to_clock_t(icsk->icsk_ack.ato),
+			(icsk->icsk_ack.quick << 1) | icsk->icsk_ack.pingpong,
+			tp->snd_cwnd,
+			tcp_in_initial_slowstart(tp) ? -1 : tp->snd_ssthresh);
 }
 
 static void get_timewait6_sock(struct seq_file *seq,
@@ -2066,17 +2117,31 @@ static void get_timewait6_sock(struct seq_file *seq,
 	destp = ntohs(tw->tw_dport);
 	srcp  = ntohs(tw->tw_sport);
 
-	seq_printf(seq,
-		   "%4d: %08X%08X%08X%08X:%04X %08X%08X%08X%08X:%04X "
-		   "%02X %08X:%08X %02X:%08lX %08X %5d %8d %d %d %p\n",
-		   i,
-		   src->s6_addr32[0], src->s6_addr32[1],
-		   src->s6_addr32[2], src->s6_addr32[3], srcp,
-		   dest->s6_addr32[0], dest->s6_addr32[1],
-		   dest->s6_addr32[2], dest->s6_addr32[3], destp,
-		   tw->tw_substate, 0, 0,
-		   3, jiffies_to_clock_t(ttd), 0, 0, 0, 0,
-		   atomic_read(&tw->tw_refcnt), tw);
+	/* Only expose kernel addresses to privileged readers */
+	if (capable(CAP_NET_ADMIN))
+		seq_printf(seq,
+			"%4d: %08X%08X%08X%08X:%04X %08X%08X%08X%08X:%04X "
+			"%02X %08X:%08X %02X:%08lX %08X %5d %8d %d %d %p\n",
+			i,
+			src->s6_addr32[0], src->s6_addr32[1],
+			src->s6_addr32[2], src->s6_addr32[3], srcp,
+			dest->s6_addr32[0], dest->s6_addr32[1],
+			dest->s6_addr32[2], dest->s6_addr32[3], destp,
+			tw->tw_substate, 0, 0,
+			3, jiffies_to_clock_t(ttd), 0, 0, 0, 0,
+			atomic_read(&tw->tw_refcnt), tw);
+	else
+		seq_printf(seq,
+			"%4d: %08X%08X%08X%08X:%04X %08X%08X%08X%08X:%04X "
+			"%02X %08X:%08X %02X:%08lX %08X %5d %8d %d %d %d\n",
+			i,
+			src->s6_addr32[0], src->s6_addr32[1],
+			src->s6_addr32[2], src->s6_addr32[3], srcp,
+			dest->s6_addr32[0], dest->s6_addr32[1],
+			dest->s6_addr32[2], dest->s6_addr32[3], destp,
+			tw->tw_substate, 0, 0,
+			3, jiffies_to_clock_t(ttd), 0, 0, 0, 0,
+			atomic_read(&tw->tw_refcnt), 0);
 }
 
 static int tcp6_seq_show(struct seq_file *seq, void *v)

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[David Miller]

This will print zero once a socket enters various closing states or
similar, since the sk->sk_socket is released and turns NULL.

Thus making the debugging information next to useless.

I'm still largely against these changes, and will not apply them
to my tree.

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[Dan Rosenberg]

> This will print zero once a socket enters various closing states or
> similar, since the sk->sk_socket is released and turns NULL.
> 
> Thus making the debugging information next to useless.
> 
> I'm still largely against these changes, and will not apply them
> to my tree.

Would you care to offer any suggestions that would both solve the
problem and be acceptable to you?

If not, then you have committed to leaving a permanent easy target for
exploits.  Please think carefully about this.

-Dan

Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/

[David Miller]

From: Dan Rosenberg <drosenberg-PiUznwcHFHrqlBn2x/YWAg@public.gmane.org>
Date: Thu, 11 Nov 2010 20:20:01 -0500

> If not, then you have committed to leaving a permanent easy target for
> exploits.

In your opinion.