* Re: [SECURITY] [PATCH] Prevent crashing when parsing bad X.25 facilities
From: Dan Rosenberg @ 2010-11-11 23:49 UTC (permalink / raw)
To: Andrew Hendry; +Cc: netdev, security
In-Reply-To: <AANLkTimL61V2wDvZcsM27ZHPUnEQqaqnk2MTb=E0nSCY@mail.gmail.com>
I can take care of it. Do you want me to just remove those items from
the printk statement?
-Dan
On Fri, 2010-11-12 at 10:24 +1100, Andrew Hendry wrote:
> Tested ok, although I noticed the CLASS_D default: message tries to
> print the facility type, length and 4 values, while it may only have 2
> values. Just needs p[4] and p[5] taken out. Do you want me to spin a
> secondary patch or do you want to put them together?
>
> Acked-by: Andrew Hendry <andrew.hendry@gmail.com>
>
> On Fri, Nov 12, 2010 at 9:43 AM, Dan Rosenberg <drosenberg@vsecurity.com> wrote:
> > Sorry I didn't catch this at the same time as my previous report so it
> > could be included in one patch.
> >
> > On parsing malformed X.25 facilities, decrementing the remaining length
> > may cause it to underflow. Since the length is an unsigned integer,
> > this will result in the loop continuing until the kernel crashes.
> >
> > This patch adds checks to ensure decrementing the remaining length does
> > not cause it to wrap around.
> >
> > Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
> > CC: stable <stable@kernel.org>
> > ---
> > net/x25/x25_facilities.c | 8 ++++++++
> > 1 files changed, 8 insertions(+), 0 deletions(-)
> >
> > diff --git a/net/x25/x25_facilities.c b/net/x25/x25_facilities.c
> > index 3a8c4c4..bef8330 100644
> > --- a/net/x25/x25_facilities.c
> > +++ b/net/x25/x25_facilities.c
> > @@ -61,6 +61,8 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
> > while (len > 0) {
> > switch (*p & X25_FAC_CLASS_MASK) {
> > case X25_FAC_CLASS_A:
> > + if (len < 2)
> > + return 0;
> > switch (*p) {
> > case X25_FAC_REVERSE:
> > if((p[1] & 0x81) == 0x81) {
> > @@ -104,6 +106,8 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
> > len -= 2;
> > break;
> > case X25_FAC_CLASS_B:
> > + if (len < 3)
> > + return 0;
> > switch (*p) {
> > case X25_FAC_PACKET_SIZE:
> > facilities->pacsize_in = p[1];
> > @@ -125,6 +129,8 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
> > len -= 3;
> > break;
> > case X25_FAC_CLASS_C:
> > + if (len < 4)
> > + return 0;
> > printk(KERN_DEBUG "X.25: unknown facility %02X, "
> > "values %02X, %02X, %02X\n",
> > p[0], p[1], p[2], p[3]);
> > @@ -132,6 +138,8 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
> > len -= 4;
> > break;
> > case X25_FAC_CLASS_D:
> > + if (len < p[1] + 2)
> > + return 0;
> > switch (*p) {
> > case X25_FAC_CALLING_AE:
> > if (p[1] > X25_MAX_DTE_FACIL_LEN || p[1] <= 1)
> >
> >
> >
^ permalink raw reply
* Re: [SECURITY] [PATCH] Prevent crashing when parsing bad X.25 facilities
From: Andrew Hendry @ 2010-11-11 23:53 UTC (permalink / raw)
To: Dan Rosenberg; +Cc: netdev, security
In-Reply-To: <1289519342.5167.19.camel@dan>
Thanks, yes just remove them from the printk.
On Fri, Nov 12, 2010 at 10:49 AM, Dan Rosenberg
<drosenberg@vsecurity.com> wrote:
> I can take care of it. Do you want me to just remove those items from
> the printk statement?
>
> -Dan
>
> On Fri, 2010-11-12 at 10:24 +1100, Andrew Hendry wrote:
>> Tested ok, although I noticed the CLASS_D default: message tries to
>> print the facility type, length and 4 values, while it may only have 2
>> values. Just needs p[4] and p[5] taken out. Do you want me to spin a
>> secondary patch or do you want to put them together?
>>
>> Acked-by: Andrew Hendry <andrew.hendry@gmail.com>
>>
>> On Fri, Nov 12, 2010 at 9:43 AM, Dan Rosenberg <drosenberg@vsecurity.com> wrote:
>> > Sorry I didn't catch this at the same time as my previous report so it
>> > could be included in one patch.
>> >
>> > On parsing malformed X.25 facilities, decrementing the remaining length
>> > may cause it to underflow. Since the length is an unsigned integer,
>> > this will result in the loop continuing until the kernel crashes.
>> >
>> > This patch adds checks to ensure decrementing the remaining length does
>> > not cause it to wrap around.
>> >
>> > Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
>> > CC: stable <stable@kernel.org>
>> > ---
>> > net/x25/x25_facilities.c | 8 ++++++++
>> > 1 files changed, 8 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/net/x25/x25_facilities.c b/net/x25/x25_facilities.c
>> > index 3a8c4c4..bef8330 100644
>> > --- a/net/x25/x25_facilities.c
>> > +++ b/net/x25/x25_facilities.c
>> > @@ -61,6 +61,8 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
>> > while (len > 0) {
>> > switch (*p & X25_FAC_CLASS_MASK) {
>> > case X25_FAC_CLASS_A:
>> > + if (len < 2)
>> > + return 0;
>> > switch (*p) {
>> > case X25_FAC_REVERSE:
>> > if((p[1] & 0x81) == 0x81) {
>> > @@ -104,6 +106,8 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
>> > len -= 2;
>> > break;
>> > case X25_FAC_CLASS_B:
>> > + if (len < 3)
>> > + return 0;
>> > switch (*p) {
>> > case X25_FAC_PACKET_SIZE:
>> > facilities->pacsize_in = p[1];
>> > @@ -125,6 +129,8 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
>> > len -= 3;
>> > break;
>> > case X25_FAC_CLASS_C:
>> > + if (len < 4)
>> > + return 0;
>> > printk(KERN_DEBUG "X.25: unknown facility %02X, "
>> > "values %02X, %02X, %02X\n",
>> > p[0], p[1], p[2], p[3]);
>> > @@ -132,6 +138,8 @@ int x25_parse_facilities(struct sk_buff *skb, struct x25_facilities *facilities,
>> > len -= 4;
>> > break;
>> > case X25_FAC_CLASS_D:
>> > + if (len < p[1] + 2)
>> > + return 0;
>> > switch (*p) {
>> > case X25_FAC_CALLING_AE:
>> > if (p[1] > X25_MAX_DTE_FACIL_LEN || p[1] <= 1)
>> >
>> >
>> >
>
>
>
^ permalink raw reply
* Re: [e1000e] BUG triggered in blink path
From: Jeff Kirsher @ 2010-11-11 23:59 UTC (permalink / raw)
To: Brandeburg, Jesse, e1000-devel, netdev
In-Reply-To: <20101111101738.GA2972@mail.eitzenberger.org>
On Thu, Nov 11, 2010 at 02:17, Holger Eitzenberger
<holger@eitzenberger.org> wrote:
> Hi Jesse,
>
> I've attached the patch against net-next-2.6. Please check if it's ok
> for you. I checked e1000, igb and ixgbe as well, they don't have that
> problem.
>
> /holger
>
Patch looks good, I have added the patch to my tree.
Cheers,
Jeff
>> > After taking a look I think this may be caused by initializing
>> > adapter->led_blink_task several times in e1000_phys_id(), while possibly
>> > led_blink_task is running:
>> >
>> > if ((hw->phy.type == e1000_phy_ife) ||
>> > (hw->mac.type == e1000_pchlan) ||
>> > (hw->mac.type == e1000_82574)) {
>> > INIT_WORK(&adapter->led_blink_task, e1000e_led_blink_task);
>> > if (!adapter->blink_timer.function) {
>> >
>> > I can't reproduce it after moving it inside the following if block,
>> > but I'm not quite sure if this catches all races in there. Especially
>> > the msleep_interruptible() may be too optimistic because it may
>> > actually not wait long enough. Someone with more knowledge of the
>> > driver should take a look.
>>
>> thanks for your investigation and troubleshooting. I don't think it is
>> correct at all to be calling INIT_WORK more than once. In fact the
>> INIT_WORK should just be moved into probe, and then e1000_phys_id should
>> just do schedule_work.
>>
>>
>
^ permalink raw reply
* Re: [PATCH] vxge: update firmware to version 1.8.1
From: David Woodhouse @ 2010-11-12 0:22 UTC (permalink / raw)
To: Jon Mason; +Cc: netdev, Sivakumar Subramani, Sreenivasa Honnur, Ram Vepa
In-Reply-To: <1289521154-16377-1-git-send-email-jon.mason@exar.com>
On Thu, 2010-11-11 at 18:19 -0600, Jon Mason wrote:
> Update firmware for the Exar X3100 10Gbps adapters to version 1.8.1
This is a compatible change which doesn't require driver changes?
--
dwmw2
^ permalink raw reply
* [PATCH] ipv4: Make rt->fl.iif tests lest obscure.
From: David Miller @ 2010-11-12 0:28 UTC (permalink / raw)
To: netdev
When we test rt->fl.iif against zero, we're seeing if it's
an output or an input route.
Make that explicit with some helper functions.
Signed-off-by: David S. Miller <davem@davemloft.net>
---
Committed to net-next-2.6, this has bugged me for some
time. :-)
diff --git a/include/net/route.h b/include/net/route.h
index cea533e..5cd46d1 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -71,6 +71,16 @@ struct rtable {
struct inet_peer *peer; /* long-living peer info */
};
+static inline bool rt_is_input_route(struct rtable *rt)
+{
+ return rt->fl.iif != 0;
+}
+
+static inline bool rt_is_output_route(struct rtable *rt)
+{
+ return rt->fl.iif == 0;
+}
+
struct ip_rt_acct {
__u32 o_bytes;
__u32 o_packets;
diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 94a9eb1..9aad1c0 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -1181,7 +1181,7 @@ static int __dn_route_output_key(struct dst_entry **pprt, const struct flowi *fl
if ((flp->fld_dst == rt->fl.fld_dst) &&
(flp->fld_src == rt->fl.fld_src) &&
(flp->mark == rt->fl.mark) &&
- (rt->fl.iif == 0) &&
+ rt_is_output_route(rt) &&
(rt->fl.oif == flp->oif)) {
dst_use(&rt->dst, jiffies);
rcu_read_unlock_bh();
@@ -1512,7 +1512,7 @@ static int dn_rt_fill_info(struct sk_buff *skb, u32 pid, u32 seq,
if (rtnl_put_cacheinfo(skb, &rt->dst, 0, 0, 0, expires,
rt->dst.error) < 0)
goto rtattr_failure;
- if (rt->fl.iif)
+ if (rt_is_input_route(rt))
RTA_PUT(skb, RTA_IIF, sizeof(int), &rt->fl.iif);
nlh->nlmsg_len = skb_tail_pointer(skb) - b;
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 96bc7f9..c6e2aff 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -506,8 +506,8 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
struct net_device *dev = NULL;
rcu_read_lock();
- if (rt->fl.iif &&
- net->ipv4.sysctl_icmp_errors_use_inbound_ifaddr)
+ if (rt_is_input_route(rt) &&
+ net->ipv4.sysctl_icmp_errors_use_inbound_ifaddr)
dev = dev_get_by_index_rcu(net, rt->fl.iif);
if (dev)
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index c8877c6..08d0d81 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -961,7 +961,7 @@ int igmp_rcv(struct sk_buff *skb)
case IGMP_HOST_MEMBERSHIP_REPORT:
case IGMPV2_HOST_MEMBERSHIP_REPORT:
/* Is it our report looped back? */
- if (skb_rtable(skb)->fl.iif == 0)
+ if (rt_is_output_route(skb_rtable(skb)))
break;
/* don't rely on MC router hearing unicast reports */
if (skb->pkt_type == PACKET_MULTICAST ||
diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 70ff77f..cab2057 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -634,7 +634,7 @@ static int ipgre_rcv(struct sk_buff *skb)
#ifdef CONFIG_NET_IPGRE_BROADCAST
if (ipv4_is_multicast(iph->daddr)) {
/* Looped back packet, drop it! */
- if (skb_rtable(skb)->fl.iif == 0)
+ if (rt_is_output_route(skb_rtable(skb)))
goto drop;
tunnel->dev->stats.multicast++;
skb->pkt_type = PACKET_BROADCAST;
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 86dd569..ef2b008 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1654,7 +1654,7 @@ static int ip_mr_forward(struct net *net, struct mr_table *mrt,
if (mrt->vif_table[vif].dev != skb->dev) {
int true_vifi;
- if (skb_rtable(skb)->fl.iif == 0) {
+ if (rt_is_output_route(skb_rtable(skb))) {
/* It is our own packet, looped back.
* Very complicated situation...
*
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 5955965..66610ea 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -623,7 +623,7 @@ static inline int rt_fast_clean(struct rtable *rth)
/* Kill broadcast/multicast entries very aggresively, if they
collide in hash table with more useful entries */
return (rth->rt_flags & (RTCF_BROADCAST | RTCF_MULTICAST)) &&
- rth->fl.iif && rth->dst.rt_next;
+ rt_is_input_route(rth) && rth->dst.rt_next;
}
static inline int rt_valuable(struct rtable *rth)
@@ -668,7 +668,7 @@ static inline u32 rt_score(struct rtable *rt)
if (rt_valuable(rt))
score |= (1<<31);
- if (!rt->fl.iif ||
+ if (rt_is_output_route(rt) ||
!(rt->rt_flags & (RTCF_BROADCAST|RTCF_MULTICAST|RTCF_LOCAL)))
score |= (1<<30);
@@ -1126,7 +1126,7 @@ restart:
*/
rt->dst.flags |= DST_NOCACHE;
- if (rt->rt_type == RTN_UNICAST || rt->fl.iif == 0) {
+ if (rt->rt_type == RTN_UNICAST || rt_is_output_route(rt)) {
int err = arp_bind_neighbour(&rt->dst);
if (err) {
if (net_ratelimit())
@@ -1224,7 +1224,7 @@ restart:
/* Try to bind route to arp only if it is output
route or unicast forwarding path.
*/
- if (rt->rt_type == RTN_UNICAST || rt->fl.iif == 0) {
+ if (rt->rt_type == RTN_UNICAST || rt_is_output_route(rt)) {
int err = arp_bind_neighbour(&rt->dst);
if (err) {
spin_unlock_bh(rt_hash_lock_addr(hash));
@@ -1406,7 +1406,7 @@ void ip_rt_redirect(__be32 old_gw, __be32 daddr, __be32 new_gw,
if (rth->fl.fl4_dst != daddr ||
rth->fl.fl4_src != skeys[i] ||
rth->fl.oif != ikeys[k] ||
- rth->fl.iif != 0 ||
+ rt_is_input_route(rth) ||
rt_is_expired(rth) ||
!net_eq(dev_net(rth->dst.dev), net)) {
rthp = &rth->dst.rt_next;
@@ -1666,7 +1666,7 @@ unsigned short ip_rt_frag_needed(struct net *net, struct iphdr *iph,
rth->rt_dst != daddr ||
rth->rt_src != iph->saddr ||
rth->fl.oif != ikeys[k] ||
- rth->fl.iif != 0 ||
+ rt_is_input_route(rth) ||
dst_metric_locked(&rth->dst, RTAX_MTU) ||
!net_eq(dev_net(rth->dst.dev), net) ||
rt_is_expired(rth))
@@ -1770,7 +1770,7 @@ void ip_rt_get_source(u8 *addr, struct rtable *rt)
__be32 src;
struct fib_result res;
- if (rt->fl.iif == 0)
+ if (rt_is_output_route(rt))
src = rt->rt_src;
else {
rcu_read_lock();
@@ -2669,7 +2669,7 @@ int __ip_route_output_key(struct net *net, struct rtable **rp,
rth = rcu_dereference_bh(rth->dst.rt_next)) {
if (rth->fl.fl4_dst == flp->fl4_dst &&
rth->fl.fl4_src == flp->fl4_src &&
- rth->fl.iif == 0 &&
+ rt_is_output_route(rth) &&
rth->fl.oif == flp->oif &&
rth->fl.mark == flp->mark &&
!((rth->fl.fl4_tos ^ flp->fl4_tos) &
@@ -2824,7 +2824,7 @@ static int rt_fill_info(struct net *net,
if (rt->dst.tclassid)
NLA_PUT_U32(skb, RTA_FLOW, rt->dst.tclassid);
#endif
- if (rt->fl.iif)
+ if (rt_is_input_route(rt))
NLA_PUT_BE32(skb, RTA_PREFSRC, rt->rt_spec_dst);
else if (rt->rt_src != rt->fl.fl4_src)
NLA_PUT_BE32(skb, RTA_PREFSRC, rt->rt_src);
@@ -2849,7 +2849,7 @@ static int rt_fill_info(struct net *net,
}
}
- if (rt->fl.iif) {
+ if (rt_is_input_route(rt)) {
#ifdef CONFIG_IP_MROUTE
__be32 dst = rt->rt_dst;
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index de04ea3..10bd39c 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -169,7 +169,7 @@ __ip_vs_reroute_locally(struct sk_buff *skb)
struct net *net = dev_net(dev);
struct iphdr *iph = ip_hdr(skb);
- if (rt->fl.iif) {
+ if (rt_is_input_route(rt)) {
unsigned long orefdst = skb->_skb_refdst;
if (ip_route_input(skb, iph->daddr, iph->saddr,
@@ -552,7 +552,8 @@ ip_vs_nat_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
#endif
/* From world but DNAT to loopback address? */
- if (local && ipv4_is_loopback(rt->rt_dst) && skb_rtable(skb)->fl.iif) {
+ if (local && ipv4_is_loopback(rt->rt_dst) &&
+ rt_is_input_route(skb_rtable(skb))) {
IP_VS_DBG_RL_PKT(1, AF_INET, pp, skb, 0, "ip_vs_nat_xmit(): "
"stopping DNAT to loopback address");
goto tx_error_put;
@@ -1165,7 +1166,8 @@ ip_vs_icmp_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
#endif
/* From world but DNAT to loopback address? */
- if (local && ipv4_is_loopback(rt->rt_dst) && skb_rtable(skb)->fl.iif) {
+ if (local && ipv4_is_loopback(rt->rt_dst) &&
+ rt_is_input_route(skb_rtable(skb))) {
IP_VS_DBG(1, "%s(): "
"stopping DNAT to loopback %pI4\n",
__func__, &cp->daddr.ip);
^ permalink raw reply related
* Re: [PATCH] ipv4: Make rt->fl.iif tests lest obscure.
From: David Miller @ 2010-11-12 0:36 UTC (permalink / raw)
To: netdev
In-Reply-To: <20101111.162814.35035010.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Thu, 11 Nov 2010 16:28:14 -0800 (PST)
> diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
> index 94a9eb1..9aad1c0 100644
> --- a/net/decnet/dn_route.c
> +++ b/net/decnet/dn_route.c
> @@ -1181,7 +1181,7 @@ static int __dn_route_output_key(struct dst_entry **pprt, const struct flowi *fl
Anyone looking at this closely will notice that I need to redo these
decnet parts.
Updated patch coming up.
^ permalink raw reply
* Re: [e1000e] BUG triggered in blink path
From: Brandeburg, Jesse @ 2010-11-12 0:40 UTC (permalink / raw)
To: Holger Eitzenberger
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org
In-Reply-To: <20101111101738.GA2972@mail.eitzenberger.org>
On Thu, 11 Nov 2010, Holger Eitzenberger wrote:
> I've attached the patch against net-next-2.6. Please check if it's ok
> for you. I checked e1000, igb and ixgbe as well, they don't have that
> problem.
Holger,
I think the patch looks good and thanks. Great catch too! I see Jeff has
put it into our testing process.
Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
^ permalink raw reply
* RE: [PATCH] vxge: update firmware to version 1.8.1
From: Ramkrishna Vepa @ 2010-11-12 0:52 UTC (permalink / raw)
To: David Woodhouse, Jon Mason
Cc: netdev@vger.kernel.org, Sivakumar Subramani, Sreenivasa Honnur
In-Reply-To: <1289521353.9408.92.camel@i7.infradead.org>
Hi David,
Yes, this firmware is compatible with the driver currently in the kernel. No driver change is required to work with this firmware version.
Thanks,
Ram
> -----Original Message-----
> From: David Woodhouse [mailto:dwmw2@infradead.org]
> Sent: Thursday, November 11, 2010 4:23 PM
> To: Jon Mason
> Cc: netdev@vger.kernel.org; Sivakumar Subramani; Sreenivasa Honnur;
> Ramkrishna Vepa
> Subject: Re: [PATCH] vxge: update firmware to version 1.8.1
>
> On Thu, 2010-11-11 at 18:19 -0600, Jon Mason wrote:
> > Update firmware for the Exar X3100 10Gbps adapters to version 1.8.1
>
> This is a compatible change which doesn't require driver changes?
>
> --
> dwmw2
The information and any attached documents contained in this message
may be confidential and/or legally privileged. The message is
intended solely for the addressee(s). If you are not the intended
recipient, you are hereby notified that any use, dissemination, or
reproduction is strictly prohibited and may be unlawful. If you are
not the intended recipient, please contact the sender immediately by
return e-mail and destroy all copies of the original message.
^ permalink raw reply
* RE: [PATCH] vxge: update firmware to version 1.8.1
From: David Woodhouse @ 2010-11-12 1:02 UTC (permalink / raw)
To: Ramkrishna Vepa
Cc: Jon Mason, netdev@vger.kernel.org, Sivakumar Subramani,
Sreenivasa Honnur
In-Reply-To: <FCA91A92EE52B041906A0358FC28FCC38EF64B4A15@FRE1EXCH02.hq.exar.com>
On Thu, 2010-11-11 at 16:52 -0800, Ramkrishna Vepa wrote:
>
> Yes, this firmware is compatible with the driver currently in the
> kernel. No driver change is required to work with this firmware
> version.
Thanks. Pushed.
--
dwmw2
^ permalink raw reply
* [PATCH 0/10] Fix leaking of kernel heap addresses in net/
From: Dan Rosenberg @ 2010-11-12 1:06 UTC (permalink / raw)
To: David S. Miller, Oliver Hartkopp, Urs Thuermann, Alexey Kuznetsov,
"Pekka Savola
This patch series resolves the leakage of kernel heap addresses to
userspace via network protocol /proc interfaces. Revealing this
information is a bad idea from a security perspective for a number of
reasons, the most obvious of which is it provides unprivileged users a
mechanism by which to create a structure in the kernel heap containing
function pointers, obtain the address of that structure, and overwrite
those function pointers by leveraging other vulnerabilities. It is my
hope that by eliminating this information leakage, in conjunction with
making statically-declared function pointer tables read-only (to be done
in a separate patch series), we can at least add a small hurdle for the
exploitation of a subset of kernel vulnerabilities.
To maintain compatibility with userspace programs relying on
consistent /proc output, the output descriptions and number of fields
are not changed. When a unique identifier for the socket is desired, the
socket address has been replaced with the socket inode number. When the
inode number is already present in the output, the address has been
replaced with a 0. In these cases, the format specifier has been changed
to %d, because a %p output of 0 from kernel space is written as
"(null)", while userspace %p can only parse "(nil)".
To address feedback that this debugging information is often useful and
should not be removed, the original kernel addresses are printed when
the reader has CAP_NET_ADMIN. The result is not especially pretty, but
I chose the more verbose option of providing separate print statements
for each case rather than inserting conditionals into the print
statements themselves, which would make for poor readability.
Signed-off-by: Dan Rosenberg <drosenberg@vsecurity.com>
---
net/atm/proc.c | 2 +-
net/can/bcm.c | 17 +++--
net/ipv4/raw.c | 27 +++++--
net/ipv4/tcp_ipv4.c | 136 +++++++++++++++++++++++++-----------
net/ipv4/udp.c | 28 +++++--
net/ipv6/tcp_ipv6.c | 173 +++++++++++++++++++++++++++++++--------------
net/key/af_key.c | 27 +++++--
net/netlink/af_netlink.c | 41 ++++++++---
net/packet/af_packet.c | 36 +++++++---
net/phonet/socket.c | 27 +++++--
net/sctp/proc.c | 56 +++++++++++----
net/unix/af_unix.c | 37 +++++++---
12 files changed, 427 insertions(+), 180 deletions(-)
^ permalink raw reply
* [PATCH 1/10] Fix leaking of kernel heap addresses in net/
From: Dan Rosenberg @ 2010-11-12 1:06 UTC (permalink / raw)
To: David S. Miller, Oliver Hartkopp, Alexey Kuznetsov, Urs Thuermann,
Hideaki
diff --git a/net/atm/proc.c b/net/atm/proc.c
index f85da07..21ec2ba 100644
--- a/net/atm/proc.c
+++ b/net/atm/proc.c
@@ -191,7 +191,7 @@ static void vcc_info(struct seq_file *seq, struct atm_vcc *vcc)
{
struct sock *sk = sk_atm(vcc);
- seq_printf(seq, "%p ", vcc);
+ seq_printf(seq, "%lu ", sock_i_ino(sk));
if (!vcc->dev)
seq_printf(seq, "Unassigned ");
else
^ permalink raw reply related
* [PATCH 2/10] Fix leaking of kernel heap addresses in net/
From: Dan Rosenberg @ 2010-11-12 1:06 UTC (permalink / raw)
To: David S. Miller, Oliver Hartkopp, Alexey Kuznetsov, Urs Thuermann,
Hideaki
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 08ffe9e..5960ad7 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -165,9 +165,16 @@ static int bcm_proc_show(struct seq_file *m, void *v)
struct bcm_sock *bo = bcm_sk(sk);
struct bcm_op *op;
- seq_printf(m, ">>> socket %p", sk->sk_socket);
- seq_printf(m, " / sk %p", sk);
- seq_printf(m, " / bo %p", bo);
+ /* Only expose kernel addresses to privileged readers */
+ if (capable(CAP_NET_ADMIN))
+ seq_printf(m, ">>> socket %p", sk->sk_socket);
+ seq_printf(m, " / sk %p", sk);
+ seq_printf(m, " / bo %p", bo);
+ else
+ seq_printf(m, ">>> socket %lu", sock_i_ino(sk));
+ seq_printf(m, " / sk %d", 0);
+ seq_printf(m, " / bo %d", 0);
+
seq_printf(m, " / dropped %lu", bo->dropped_usr_msgs);
seq_printf(m, " / bound %s", bcm_proc_getifname(ifname, bo->ifindex));
seq_printf(m, " <<<\n");
@@ -1520,8 +1527,8 @@ static int bcm_connect(struct socket *sock, struct sockaddr *uaddr, int len,
bo->bound = 1;
if (proc_dir) {
- /* unique socket address as filename */
- sprintf(bo->procname, "%p", sock);
+ /* unique socket inode as filename */
+ sprintf(bo->procname, "%lx", sock_i_ino(sk));
bo->bcm_proc_read = proc_create_data(bo->procname, 0644,
proc_dir,
&bcm_proc_fops, sk);
^ permalink raw reply related
* [PATCH 3/10] Fix leaking of kernel heap addresses in net/
From: Dan Rosenberg @ 2010-11-12 1:07 UTC (permalink / raw)
To: David S. Miller, Oliver Hartkopp, Alexey Kuznetsov, Urs Thuermann,
Hideaki
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 1f85ef2..0ac8ff2 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -948,13 +948,26 @@ static void raw_sock_seq_show(struct seq_file *seq, struct sock *sp, int i)
__u16 destp = 0,
srcp = inet->inet_num;
- seq_printf(seq, "%4d: %08X:%04X %08X:%04X"
- " %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d\n",
- i, src, srcp, dest, destp, sp->sk_state,
- sk_wmem_alloc_get(sp),
- sk_rmem_alloc_get(sp),
- 0, 0L, 0, sock_i_uid(sp), 0, sock_i_ino(sp),
- atomic_read(&sp->sk_refcnt), sp, atomic_read(&sp->sk_drops));
+ /* Only expose kernel addresses to privileged readers */
+ if (capable(CAP_NET_ADMIN))
+ seq_printf(seq, "%4d: %08X:%04X %08X:%04X"
+ " %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d\n",
+ i, src, srcp, dest, destp, sp->sk_state,
+ sk_wmem_alloc_get(sp),
+ sk_rmem_alloc_get(sp),
+ 0, 0L, 0, sock_i_uid(sp), 0, sock_i_ino(sp),
+ atomic_read(&sp->sk_refcnt),
+ sp, atomic_read(&sp->sk_drops));
+ else
+ seq_printf(seq, "%4d: %08X:%04X %08X:%04X"
+ " %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %d %d\n",
+ i, src, srcp, dest, destp, sp->sk_state,
+ sk_wmem_alloc_get(sp),
+ sk_rmem_alloc_get(sp),
+ 0, 0L, 0, sock_i_uid(sp), 0, sock_i_ino(sp),
+ atomic_read(&sp->sk_refcnt),
+ 0, atomic_read(&sp->sk_drops));
+
}
static int raw_seq_show(struct seq_file *seq, void *v)
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 8f8527d..a37eeb4 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2388,24 +2388,47 @@ static void get_openreq4(struct sock *sk, struct request_sock *req,
const struct inet_request_sock *ireq = inet_rsk(req);
int ttd = req->expires - jiffies;
- seq_printf(f, "%4d: %08X:%04X %08X:%04X"
- " %02X %08X:%08X %02X:%08lX %08X %5d %8d %u %d %p%n",
- i,
- ireq->loc_addr,
- ntohs(inet_sk(sk)->inet_sport),
- ireq->rmt_addr,
- ntohs(ireq->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 */
- atomic_read(&sk->sk_refcnt),
- req,
- len);
+ /* Only expose kernel addresses to privileged readers */
+ if (capable(CAP_NET_ADMIN))
+ seq_printf(f, "%4d: %08X:%04X %08X:%04X"
+ " %02X %08X:%08X %02X:%08lX %08X %5d %8d %u %d %p%n",
+ i,
+ ireq->loc_addr,
+ ntohs(inet_sk(sk)->inet_sport),
+ ireq->rmt_addr,
+ ntohs(ireq->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 */
+ atomic_read(&sk->sk_refcnt),
+ req,
+ len);
+ else
+ seq_printf(f, "%4d: %08X:%04X %08X:%04X"
+ " %02X %08X:%08X %02X:%08lX %08X %5d %8d %u %d %d%n",
+ i,
+ ireq->loc_addr,
+ ntohs(inet_sk(sk)->inet_sport),
+ ireq->rmt_addr,
+ ntohs(ireq->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 */
+ atomic_read(&sk->sk_refcnt),
+ 0,
+ len);
}
static void get_tcp4_sock(struct sock *sk, struct seq_file *f, int i, int *len)
@@ -2443,24 +2466,46 @@ static void get_tcp4_sock(struct sock *sk, struct seq_file *f, int i, int *len)
*/
rx_queue = max_t(int, tp->rcv_nxt - tp->copied_seq, 0);
- seq_printf(f, "%4d: %08X:%04X %08X:%04X %02X %08X:%08X %02X:%08lX "
- "%08X %5d %8d %lu %d %p %lu %lu %u %u %d%n",
- i, src, srcp, dest, destp, sk->sk_state,
- tp->write_seq - tp->snd_una,
- rx_queue,
- timer_active,
- jiffies_to_clock_t(timer_expires - jiffies),
- icsk->icsk_retransmits,
- sock_i_uid(sk),
- icsk->icsk_probes_out,
- sock_i_ino(sk),
- atomic_read(&sk->sk_refcnt), sk,
- 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,
- len);
+ /* Only expose kernel addresses to privileged readers */
+ if (capable(CAP_NET_ADMIN))
+ seq_printf(f, "%4d: %08X:%04X %08X:%04X %02X %08X:%08X "
+ "%02X:%08lX %08X %5d %8d %lu %d %p %lu %lu %u %u %d%n",
+ i, src, srcp, dest, destp, sk->sk_state,
+ tp->write_seq - tp->snd_una,
+ rx_queue,
+ timer_active,
+ jiffies_to_clock_t(timer_expires - jiffies),
+ icsk->icsk_retransmits,
+ sock_i_uid(sk),
+ icsk->icsk_probes_out,
+ sock_i_ino(sk),
+ atomic_read(&sk->sk_refcnt), sk,
+ 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,
+ len);
+ else
+ seq_printf(f, "%4d: %08X:%04X %08X:%04X %02X %08X:%08X "
+ "%02X:%08lX %08X %5d %8d %lu %d %d %lu %lu %u %u %d%n",
+ i, src, srcp, dest, destp, sk->sk_state,
+ tp->write_seq - tp->snd_una,
+ rx_queue,
+ timer_active,
+ jiffies_to_clock_t(timer_expires - jiffies),
+ icsk->icsk_retransmits,
+ sock_i_uid(sk),
+ icsk->icsk_probes_out,
+ sock_i_ino(sk),
+ atomic_read(&sk->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,
+ len);
+
}
static void get_timewait4_sock(struct inet_timewait_sock *tw,
@@ -2478,11 +2523,20 @@ static void get_timewait4_sock(struct inet_timewait_sock *tw,
destp = ntohs(tw->tw_dport);
srcp = ntohs(tw->tw_sport);
- seq_printf(f, "%4d: %08X:%04X %08X:%04X"
- " %02X %08X:%08X %02X:%08lX %08X %5d %8d %d %d %p%n",
- i, src, srcp, dest, destp, tw->tw_substate, 0, 0,
- 3, jiffies_to_clock_t(ttd), 0, 0, 0, 0,
- atomic_read(&tw->tw_refcnt), tw, len);
+ /* Only expose kernel addresses to privileged readers */
+ if (capable(CAP_NET_ADMIN))
+ seq_printf(f, "%4d: %08X:%04X %08X:%04X"
+ " %02X %08X:%08X %02X:%08lX %08X %5d %8d %d %d %p%n",
+ i, src, srcp, dest, destp, tw->tw_substate, 0, 0,
+ 3, jiffies_to_clock_t(ttd), 0, 0, 0, 0,
+ atomic_read(&tw->tw_refcnt), tw, len);
+ else
+ seq_printf(f, "%4d: %08X:%04X %08X:%04X"
+ " %02X %08X:%08X %02X:%08lX %08X %5d %8d %d %d %d%n",
+ i, src, srcp, dest, destp, tw->tw_substate, 0, 0,
+ 3, jiffies_to_clock_t(ttd), 0, 0, 0, 0,
+ atomic_read(&tw->tw_refcnt), 0, len);
+
}
#define TMPSZ 150
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 28cb2d7..41c391d 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2045,14 +2045,26 @@ static void udp4_format_sock(struct sock *sp, struct seq_file *f,
__u16 destp = ntohs(inet->inet_dport);
__u16 srcp = ntohs(inet->inet_sport);
- seq_printf(f, "%5d: %08X:%04X %08X:%04X"
- " %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d%n",
- bucket, src, srcp, dest, destp, sp->sk_state,
- sk_wmem_alloc_get(sp),
- sk_rmem_alloc_get(sp),
- 0, 0L, 0, sock_i_uid(sp), 0, sock_i_ino(sp),
- atomic_read(&sp->sk_refcnt), sp,
- atomic_read(&sp->sk_drops), len);
+ /* Only expose kernel addresses to privileged readers */
+ if (capable(CAP_NET_ADMIN))
+ seq_printf(f, "%5d: %08X:%04X %08X:%04X"
+ " %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d%n",
+ bucket, src, srcp, dest, destp, sp->sk_state,
+ sk_wmem_alloc_get(sp),
+ sk_rmem_alloc_get(sp),
+ 0, 0L, 0, sock_i_uid(sp), 0, sock_i_ino(sp),
+ atomic_read(&sp->sk_refcnt), sp,
+ atomic_read(&sp->sk_drops), len);
+ else
+ seq_printf(f, "%5d: %08X:%04X %08X:%04X"
+ " %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %d %d%n",
+ bucket, src, srcp, dest, destp, sp->sk_state,
+ sk_wmem_alloc_get(sp),
+ sk_rmem_alloc_get(sp),
+ 0, 0L, 0, sock_i_uid(sp), 0, sock_i_ino(sp),
+ atomic_read(&sp->sk_refcnt), 0,
+ atomic_read(&sp->sk_drops), len);
+
}
int udp4_seq_show(struct seq_file *seq, void *v)
^ permalink raw reply related
* [PATCH 4/10] Fix leaking of kernel heap addresses in net/
From: Dan Rosenberg @ 2010-11-12 1:07 UTC (permalink / raw)
To: David S. Miller, Oliver Hartkopp, Alexey Kuznetsov, Urs Thuermann,
Hideaki
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)
^ permalink raw reply related
* [PATCH 5/10] Fix leaking of kernel heap addresses in net/
From: Dan Rosenberg @ 2010-11-12 1:07 UTC (permalink / raw)
To: David S. Miller, Oliver Hartkopp, Alexey Kuznetsov, Urs Thuermann,
Hideaki
diff --git a/net/key/af_key.c b/net/key/af_key.c
index d87c22d..977481a 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -3643,14 +3643,25 @@ static int pfkey_seq_show(struct seq_file *f, void *v)
if (v == SEQ_START_TOKEN)
seq_printf(f ,"sk RefCnt Rmem Wmem User Inode\n");
else
- seq_printf(f ,"%p %-6d %-6u %-6u %-6u %-6lu\n",
- s,
- atomic_read(&s->sk_refcnt),
- sk_rmem_alloc_get(s),
- sk_wmem_alloc_get(s),
- sock_i_uid(s),
- sock_i_ino(s)
- );
+ /* Only expose kernel addresses to privileged readers */
+ if (capable(CAP_NET_ADMIN))
+ seq_printf(f, "%p %-6d %-6u %-6u %-6u %-6lu\n",
+ s,
+ atomic_read(&s->sk_refcnt),
+ sk_rmem_alloc_get(s),
+ sk_wmem_alloc_get(s),
+ sock_i_uid(s),
+ sock_i_ino(s)
+ );
+ else
+ seq_printf(f, "%d %-6d %-6u %-6u %-6u %-6lu\n",
+ 0,
+ atomic_read(&s->sk_refcnt),
+ sk_rmem_alloc_get(s),
+ sk_wmem_alloc_get(s),
+ sock_i_uid(s),
+ sock_i_ino(s)
+ );
return 0;
}
^ permalink raw reply related
* [PATCH 6/10] Fix leaking of kernel heap addresses in net/
From: Dan Rosenberg @ 2010-11-12 1:07 UTC (permalink / raw)
To: David S. Miller, Oliver Hartkopp, Alexey Kuznetsov, Urs Thuermann,
Hideaki
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 478181d..88be3ab 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -1990,18 +1990,35 @@ static int netlink_seq_show(struct seq_file *seq, void *v)
struct sock *s = v;
struct netlink_sock *nlk = nlk_sk(s);
- seq_printf(seq, "%p %-3d %-6d %08x %-8d %-8d %p %-8d %-8d %-8lu\n",
- s,
- s->sk_protocol,
- nlk->pid,
- nlk->groups ? (u32)nlk->groups[0] : 0,
- sk_rmem_alloc_get(s),
- sk_wmem_alloc_get(s),
- nlk->cb,
- atomic_read(&s->sk_refcnt),
- atomic_read(&s->sk_drops),
- sock_i_ino(s)
- );
+ /* Only expose kernel addresses to privileged readers */
+ if (capable(CAP_NET_ADMIN))
+ seq_printf(seq, "%p %-3d %-6d %08x %-8d "
+ "%-8d %p %-8d %-8d %-8lu\n",
+ s,
+ s->sk_protocol,
+ nlk->pid,
+ nlk->groups ? (u32)nlk->groups[0] : 0,
+ sk_rmem_alloc_get(s),
+ sk_wmem_alloc_get(s),
+ nlk->cb,
+ atomic_read(&s->sk_refcnt),
+ atomic_read(&s->sk_drops),
+ sock_i_ino(s)
+ );
+ else
+ seq_printf(seq, "%d %-3d %-6d %08x %-8d "
+ "%-8d %d %-8d %-8d %-8lu\n",
+ 0,
+ s->sk_protocol,
+ nlk->pid,
+ nlk->groups ? (u32)nlk->groups[0] : 0,
+ sk_rmem_alloc_get(s),
+ sk_wmem_alloc_get(s),
+ 0,
+ atomic_read(&s->sk_refcnt),
+ atomic_read(&s->sk_drops),
+ sock_i_ino(s)
+ );
}
return 0;
^ permalink raw reply related
* [PATCH 7/10] Fix leaking of kernel heap addresses in net/
From: Dan Rosenberg @ 2010-11-12 1:07 UTC (permalink / raw)
To: David S. Miller, Oliver Hartkopp, Alexey Kuznetsov, Urs Thuermann,
Hideaki
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 3616f27..3dbf2b5 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2635,17 +2635,31 @@ static int packet_seq_show(struct seq_file *seq, void *v)
struct sock *s = sk_entry(v);
const struct packet_sock *po = pkt_sk(s);
- seq_printf(seq,
- "%p %-6d %-4d %04x %-5d %1d %-6u %-6u %-6lu\n",
- s,
- atomic_read(&s->sk_refcnt),
- s->sk_type,
- ntohs(po->num),
- po->ifindex,
- po->running,
- atomic_read(&s->sk_rmem_alloc),
- sock_i_uid(s),
- sock_i_ino(s));
+ /* Only expose kernel addresses to privileged readers */
+ if (capable(CAP_NET_ADMIN))
+ seq_printf(seq,
+ "%p %-6d %-4d %04x %-5d %1d %-6u %-6u %-6lu\n",
+ s,
+ atomic_read(&s->sk_refcnt),
+ s->sk_type,
+ ntohs(po->num),
+ po->ifindex,
+ po->running,
+ atomic_read(&s->sk_rmem_alloc),
+ sock_i_uid(s),
+ sock_i_ino(s));
+ else
+ seq_printf(seq,
+ "%d %-6d %-4d %04x %-5d %1d %-6u %-6u %-6lu\n",
+ 0,
+ atomic_read(&s->sk_refcnt),
+ s->sk_type,
+ ntohs(po->num),
+ po->ifindex,
+ po->running,
+ atomic_read(&s->sk_rmem_alloc),
+ sock_i_uid(s),
+ sock_i_ino(s));
}
return 0;
^ permalink raw reply related
* [PATCH 8/10] Fix leaking of kernel heap addresses in net/
From: Dan Rosenberg @ 2010-11-12 1:07 UTC (permalink / raw)
To: David S. Miller, Oliver Hartkopp, Alexey Kuznetsov, Urs Thuermann,
Hideaki
diff --git a/net/phonet/socket.c b/net/phonet/socket.c
index 25f746d..2ca2a87 100644
--- a/net/phonet/socket.c
+++ b/net/phonet/socket.c
@@ -631,14 +631,25 @@ static int pn_sock_seq_show(struct seq_file *seq, void *v)
struct sock *sk = v;
struct pn_sock *pn = pn_sk(sk);
- seq_printf(seq, "%2d %04X:%04X:%02X %02X %08X:%08X %5d %lu "
- "%d %p %d%n",
- sk->sk_protocol, pn->sobject, 0, pn->resource,
- sk->sk_state,
- sk_wmem_alloc_get(sk), sk_rmem_alloc_get(sk),
- sock_i_uid(sk), sock_i_ino(sk),
- atomic_read(&sk->sk_refcnt), sk,
- atomic_read(&sk->sk_drops), &len);
+ /* Only expose kernel addresses to privileged readers */
+ if (capable(CAP_NET_ADMIN))
+ seq_printf(seq, "%2d %04X:%04X:%02X %02X %08X:%08X "
+ "%5d %lu %d %p %d%n",
+ sk->sk_protocol, pn->sobject, 0, pn->resource,
+ sk->sk_state,
+ sk_wmem_alloc_get(sk), sk_rmem_alloc_get(sk),
+ sock_i_uid(sk), sock_i_ino(sk),
+ atomic_read(&sk->sk_refcnt), sk,
+ atomic_read(&sk->sk_drops), &len);
+ else
+ seq_printf(seq, "%2d %04X:%04X:%02X %02X %08X:%08X "
+ "%5d %lu %d %d %d%n",
+ sk->sk_protocol, pn->sobject, 0, pn->resource,
+ sk->sk_state,
+ sk_wmem_alloc_get(sk), sk_rmem_alloc_get(sk),
+ sock_i_uid(sk), sock_i_ino(sk),
+ atomic_read(&sk->sk_refcnt), 0,
+ atomic_read(&sk->sk_drops), &len);
}
seq_printf(seq, "%*s\n", 127 - len, "");
return 0;
^ permalink raw reply related
* Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/
From: David Miller @ 2010-11-12 1:10 UTC (permalink / raw)
To: drosenberg-PiUznwcHFHrqlBn2x/YWAg
Cc: urs.thuermann-l29pVbxQd1IUtdQbppsyvg,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
linux-sctp-u79uwXL29TY76Z2rM5mHXA,
shemminger-ZtmgI6mnKB3QT0dZR+AlfA, xemul-GEFAQzZX7r8dnm+yROfE0A,
pekkas-UjJjq++bwZ7HOG6cAo2yLw,
eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
jmorris-gx6/JNMH7DfYtjvyW6yDsg, kuznet-v/Mj1YrvjDBInbfyfbPRSQ,
adobriyan-Re5JQEeQqe8AvxtiuMwx3w, sri-r/Jw6+rmf7HQT0dZR+AlfA,
johannes.berg-ral2JQCrhuEAvxtiuMwx3w, hadi-jkUAjuhPggJWk0Htik3J/w,
vladislav.yasevich-VXdhtT5mjnY, lizf-BthXqXjhjHXQFUHtdCDX3A,
tj-DgEjT+Ai2ygdnm+yROfE0A,
remi.denis-courmont-xNZwKgViW5gAvxtiuMwx3w,
daniel.lezcano-GANU6spQydw, jpirko-H+wXaHxf7aLQT0dZR+AlfA,
yoshfuji-VfPWfsRibaP+Ru+s062T9g, socketcan-fJ+pQTUTwRTk1uMJSBkQmQ,
ebiederm-aS9lmoZGLiVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
joe-6d6DIl74uiNBDgjK7y7TUQ, kaber-dcUjhNyLwpNeoWH0uzbU5w
In-Reply-To: <1289524026.5167.68.camel@dan>
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.
^ permalink raw reply
* [PATCH 9/10] Fix leaking of kernel heap addresses in net/
From: Dan Rosenberg @ 2010-11-12 1:07 UTC (permalink / raw)
To: David S. Miller, Oliver Hartkopp, Alexey Kuznetsov, Urs Thuermann,
Hideaki
diff --git a/net/sctp/proc.c b/net/sctp/proc.c
index 61aacfb..9d28702 100644
--- a/net/sctp/proc.c
+++ b/net/sctp/proc.c
@@ -212,10 +212,20 @@ static int sctp_eps_seq_show(struct seq_file *seq, void *v)
sctp_for_each_hentry(epb, node, &head->chain) {
ep = sctp_ep(epb);
sk = epb->sk;
- seq_printf(seq, "%8p %8p %-3d %-3d %-4d %-5d %5d %5lu ", ep, sk,
- sctp_sk(sk)->type, sk->sk_state, hash,
- epb->bind_addr.port,
- sock_i_uid(sk), sock_i_ino(sk));
+
+ /* Only expose kernel addresses to privileged readers */
+ if (capable(CAP_NET_ADMIN))
+ seq_printf(seq, "%8p %8p %-3d %-3d %-4d %-5d %5d %5lu ",
+ ep, sk,
+ sctp_sk(sk)->type, sk->sk_state, hash,
+ epb->bind_addr.port,
+ sock_i_uid(sk), sock_i_ino(sk));
+ else
+ seq_printf(seq, "%d %d %-3d %-3d %-4d %-5d %5d %5lu ",
+ 0, 0,
+ sctp_sk(sk)->type, sk->sk_state, hash,
+ epb->bind_addr.port,
+ sock_i_uid(sk), sock_i_ino(sk));
sctp_seq_dump_local_addrs(seq, epb);
seq_printf(seq, "\n");
@@ -315,17 +325,33 @@ static int sctp_assocs_seq_show(struct seq_file *seq, void *v)
sctp_for_each_hentry(epb, node, &head->chain) {
assoc = sctp_assoc(epb);
sk = epb->sk;
- seq_printf(seq,
- "%8p %8p %-3d %-3d %-2d %-4d "
- "%4d %8d %8d %7d %5lu %-5d %5d ",
- assoc, sk, sctp_sk(sk)->type, sk->sk_state,
- assoc->state, hash,
- assoc->assoc_id,
- assoc->sndbuf_used,
- atomic_read(&assoc->rmem_alloc),
- sock_i_uid(sk), sock_i_ino(sk),
- epb->bind_addr.port,
- assoc->peer.port);
+
+ /* Only expose kernel addresses to privileged readers */
+ if (capable(CAP_NET_ADMIN))
+ seq_printf(seq,
+ "%8p %8p %-3d %-3d %-2d %-4d "
+ "%4d %8d %8d %7d %5lu %-5d %5d ",
+ assoc, sk, sctp_sk(sk)->type, sk->sk_state,
+ assoc->state, hash,
+ assoc->assoc_id,
+ assoc->sndbuf_used,
+ atomic_read(&assoc->rmem_alloc),
+ sock_i_uid(sk), sock_i_ino(sk),
+ epb->bind_addr.port,
+ assoc->peer.port);
+ else
+ seq_printf(seq,
+ "%d %d %-3d %-3d %-2d %-4d "
+ "%4d %8d %8d %7d %5lu %-5d %5d ",
+ 0, 0, sctp_sk(sk)->type, sk->sk_state,
+ assoc->state, hash,
+ assoc->assoc_id,
+ assoc->sndbuf_used,
+ atomic_read(&assoc->rmem_alloc),
+ sock_i_uid(sk), sock_i_ino(sk),
+ epb->bind_addr.port,
+ assoc->peer.port);
+
seq_printf(seq, " ");
sctp_seq_dump_local_addrs(seq, epb);
seq_printf(seq, "<-> ");
^ permalink raw reply related
* [PATCH 10/10] Fix leaking of kernel heap addresses in net/
From: Dan Rosenberg @ 2010-11-12 1:07 UTC (permalink / raw)
To: David S. Miller, Oliver Hartkopp, Alexey Kuznetsov, Urs Thuermann,
Hideaki
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 3c95304..ba7cf7c 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2198,16 +2198,33 @@ static int unix_seq_show(struct seq_file *seq, void *v)
struct unix_sock *u = unix_sk(s);
unix_state_lock(s);
- seq_printf(seq, "%p: %08X %08X %08X %04X %02X %5lu",
- s,
- atomic_read(&s->sk_refcnt),
- 0,
- s->sk_state == TCP_LISTEN ? __SO_ACCEPTCON : 0,
- s->sk_type,
- s->sk_socket ?
- (s->sk_state == TCP_ESTABLISHED ? SS_CONNECTED : SS_UNCONNECTED) :
- (s->sk_state == TCP_ESTABLISHED ? SS_CONNECTING : SS_DISCONNECTING),
- sock_i_ino(s));
+ /* Only expose kernel addresses to privileged readers */
+ if (capable(CAP_NET_ADMIN))
+ seq_printf(seq, "%p: %08X %08X %08X %04X %02X %5lu",
+ s,
+ atomic_read(&s->sk_refcnt),
+ 0,
+ s->sk_state == TCP_LISTEN ? __SO_ACCEPTCON : 0,
+ s->sk_type,
+ s->sk_socket ?
+ (s->sk_state == TCP_ESTABLISHED ?
+ SS_CONNECTED : SS_UNCONNECTED) :
+ (s->sk_state == TCP_ESTABLISHED ?
+ SS_CONNECTING : SS_DISCONNECTING),
+ sock_i_ino(s));
+ else
+ seq_printf(seq, "%d: %08X %08X %08X %04X %02X %5lu",
+ 0,
+ atomic_read(&s->sk_refcnt),
+ 0,
+ s->sk_state == TCP_LISTEN ? __SO_ACCEPTCON : 0,
+ s->sk_type,
+ s->sk_socket ?
+ (s->sk_state == TCP_ESTABLISHED ?
+ SS_CONNECTED : SS_UNCONNECTED) :
+ (s->sk_state == TCP_ESTABLISHED ?
+ SS_CONNECTING : SS_DISCONNECTING),
+ sock_i_ino(s));
if (u->addr) {
int i, len;
^ permalink raw reply related
* Re: [PATCH 2/10] Fix leaking of kernel heap addresses in net/
From: Stephen Hemminger @ 2010-11-12 1:17 UTC (permalink / raw)
To: Dan Rosenberg
Cc: David S. Miller, Oliver Hartkopp, Alexey Kuznetsov, Urs Thuermann,
Hideaki YOSHIFUJI, Patrick McHardy, James Morris,
Remi Denis-Courmont, Pekka Savola (ipv6), Sridhar Samudrala,
Vlad Yasevich, Tejun Heo, Eric Dumazet, Li Zefan, Joe Perches,
Jamal Hadi Salim, Eric W. Biederman, Alexey Dobriyan, Jiri Pirko,
Johannes Berg, Daniel Lezcano, Pavel Emelyanov, socketcan-core
In-Reply-To: <1289524019.5167.66.camel@dan>
On Thu, 11 Nov 2010 20:06:59 -0500
Dan Rosenberg <drosenberg@vsecurity.com> wrote:
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 08ffe9e..5960ad7 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -165,9 +165,16 @@ static int bcm_proc_show(struct seq_file *m, void *v)
> struct bcm_sock *bo = bcm_sk(sk);
> struct bcm_op *op;
>
> - seq_printf(m, ">>> socket %p", sk->sk_socket);
> - seq_printf(m, " / sk %p", sk);
> - seq_printf(m, " / bo %p", bo);
> + /* Only expose kernel addresses to privileged readers */
> + if (capable(CAP_NET_ADMIN))
> + seq_printf(m, ">>> socket %p", sk->sk_socket);
> + seq_printf(m, " / sk %p", sk);
> + seq_printf(m, " / bo %p", bo);
> + else
> + seq_printf(m, ">>> socket %lu", sock_i_ino(sk));
> + seq_printf(m, " / sk %d", 0);
> + seq_printf(m, " / bo %d", 0);
> +
Printing different data based on security state seems like an ABI
nightmare.
--
^ permalink raw reply
* Re: [PATCH 4/10] Fix leaking of kernel heap addresses in net/
From: Dan Rosenberg @ 2010-11-12 1:20 UTC (permalink / raw)
To: David Miller
Cc: socketcan, kuznet, urs.thuermann, yoshfuji, kaber, jmorris,
remi.denis-courmont, pekkas, sri, vladislav.yasevich, tj,
eric.dumazet, lizf, joe, shemminger, hadi, ebiederm, adobriyan,
jpirko, johannes.berg, daniel.lezcano, xemul, socketcan-core,
netdev, linux-sctp
In-Reply-To: <20101111.171024.242132239.davem@davemloft.net>
> 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
^ permalink raw reply
* Re: [PATCH 3/10] Fix leaking of kernel heap addresses in net/
From: Thomas Graf @ 2010-11-12 1:20 UTC (permalink / raw)
To: Dan Rosenberg
Cc: David S. Miller, Oliver Hartkopp, Alexey Kuznetsov, Urs Thuermann,
Hideaki YOSHIFUJI, Patrick McHardy, James Morris,
Remi Denis-Courmont, Pekka Savola (ipv6), Sridhar Samudrala,
Vlad Yasevich, Tejun Heo, Eric Dumazet, Li Zefan, Joe Perches,
Stephen Hemminger, Jamal Hadi Salim, Eric W. Biederman,
Alexey Dobriyan, Jiri Pirko, Johannes Berg, Daniel Lezcano,
Pavel
In-Reply-To: <1289524023.5167.67.camel@dan>
On Thu, Nov 11, 2010 at 08:07:03PM -0500, Dan Rosenberg wrote:
> diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
> index 1f85ef2..0ac8ff2 100644
> --- a/net/ipv4/raw.c
> +++ b/net/ipv4/raw.c
> @@ -948,13 +948,26 @@ static void raw_sock_seq_show(struct seq_file *seq, struct sock *sp, int i)
> __u16 destp = 0,
> srcp = inet->inet_num;
>
> - seq_printf(seq, "%4d: %08X:%04X %08X:%04X"
> - " %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d\n",
> - i, src, srcp, dest, destp, sp->sk_state,
> - sk_wmem_alloc_get(sp),
> - sk_rmem_alloc_get(sp),
> - 0, 0L, 0, sock_i_uid(sp), 0, sock_i_ino(sp),
> - atomic_read(&sp->sk_refcnt), sp, atomic_read(&sp->sk_drops));
> + /* Only expose kernel addresses to privileged readers */
> + if (capable(CAP_NET_ADMIN))
> + seq_printf(seq, "%4d: %08X:%04X %08X:%04X"
> + " %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %p %d\n",
> + i, src, srcp, dest, destp, sp->sk_state,
> + sk_wmem_alloc_get(sp),
> + sk_rmem_alloc_get(sp),
> + 0, 0L, 0, sock_i_uid(sp), 0, sock_i_ino(sp),
> + atomic_read(&sp->sk_refcnt),
> + sp, atomic_read(&sp->sk_drops));
> + else
> + seq_printf(seq, "%4d: %08X:%04X %08X:%04X"
> + " %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %d %d\n",
> + i, src, srcp, dest, destp, sp->sk_state,
> + sk_wmem_alloc_get(sp),
> + sk_rmem_alloc_get(sp),
> + 0, 0L, 0, sock_i_uid(sp), 0, sock_i_ino(sp),
> + atomic_read(&sp->sk_refcnt),
> + 0, atomic_read(&sp->sk_drops));
If we really have to do this. At least don't duplicate all this code. Do
the check in the printf argument:
seq_printf(seq, "%4d: %08X:%04X %08X:%04X"
...
capable(CAP_NET_ADMIN) ? sp : 0,
I would even move the decision whether to expose kernel addresses or not
to a function so we can change behavior in one place.
^ permalink raw reply
* Re: [PATCH 2/10] Fix leaking of kernel heap addresses in net/
From: Dan Rosenberg @ 2010-11-12 1:22 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David S. Miller, Oliver Hartkopp, Alexey Kuznetsov, Urs Thuermann,
Hideaki YOSHIFUJI, Patrick McHardy, James Morris,
Remi Denis-Courmont, Pekka Savola (ipv6), Sridhar Samudrala,
Vlad Yasevich, Tejun Heo, Eric Dumazet, Li Zefan, Joe Perches,
Jamal Hadi Salim, Eric W. Biederman, Alexey Dobriyan, Jiri Pirko,
Johannes Berg, Daniel Lezcano, Pavel Emelyanov, socketcan-core
In-Reply-To: <20101111171754.0198e151@nehalam>
>
> Printing different data based on security state seems like an ABI
> nightmare.
>
I can't remove the data entirely, because that would seriously break the
ABI. I deliberately kept the same format so as not to break any
userspace programs relying on consistent output - are there really
programs that would break when they read a 0 instead of an address?
-Dan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox