* [PATCH] infinite recursion in netlink
@ 2007-04-25 18:38 Alexey Kuznetsov
2007-04-25 19:59 ` Greg KH
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: Alexey Kuznetsov @ 2007-04-25 18:38 UTC (permalink / raw)
To: davem, security, greg, netdev; +Cc: jaco
Hello!
Reply to NETLINK_FIB_LOOKUP messages were misrouted back to kernel,
which resulted in infinite recursion and stack overflow.
The bug is present in all kernel versions since the feature appeared.
The patch also makes some minimal cleanup:
1. Return something consistent (-ENOENT) when fib table is missing
2. Do not crash when queue is empty (does not happen, but yet)
3. Put result of lookup
Signed-off-by: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index fc920f6..cac06c4 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -776,6 +776,8 @@ static void nl_fib_lookup(struct fib_res
.nl_u = { .ip4_u = { .daddr = frn->fl_addr,
.tos = frn->fl_tos,
.scope = frn->fl_scope } } };
+
+ frn->err = -ENOENT;
if (tb) {
local_bh_disable();
@@ -787,6 +789,7 @@ static void nl_fib_lookup(struct fib_res
frn->nh_sel = res.nh_sel;
frn->type = res.type;
frn->scope = res.scope;
+ fib_res_put(&res);
}
local_bh_enable();
}
@@ -801,6 +804,9 @@ static void nl_fib_input(struct sock *sk
struct fib_table *tb;
skb = skb_dequeue(&sk->sk_receive_queue);
+ if (skb == NULL)
+ return;
+
nlh = (struct nlmsghdr *)skb->data;
if (skb->len < NLMSG_SPACE(0) || skb->len < nlh->nlmsg_len ||
nlh->nlmsg_len < NLMSG_LENGTH(sizeof(*frn))) {
@@ -813,7 +819,7 @@ static void nl_fib_input(struct sock *sk
nl_fib_lookup(frn, tb);
- pid = nlh->nlmsg_pid; /*pid of sending process */
+ pid = NETLINK_CB(skb).pid; /* pid of sending process */
NETLINK_CB(skb).pid = 0; /* from kernel */
NETLINK_CB(skb).dst_group = 0; /* unicast */
netlink_unicast(sk, skb, pid, MSG_DONTWAIT);
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] infinite recursion in netlink
2007-04-25 18:38 [PATCH] infinite recursion in netlink Alexey Kuznetsov
@ 2007-04-25 19:59 ` Greg KH
2007-04-25 20:05 ` David Miller
2007-04-25 22:21 ` Jaco Kroon
2007-04-25 20:09 ` David Miller
2007-04-25 20:15 ` [Security] " Linus Torvalds
2 siblings, 2 replies; 19+ messages in thread
From: Greg KH @ 2007-04-25 19:59 UTC (permalink / raw)
To: Alexey Kuznetsov; +Cc: davem, security, netdev, jaco
On Wed, Apr 25, 2007 at 10:38:56PM +0400, Alexey Kuznetsov wrote:
> Hello!
>
> Reply to NETLINK_FIB_LOOKUP messages were misrouted back to kernel,
> which resulted in infinite recursion and stack overflow.
>
> The bug is present in all kernel versions since the feature appeared.
Any hint on when this feature appeared so that we can notify the distros
for older releases?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] infinite recursion in netlink
2007-04-25 19:59 ` Greg KH
@ 2007-04-25 20:05 ` David Miller
2007-04-25 22:21 ` Jaco Kroon
1 sibling, 0 replies; 19+ messages in thread
From: David Miller @ 2007-04-25 20:05 UTC (permalink / raw)
To: greg; +Cc: kuznet, security, netdev, jaco
From: Greg KH <greg@kroah.com>
Date: Wed, 25 Apr 2007 12:59:41 -0700
> On Wed, Apr 25, 2007 at 10:38:56PM +0400, Alexey Kuznetsov wrote:
> > Hello!
> >
> > Reply to NETLINK_FIB_LOOKUP messages were misrouted back to kernel,
> > which resulted in infinite recursion and stack overflow.
> >
> > The bug is present in all kernel versions since the feature appeared.
>
> Any hint on when this feature appeared so that we can notify the distros
> for older releases?
It's been there since Jun 20th, 2005
commit 246955fe4c38bd706ae30e37c64892c94213775d
Author: Robert Olsson <Robert.Olsson@data.slu.se>
Date: Mon Jun 20 13:36:39 2005 -0700
[NETLINK]: fib_lookup() via netlink
Below is a more generic patch to do fib_lookup via netlink. For others
we should say that we discussed this as a way to verify route selection.
It's also possible there are others uses for this.
In short the fist half of struct fib_result_nl is filled in by caller
and netlink call fills in the other half and returns it.
In case anyone is interested there is a corresponding user app to compare
the full routing table this was used to test implementation of the LC-trie.
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/include/linux/netlink.h b/include/linux/netlink.h
index e38407a..561d4dc 100644
--- a/include/linux/netlink.h
+++ b/include/linux/netlink.h
@@ -14,6 +14,7 @@
#define NETLINK_SELINUX 7 /* SELinux event notifications */
#define NETLINK_ARPD 8
#define NETLINK_AUDIT 9 /* auditing */
+#define NETLINK_FIB_LOOKUP 10
#define NETLINK_ROUTE6 11 /* af_inet6 route comm channel */
#define NETLINK_IP6_FW 13
#define NETLINK_DNRTMSG 14 /* DECnet routing messages */
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index e5a5f6b..a4208a3 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -109,6 +109,20 @@ struct fib_result {
#endif
};
+struct fib_result_nl {
+ u32 fl_addr; /* To be looked up*/
+ u32 fl_fwmark;
+ unsigned char fl_tos;
+ unsigned char fl_scope;
+ unsigned char tb_id_in;
+
+ unsigned char tb_id; /* Results */
+ unsigned char prefixlen;
+ unsigned char nh_sel;
+ unsigned char type;
+ unsigned char scope;
+ int err;
+};
#ifdef CONFIG_IP_ROUTE_MULTIPATH
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 563e7d6..cd8e45a 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -516,6 +516,60 @@ static void fib_del_ifaddr(struct in_ifaddr *ifa)
#undef BRD1_OK
}
+static void nl_fib_lookup(struct fib_result_nl *frn, struct fib_table *tb )
+{
+
+ struct fib_result res;
+ struct flowi fl = { .nl_u = { .ip4_u = { .daddr = frn->fl_addr,
+ .fwmark = frn->fl_fwmark,
+ .tos = frn->fl_tos,
+ .scope = frn->fl_scope } } };
+ if (tb) {
+ local_bh_disable();
+
+ frn->tb_id = tb->tb_id;
+ frn->err = tb->tb_lookup(tb, &fl, &res);
+
+ if (!frn->err) {
+ frn->prefixlen = res.prefixlen;
+ frn->nh_sel = res.nh_sel;
+ frn->type = res.type;
+ frn->scope = res.scope;
+ }
+ local_bh_enable();
+ }
+}
+
+static void nl_fib_input(struct sock *sk, int len)
+{
+ struct sk_buff *skb = NULL;
+ struct nlmsghdr *nlh = NULL;
+ struct fib_result_nl *frn;
+ int err;
+ u32 pid;
+ struct fib_table *tb;
+
+ skb = skb_recv_datagram(sk, 0, 0, &err);
+ nlh = (struct nlmsghdr *)skb->data;
+
+ frn = (struct fib_result_nl *) NLMSG_DATA(nlh);
+ tb = fib_get_table(frn->tb_id_in);
+
+ nl_fib_lookup(frn, tb);
+
+ pid = nlh->nlmsg_pid; /*pid of sending process */
+ NETLINK_CB(skb).groups = 0; /* not in mcast group */
+ NETLINK_CB(skb).pid = 0; /* from kernel */
+ NETLINK_CB(skb).dst_pid = pid;
+ NETLINK_CB(skb).dst_groups = 0; /* unicast */
+ netlink_unicast(sk, skb, pid, MSG_DONTWAIT);
+}
+
+static void nl_fib_lookup_init(void)
+{
+ netlink_kernel_create(NETLINK_FIB_LOOKUP, nl_fib_input);
+}
+
static void fib_disable_ip(struct net_device *dev, int force)
{
if (fib_sync_down(0, dev, force))
@@ -604,6 +658,7 @@ void __init ip_fib_init(void)
register_netdevice_notifier(&fib_netdev_notifier);
register_inetaddr_notifier(&fib_inetaddr_notifier);
+ nl_fib_lookup_init();
}
EXPORT_SYMBOL(inet_addr_type);
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] infinite recursion in netlink
2007-04-25 18:38 [PATCH] infinite recursion in netlink Alexey Kuznetsov
2007-04-25 19:59 ` Greg KH
@ 2007-04-25 20:09 ` David Miller
2007-04-25 20:15 ` [Security] " Linus Torvalds
2 siblings, 0 replies; 19+ messages in thread
From: David Miller @ 2007-04-25 20:09 UTC (permalink / raw)
To: kuznet; +Cc: security, greg, netdev, jaco
From: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Date: Wed, 25 Apr 2007 22:38:56 +0400
> Reply to NETLINK_FIB_LOOKUP messages were misrouted back to kernel,
> which resulted in infinite recursion and stack overflow.
>
> The bug is present in all kernel versions since the feature appeared.
>
> The patch also makes some minimal cleanup:
>
> 1. Return something consistent (-ENOENT) when fib table is missing
> 2. Do not crash when queue is empty (does not happen, but yet)
> 3. Put result of lookup
>
> Signed-off-by: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Applied, thanks a lot Alexey.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Security] [PATCH] infinite recursion in netlink
2007-04-25 18:38 [PATCH] infinite recursion in netlink Alexey Kuznetsov
2007-04-25 19:59 ` Greg KH
2007-04-25 20:09 ` David Miller
@ 2007-04-25 20:15 ` Linus Torvalds
2007-04-25 20:18 ` David Miller
2007-04-26 5:29 ` Greg KH
2 siblings, 2 replies; 19+ messages in thread
From: Linus Torvalds @ 2007-04-25 20:15 UTC (permalink / raw)
To: Alexey Kuznetsov; +Cc: davem, security, greg, netdev, jaco
On Wed, 25 Apr 2007, Alexey Kuznetsov wrote:
>
> Reply to NETLINK_FIB_LOOKUP messages were misrouted back to kernel,
> which resulted in infinite recursion and stack overflow.
So I assume it's this line that actually _fixes_ it:
> - pid = nlh->nlmsg_pid; /*pid of sending process */
> + pid = NETLINK_CB(skb).pid; /* pid of sending process */
> NETLINK_CB(skb).pid = 0; /* from kernel */
> NETLINK_CB(skb).dst_group = 0; /* unicast */
> netlink_unicast(sk, skb, pid, MSG_DONTWAIT);
No?
If so, shouldn't we also have some safety-net to make sure it doesn't
still get routed back forever, ie adding something like
if (!pid) {
skb_free(skb);
return -EINVAL;
}
or similar? I don't know the netlink layer from a dolphin, but if the old
code could cause infinite recursion, it sounds like the new code could too
with the right pid, since the only change is the choice of pid.
Yes/No/This is why Linus is a dickweed and doesn't understand the problem?
Linus
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Security] [PATCH] infinite recursion in netlink
2007-04-25 20:15 ` [Security] " Linus Torvalds
@ 2007-04-25 20:18 ` David Miller
2007-04-26 5:29 ` Greg KH
1 sibling, 0 replies; 19+ messages in thread
From: David Miller @ 2007-04-25 20:18 UTC (permalink / raw)
To: torvalds; +Cc: kuznet, security, greg, netdev, jaco
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Wed, 25 Apr 2007 13:15:12 -0700 (PDT)
> If so, shouldn't we also have some safety-net to make sure it doesn't
> still get routed back forever, ie adding something like
>
> if (!pid) {
> skb_free(skb);
> return -EINVAL;
> }
>
> or similar? I don't know the netlink layer from a dolphin, but if the old
> code could cause infinite recursion, it sounds like the new code could too
> with the right pid, since the only change is the choice of pid.
Netlink pids are more like "port numbers" in the socket sense, do
not confuse them with process pids or similar.
The kernel explicitly assigns them to sockets, and zero is special.
The fact that the process pid of the socket creator is used as
an initial selection heuristic, is just that, a heuristic.
Alexey's fix is %100 the right way to go IMHO.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] infinite recursion in netlink
2007-04-25 19:59 ` Greg KH
2007-04-25 20:05 ` David Miller
@ 2007-04-25 22:21 ` Jaco Kroon
1 sibling, 0 replies; 19+ messages in thread
From: Jaco Kroon @ 2007-04-25 22:21 UTC (permalink / raw)
To: Greg KH; +Cc: Alexey Kuznetsov, davem, security, netdev
Greg KH wrote:
> On Wed, Apr 25, 2007 at 10:38:56PM +0400, Alexey Kuznetsov wrote:
>> Hello!
>>
>> Reply to NETLINK_FIB_LOOKUP messages were misrouted back to kernel,
>> which resulted in infinite recursion and stack overflow.
>>
>> The bug is present in all kernel versions since the feature appeared.
>
> Any hint on when this feature appeared so that we can notify the distros
> for older releases?
>
> thanks,
2.6.13 if I'm not mistaken, confirmed on debian testing by Simeon
Miteff. From man 7 netlink:
NETLINK_W1 and NETLINK_FIB_LOOKUP appeared in Linux 2.6.13.
Jaco
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Security] [PATCH] infinite recursion in netlink
2007-04-25 20:15 ` [Security] " Linus Torvalds
2007-04-25 20:18 ` David Miller
@ 2007-04-26 5:29 ` Greg KH
2007-04-26 5:32 ` David Miller
` (2 more replies)
1 sibling, 3 replies; 19+ messages in thread
From: Greg KH @ 2007-04-26 5:29 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Alexey Kuznetsov, davem, security, netdev, jaco
On Wed, Apr 25, 2007 at 01:15:12PM -0700, Linus Torvalds wrote:
>
>
> On Wed, 25 Apr 2007, Alexey Kuznetsov wrote:
> >
> > Reply to NETLINK_FIB_LOOKUP messages were misrouted back to kernel,
> > which resulted in infinite recursion and stack overflow.
Wait, I just had the bright idea of actually testing this before I
pushed out a 2.6.20.9 kernel with another fix in it, and nope, still
crashes, even with this patch :(
Full stackdump in a picture (forgot to have netconsole running) at:
http://www.kroah.com/netlink_oops.jpg
Any thoughts?
I'll go try 2.6.21 now too...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Security] [PATCH] infinite recursion in netlink
2007-04-26 5:29 ` Greg KH
@ 2007-04-26 5:32 ` David Miller
2007-04-26 5:44 ` Greg KH
2007-04-26 5:37 ` Chris Wright
2007-04-26 15:44 ` [PATCH] [IPV4] nl_fib_lookup: Initialise res.r before fib_res_put(&res) Sergey Vlasov
2 siblings, 1 reply; 19+ messages in thread
From: David Miller @ 2007-04-26 5:32 UTC (permalink / raw)
To: greg; +Cc: torvalds, kuznet, security, netdev, jaco
From: Greg KH <greg@kroah.com>
Date: Wed, 25 Apr 2007 22:29:12 -0700
> On Wed, Apr 25, 2007 at 01:15:12PM -0700, Linus Torvalds wrote:
> >
> >
> > On Wed, 25 Apr 2007, Alexey Kuznetsov wrote:
> > >
> > > Reply to NETLINK_FIB_LOOKUP messages were misrouted back to kernel,
> > > which resulted in infinite recursion and stack overflow.
>
> Wait, I just had the bright idea of actually testing this before I
> pushed out a 2.6.20.9 kernel with another fix in it, and nope, still
> crashes, even with this patch :(
>
> Full stackdump in a picture (forgot to have netconsole running) at:
> http://www.kroah.com/netlink_oops.jpg
>
> Any thoughts?
>
> I'll go try 2.6.21 now too...
Crap. We should have let this one simmer for a day to get
more eyes on it.
Thanks for catching this Greg.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Security] [PATCH] infinite recursion in netlink
2007-04-26 5:29 ` Greg KH
2007-04-26 5:32 ` David Miller
@ 2007-04-26 5:37 ` Chris Wright
2007-04-26 15:44 ` [PATCH] [IPV4] nl_fib_lookup: Initialise res.r before fib_res_put(&res) Sergey Vlasov
2 siblings, 0 replies; 19+ messages in thread
From: Chris Wright @ 2007-04-26 5:37 UTC (permalink / raw)
To: Greg KH; +Cc: Linus Torvalds, Alexey Kuznetsov, security, davem, jaco, netdev
* Greg KH (greg@kroah.com) wrote:
> On Wed, Apr 25, 2007 at 01:15:12PM -0700, Linus Torvalds wrote:
> >
> >
> > On Wed, 25 Apr 2007, Alexey Kuznetsov wrote:
> > >
> > > Reply to NETLINK_FIB_LOOKUP messages were misrouted back to kernel,
> > > which resulted in infinite recursion and stack overflow.
>
> Wait, I just had the bright idea of actually testing this before I
> pushed out a 2.6.20.9 kernel with another fix in it, and nope, still
> crashes, even with this patch :(
Odd, I tested it too (on linus-git), and it's fixed (it was definitely
the problem, of sending back to kernel).
thanks,
-chris
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Security] [PATCH] infinite recursion in netlink
2007-04-26 5:32 ` David Miller
@ 2007-04-26 5:44 ` Greg KH
2007-04-26 5:48 ` Greg KH
0 siblings, 1 reply; 19+ messages in thread
From: Greg KH @ 2007-04-26 5:44 UTC (permalink / raw)
To: David Miller; +Cc: torvalds, kuznet, security, netdev, jaco
On Wed, Apr 25, 2007 at 10:32:01PM -0700, David Miller wrote:
> From: Greg KH <greg@kroah.com>
> Date: Wed, 25 Apr 2007 22:29:12 -0700
>
> > On Wed, Apr 25, 2007 at 01:15:12PM -0700, Linus Torvalds wrote:
> > >
> > >
> > > On Wed, 25 Apr 2007, Alexey Kuznetsov wrote:
> > > >
> > > > Reply to NETLINK_FIB_LOOKUP messages were misrouted back to kernel,
> > > > which resulted in infinite recursion and stack overflow.
> >
> > Wait, I just had the bright idea of actually testing this before I
> > pushed out a 2.6.20.9 kernel with another fix in it, and nope, still
> > crashes, even with this patch :(
> >
> > Full stackdump in a picture (forgot to have netconsole running) at:
> > http://www.kroah.com/netlink_oops.jpg
> >
> > Any thoughts?
> >
> > I'll go try 2.6.21 now too...
>
> Crap. We should have let this one simmer for a day to get
> more eyes on it.
>
> Thanks for catching this Greg.
Odd, 2.6.21 doesn't crash at all.
Can anyone verify that I made the 2.6.20.8 release correctly with the
proper patch?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Security] [PATCH] infinite recursion in netlink
2007-04-26 5:44 ` Greg KH
@ 2007-04-26 5:48 ` Greg KH
2007-04-26 5:52 ` Chris Wright
0 siblings, 1 reply; 19+ messages in thread
From: Greg KH @ 2007-04-26 5:48 UTC (permalink / raw)
To: David Miller; +Cc: torvalds, kuznet, security, netdev, jaco
On Wed, Apr 25, 2007 at 10:44:20PM -0700, Greg KH wrote:
> On Wed, Apr 25, 2007 at 10:32:01PM -0700, David Miller wrote:
> > From: Greg KH <greg@kroah.com>
> > Date: Wed, 25 Apr 2007 22:29:12 -0700
> >
> > > On Wed, Apr 25, 2007 at 01:15:12PM -0700, Linus Torvalds wrote:
> > > >
> > > >
> > > > On Wed, 25 Apr 2007, Alexey Kuznetsov wrote:
> > > > >
> > > > > Reply to NETLINK_FIB_LOOKUP messages were misrouted back to kernel,
> > > > > which resulted in infinite recursion and stack overflow.
> > >
> > > Wait, I just had the bright idea of actually testing this before I
> > > pushed out a 2.6.20.9 kernel with another fix in it, and nope, still
> > > crashes, even with this patch :(
> > >
> > > Full stackdump in a picture (forgot to have netconsole running) at:
> > > http://www.kroah.com/netlink_oops.jpg
> > >
> > > Any thoughts?
> > >
> > > I'll go try 2.6.21 now too...
> >
> > Crap. We should have let this one simmer for a day to get
> > more eyes on it.
> >
> > Thanks for catching this Greg.
>
> Odd, 2.6.21 doesn't crash at all.
>
> Can anyone verify that I made the 2.6.20.8 release correctly with the
> proper patch?
fyi, here's the patch that I applied, perhaps 2.6.20 needed something
else too?
thanks,
greg k-h
Subject: NETLINK: Infinite recursion in netlink.
From: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
[NETLINK]: Infinite recursion in netlink.
Reply to NETLINK_FIB_LOOKUP messages were misrouted back to kernel,
which resulted in infinite recursion and stack overflow.
The bug is present in all kernel versions since the feature appeared.
The patch also makes some minimal cleanup:
1. Return something consistent (-ENOENT) when fib table is missing
2. Do not crash when queue is empty (does not happen, but yet)
3. Put result of lookup
Signed-off-by: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
---
net/ipv4/fib_frontend.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -772,6 +772,8 @@ static void nl_fib_lookup(struct fib_res
.nl_u = { .ip4_u = { .daddr = frn->fl_addr,
.tos = frn->fl_tos,
.scope = frn->fl_scope } } };
+
+ frn->err = -ENOENT;
if (tb) {
local_bh_disable();
@@ -783,6 +785,7 @@ static void nl_fib_lookup(struct fib_res
frn->nh_sel = res.nh_sel;
frn->type = res.type;
frn->scope = res.scope;
+ fib_res_put(&res);
}
local_bh_enable();
}
@@ -797,6 +800,9 @@ static void nl_fib_input(struct sock *sk
struct fib_table *tb;
skb = skb_dequeue(&sk->sk_receive_queue);
+ if (skb == NULL)
+ return;
+
nlh = (struct nlmsghdr *)skb->data;
if (skb->len < NLMSG_SPACE(0) || skb->len < nlh->nlmsg_len ||
nlh->nlmsg_len < NLMSG_LENGTH(sizeof(*frn))) {
@@ -809,7 +815,7 @@ static void nl_fib_input(struct sock *sk
nl_fib_lookup(frn, tb);
- pid = nlh->nlmsg_pid; /*pid of sending process */
+ pid = NETLINK_CB(skb).pid; /* pid of sending process */
NETLINK_CB(skb).pid = 0; /* from kernel */
NETLINK_CB(skb).dst_group = 0; /* unicast */
netlink_unicast(sk, skb, pid, MSG_DONTWAIT);
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Security] [PATCH] infinite recursion in netlink
2007-04-26 5:48 ` Greg KH
@ 2007-04-26 5:52 ` Chris Wright
2007-04-26 6:26 ` Chris Wright
0 siblings, 1 reply; 19+ messages in thread
From: Chris Wright @ 2007-04-26 5:52 UTC (permalink / raw)
To: Greg KH; +Cc: David Miller, kuznet, security, torvalds, jaco, netdev
* Greg KH (greg@kroah.com) wrote:
> fyi, here's the patch that I applied, perhaps 2.6.20 needed something
> else too?
<snip>
> @@ -809,7 +815,7 @@ static void nl_fib_input(struct sock *sk
>
> nl_fib_lookup(frn, tb);
>
> - pid = nlh->nlmsg_pid; /*pid of sending process */
> + pid = NETLINK_CB(skb).pid; /* pid of sending process */
That's the important bit. I'm testing against 2.6.20.8 right now.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Security] [PATCH] infinite recursion in netlink
2007-04-26 5:52 ` Chris Wright
@ 2007-04-26 6:26 ` Chris Wright
2007-04-26 6:31 ` David Miller
0 siblings, 1 reply; 19+ messages in thread
From: Chris Wright @ 2007-04-26 6:26 UTC (permalink / raw)
To: Greg KH; +Cc: security, netdev, jaco, kuznet, torvalds, David Miller
* Chris Wright (chrisw@sous-sol.org) wrote:
> * Greg KH (greg@kroah.com) wrote:
> > fyi, here's the patch that I applied, perhaps 2.6.20 needed something
> > else too?
> <snip>
> > @@ -809,7 +815,7 @@ static void nl_fib_input(struct sock *sk
> >
> > nl_fib_lookup(frn, tb);
> >
> > - pid = nlh->nlmsg_pid; /*pid of sending process */
> > + pid = NETLINK_CB(skb).pid; /* pid of sending process */
>
> That's the important bit. I'm testing against 2.6.20.8 right now.
Working fine here. Any chance you booted a stale kernel?
If not, what's your nl_fib_input+0xe4. Any chance that's
actually in nl_fib_lookup?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Security] [PATCH] infinite recursion in netlink
2007-04-26 6:26 ` Chris Wright
@ 2007-04-26 6:31 ` David Miller
2007-04-26 6:51 ` Greg KH
0 siblings, 1 reply; 19+ messages in thread
From: David Miller @ 2007-04-26 6:31 UTC (permalink / raw)
To: chrisw; +Cc: greg, security, netdev, jaco, kuznet, torvalds
From: Chris Wright <chrisw@sous-sol.org>
Date: Wed, 25 Apr 2007 23:26:01 -0700
> Working fine here. Any chance you booted a stale kernel?
> If not, what's your nl_fib_input+0xe4. Any chance that's
> actually in nl_fib_lookup?
I'm seriously hoping it's a stale kernel or similar,
because I can't account for it any other way :-)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Security] [PATCH] infinite recursion in netlink
2007-04-26 6:31 ` David Miller
@ 2007-04-26 6:51 ` Greg KH
2007-04-26 7:02 ` David Miller
0 siblings, 1 reply; 19+ messages in thread
From: Greg KH @ 2007-04-26 6:51 UTC (permalink / raw)
To: David Miller; +Cc: chrisw, security, netdev, jaco, kuznet, torvalds
On Wed, Apr 25, 2007 at 11:31:21PM -0700, David Miller wrote:
> From: Chris Wright <chrisw@sous-sol.org>
> Date: Wed, 25 Apr 2007 23:26:01 -0700
>
> > Working fine here. Any chance you booted a stale kernel?
> > If not, what's your nl_fib_input+0xe4. Any chance that's
> > actually in nl_fib_lookup?
>
> I'm seriously hoping it's a stale kernel or similar,
> because I can't account for it any other way :-)
I did a 'make clean' and rebuilt 2.6.20.9 and I can't duplicate this
again.
Sorry for the false alarm, I have no idea what when wrong here. Glad
the bug is really fixed.
Now to push 2.6.20.9 out...
again, sorry,
greg k-h
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Security] [PATCH] infinite recursion in netlink
2007-04-26 6:51 ` Greg KH
@ 2007-04-26 7:02 ` David Miller
0 siblings, 0 replies; 19+ messages in thread
From: David Miller @ 2007-04-26 7:02 UTC (permalink / raw)
To: greg; +Cc: chrisw, security, netdev, jaco, kuznet, torvalds
From: Greg KH <greg@kroah.com>
Date: Wed, 25 Apr 2007 23:51:51 -0700
> Sorry for the false alarm, I have no idea what when wrong here. Glad
> the bug is really fixed.
Nothing to be sorry about, it's great that you double checked
things even if it turned out to be a false alarm in the end.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] [IPV4] nl_fib_lookup: Initialise res.r before fib_res_put(&res)
2007-04-26 5:29 ` Greg KH
2007-04-26 5:32 ` David Miller
2007-04-26 5:37 ` Chris Wright
@ 2007-04-26 15:44 ` Sergey Vlasov
2007-04-26 16:11 ` Alexey Kuznetsov
2 siblings, 1 reply; 19+ messages in thread
From: Sergey Vlasov @ 2007-04-26 15:44 UTC (permalink / raw)
To: Greg KH; +Cc: Linus Torvalds, Alexey Kuznetsov, davem, security, netdev, jaco
When CONFIG_IP_MULTIPLE_TABLES is enabled, the code in nl_fib_lookup()
needs to initialize the res.r field before fib_res_put(&res) - unlike
fib_lookup(), a direct call to ->tb_lookup does not set this field.
Signed-off-by: Sergey Vlasov <vsu@altlinux.ru>
---
net/ipv4/fib_frontend.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
On Wed, 25 Apr 2007 22:29:12 -0700 Greg KH wrote:
> On Wed, Apr 25, 2007 at 01:15:12PM -0700, Linus Torvalds wrote:
> >
> >
> > On Wed, 25 Apr 2007, Alexey Kuznetsov wrote:
> > >
> > > Reply to NETLINK_FIB_LOOKUP messages were misrouted back to kernel,
> > > which resulted in infinite recursion and stack overflow.
>
> Wait, I just had the bright idea of actually testing this before I
> pushed out a 2.6.20.9 kernel with another fix in it, and nope, still
> crashes, even with this patch :(
>
> Full stackdump in a picture (forgot to have netconsole running) at:
> http://www.kroah.com/netlink_oops.jpg
Here is an oops from the 2.6.18 backport of that patch (just adjusted
for whitespace changes):
BUG: unable to handle kernel paging request at virtual address 80000007
printing eip:
c027a9f7
*pde = 00000000
Oops: 0002 [#1]
SMP
Modules linked in: nfs lockd nfs_acl sunrpc af_packet dm_mod rtc tsdev psmouse ne2k_pci i2c_piix4 ide_cd cdrom serio_raw 8390 pcspkr i2c_core evdev ext2 mbcache ide_generic generic ide_disk piix ide_core
CPU: 0
EIP: 0060:[<c027a9f7>] Not tainted VLI
EFLAGS: 00010292 (2.6.18-std-smp-alt5.2 #2)
EIP is at fib_rule_put+0x6/0x38
eax: 7fffffff ebx: d626de10 ecx: 00000000 edx: 7fffffff
esi: d7f523c0 edi: d6126bc0 ebp: d6909d1c esp: d6909d08
ds: 007b es: 007b ss: 0068
Process netlink1 (pid: 1960, ti=d6908000 task=d7ec8bd0 task.ti=d6908000)
Stack: d6126bc0 d6909d1c c0276778 00000202 d7e31400 00000000 00000000 00000000
00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000 00000000 00000000 00000000 00000000 00010000
Call Trace:
[<c0276778>] nl_fib_input+0xf2/0x128
[<c024a230>] netlink_data_ready+0x12/0x4c
[<c0249234>] netlink_sendskb+0x19/0x30
[<c024a212>] netlink_sendmsg+0x278/0x284
[<c022cdc1>] sock_sendmsg+0xd0/0xeb
[<c022de88>] sys_sendto+0x113/0x13d
[<c022e883>] sys_socketcall+0x17b/0x261
[<c0102e17>] syscall_call+0x7/0xb
DWARF2 unwinder stuck at syscall_call+0x7/0xb
Leftover inexact backtrace:
Code: a1 c0 b3 3f c0 31 c9 89 da c7 44 24 04 d0 00 00 00 c7 04 24 08 00 00 00 e8 a6 eb fc ff 83 c4 10 5b 5e 5f 5d c3 83 ec 08 89 c2 90 <ff> 48 08 0f 94 c0 84 c0 74 25 83 7a 48 00 74 0f 59 59 8d 42 4c
EIP: [<c027a9f7>] fib_rule_put+0x6/0x38 SS:ESP 0068:d6909d08
<0>Kernel panic - not syncing: Fatal exception in interrupt
This does not exactly match your oops - probably due to different
inlining, but looks sufficiently similar. Was your test kernel
configured with CONFIG_IP_MULTIPLE_TABLES=y (this 2.6.18 has it)?
I have noticed that in two other places in fib_frontend.c the following
code is used to initialize struct fib_result:
#ifdef CONFIG_IP_MULTIPLE_TABLES
res.r = NULL;
#endif
The .r field is initialized after a successful fib_lookup() call, but in
places where ->tb_lookup is called directly this field needs to be
initialized manually. Calling fib_res_put() with uninitialized res.r
pointer causes an oops; this patch fixes the problem.
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index cac06c4..444a56b 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -777,6 +777,10 @@ static void nl_fib_lookup(struct fib_result_nl *frn, struct fib_table *tb )
.tos = frn->fl_tos,
.scope = frn->fl_scope } } };
+#ifdef CONFIG_IP_MULTIPLE_TABLES
+ res.r = NULL;
+#endif
+
frn->err = -ENOENT;
if (tb) {
local_bh_disable();
--
1.5.1.1.197.g66b3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] [IPV4] nl_fib_lookup: Initialise res.r before fib_res_put(&res)
2007-04-26 15:44 ` [PATCH] [IPV4] nl_fib_lookup: Initialise res.r before fib_res_put(&res) Sergey Vlasov
@ 2007-04-26 16:11 ` Alexey Kuznetsov
0 siblings, 0 replies; 19+ messages in thread
From: Alexey Kuznetsov @ 2007-04-26 16:11 UTC (permalink / raw)
To: Sergey Vlasov; +Cc: Greg KH, Linus Torvalds, davem, security, netdev, jaco
Hello!
> When CONFIG_IP_MULTIPLE_TABLES is enabled, the code in nl_fib_lookup()
> needs to initialize the res.r field before fib_res_put(&res) - unlike
> fib_lookup(), a direct call to ->tb_lookup does not set this field.
Indeed, I am sorry.
Alexey
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2007-04-26 16:13 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-04-25 18:38 [PATCH] infinite recursion in netlink Alexey Kuznetsov
2007-04-25 19:59 ` Greg KH
2007-04-25 20:05 ` David Miller
2007-04-25 22:21 ` Jaco Kroon
2007-04-25 20:09 ` David Miller
2007-04-25 20:15 ` [Security] " Linus Torvalds
2007-04-25 20:18 ` David Miller
2007-04-26 5:29 ` Greg KH
2007-04-26 5:32 ` David Miller
2007-04-26 5:44 ` Greg KH
2007-04-26 5:48 ` Greg KH
2007-04-26 5:52 ` Chris Wright
2007-04-26 6:26 ` Chris Wright
2007-04-26 6:31 ` David Miller
2007-04-26 6:51 ` Greg KH
2007-04-26 7:02 ` David Miller
2007-04-26 5:37 ` Chris Wright
2007-04-26 15:44 ` [PATCH] [IPV4] nl_fib_lookup: Initialise res.r before fib_res_put(&res) Sergey Vlasov
2007-04-26 16:11 ` Alexey Kuznetsov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).