Re: Possible netfilter-related memory corruption in 2.6.37
Am 14.02.2011 17:52, schrieb Patrick McHardy:
Am 14.02.2011 17:48, schrieb Eric Dumazet:
I am not sure, but I guess nf_reinject() needs a fix too ;)
I agree. That one looks uglier though, I guess we'll have to
iterate through all hooks to note the previous one.
How about this? Unfortunately I don't think we can avoid
iterating through all hooks without violating RCU rules.
diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c
index 74aebed..834bb07 100644
--- a/net/netfilter/nf_queue.c
+++ b/net/netfilter/nf_queue.c
@@ -235,6 +235,7 @@ int nf_queue(struct sk_buff *skb,
void nf_reinject(struct nf_queue_entry *entry, unsigned int verdict)
{
struct sk_buff *skb = entry-skb;
+ struct nf_hook_ops *i, *prev;
struct list_head *elem = entry-elem-list;
const struct nf_afinfo *afinfo;
@@ -244,8 +245,21 @@ void nf_reinject(struct nf_queue_entry *entry, unsigned
int verdict)
/* Continue traversal iff userspace said ok... */
if (verdict == NF_REPEAT) {
- elem = elem-prev;
- verdict = NF_ACCEPT;
+ prev = NULL;
+ list_for_each_entry_rcu(i, nf_hooks[entry-pf][entry-hook],
+ list) {
+ if (i-list == elem)
+ break;
+ prev = i;
+ }
+
+ if (prev == NULL ||
+ i-list == nf_hooks[entry-pf][entry-hook])
+ verdict = NF_DROP;
+ else {
+ elem = prev-list;
+ verdict = NF_ACCEPT;
+ }
}
if (verdict == NF_ACCEPT) {
Re: Possible netfilter-related memory corruption in 2.6.37
Le vendredi 18 février 2011 à 19:37 +0100, Patrick McHardy a écrit :
Am 14.02.2011 17:52, schrieb Patrick McHardy:
Am 14.02.2011 17:48, schrieb Eric Dumazet:
I am not sure, but I guess nf_reinject() needs a fix too ;)
I agree. That one looks uglier though, I guess we'll have to
iterate through all hooks to note the previous one.
How about this? Unfortunately I don't think we can avoid
iterating through all hooks without violating RCU rules.
/* Continue traversal iff userspace said ok... */
if (verdict == NF_REPEAT) {
- elem = elem-prev;
- verdict = NF_ACCEPT;
+ prev = NULL;
+ list_for_each_entry_rcu(i,
nf_hooks[entry-pf][entry-hook],
+ list) {
+ if (i-list == elem)
+ break;
+ prev = i;
Hmm... what happens if elem was the first elem in list ?
We exit with prev = NULL -- NF_DROP ?
I must miss something...
+ }
+
+ if (prev == NULL ||
+ i-list == nf_hooks[entry-pf][entry-hook])
+ verdict = NF_DROP;
+ else {
+ elem = prev-list;
+ verdict = NF_ACCEPT;
+ }
}
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Possible netfilter-related memory corruption in 2.6.37
We see severe memory corruption in kvm while used in conjunction with bridge/netfilter. Enabling slab debugging points the finger at a netfilter chain invoked from the bridge code. Can someone take a look? https://bugzilla.kernel.org/show_bug.cgi?id=27052 -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible netfilter-related memory corruption in 2.6.37
Le lundi 14 février 2011 à 16:58 +0200, Avi Kivity a écrit : We see severe memory corruption in kvm while used in conjunction with bridge/netfilter. Enabling slab debugging points the finger at a netfilter chain invoked from the bridge code. Can someone take a look? https://bugzilla.kernel.org/show_bug.cgi?id=27052 CC netdev Does a revert of commit ca44ac386181ba7 help a bit ? (net: don't reallocate skb-head unless the current one hasn't the needed extra size or is shared) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible netfilter-related memory corruption in 2.6.37
On Monday 2011-02-14 16:11, Eric Dumazet wrote: Le lundi 14 février 2011 à 16:58 +0200, Avi Kivity a écrit : We see severe memory corruption in kvm while used in conjunction with bridge/netfilter. Enabling slab debugging points the finger at a netfilter chain invoked from the bridge code. Can someone take a look? https://bugzilla.kernel.org/show_bug.cgi?id=27052 Maybe looks familiar to https://lkml.org/lkml/2011/2/3/147 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible netfilter-related memory corruption in 2.6.37
Le lundi 14 février 2011 à 16:18 +0100, Jan Engelhardt a écrit :
On Monday 2011-02-14 16:11, Eric Dumazet wrote:
Le lundi 14 février 2011 à 16:58 +0200, Avi Kivity a écrit :
We see severe memory corruption in kvm while used in conjunction with
bridge/netfilter. Enabling slab debugging points the finger at a
netfilter chain invoked from the bridge code.
Can someone take a look?
https://bugzilla.kernel.org/show_bug.cgi?id=27052
Maybe looks familiar to https://lkml.org/lkml/2011/2/3/147
Are you sure Jan ?
IMHO it looks like in your case, a NULL -hook() is called, from
nf_iterate()
BTW, list_for_each_continue_rcu() really should be converted to
list_for_each_entry_continue_rcu()
This is a bit ugly :
list_for_each_continue_rcu(*i, head) {
struct nf_hook_ops *elem = (struct nf_hook_ops *)*i;
Also, I wonder if RCU rules are respected in nf_iterate().
For example this line is really suspicious :
*i = (*i)-prev;
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible netfilter-related memory corruption in 2.6.37
Am 14.02.2011 16:50, schrieb Eric Dumazet:
Le lundi 14 février 2011 à 16:18 +0100, Jan Engelhardt a écrit :
On Monday 2011-02-14 16:11, Eric Dumazet wrote:
Le lundi 14 février 2011 à 16:58 +0200, Avi Kivity a écrit :
We see severe memory corruption in kvm while used in conjunction with
bridge/netfilter. Enabling slab debugging points the finger at a
netfilter chain invoked from the bridge code.
Can someone take a look?
https://bugzilla.kernel.org/show_bug.cgi?id=27052
Maybe looks familiar to https://lkml.org/lkml/2011/2/3/147
Are you sure Jan ?
IMHO it looks like in your case, a NULL -hook() is called, from
nf_iterate()
BTW, list_for_each_continue_rcu() really should be converted to
list_for_each_entry_continue_rcu()
This is a bit ugly :
list_for_each_continue_rcu(*i, head) {
struct nf_hook_ops *elem = (struct nf_hook_ops *)*i;
Also, I wonder if RCU rules are respected in nf_iterate().
For example this line is really suspicious :
*i = (*i)-prev;
Yeah, that definitely looks wrong. How about this instead?
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 1e00bf7..899b71c 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -133,6 +133,7 @@ unsigned int nf_iterate(struct list_head *head,
/* Optimization: we don't need to hold module
reference here, since function can't sleep. --RR */
+repeat:
verdict = elem-hook(hook, skb, indev, outdev, okfn);
if (verdict != NF_ACCEPT) {
#ifdef CONFIG_NETFILTER_DEBUG
@@ -145,7 +146,7 @@ unsigned int nf_iterate(struct list_head *head,
#endif
if (verdict != NF_REPEAT)
return verdict;
- *i = (*i)-prev;
+ goto repeat;
}
}
return NF_ACCEPT;
Re: Possible netfilter-related memory corruption in 2.6.37
Le lundi 14 février 2011 à 17:24 +0100, Patrick McHardy a écrit :
Am 14.02.2011 16:50, schrieb Eric Dumazet:
Le lundi 14 février 2011 à 16:18 +0100, Jan Engelhardt a écrit :
On Monday 2011-02-14 16:11, Eric Dumazet wrote:
Le lundi 14 février 2011 à 16:58 +0200, Avi Kivity a écrit :
We see severe memory corruption in kvm while used in conjunction with
bridge/netfilter. Enabling slab debugging points the finger at a
netfilter chain invoked from the bridge code.
Can someone take a look?
https://bugzilla.kernel.org/show_bug.cgi?id=27052
Maybe looks familiar to https://lkml.org/lkml/2011/2/3/147
Are you sure Jan ?
IMHO it looks like in your case, a NULL -hook() is called, from
nf_iterate()
BTW, list_for_each_continue_rcu() really should be converted to
list_for_each_entry_continue_rcu()
This is a bit ugly :
list_for_each_continue_rcu(*i, head) {
struct nf_hook_ops *elem = (struct nf_hook_ops *)*i;
Also, I wonder if RCU rules are respected in nf_iterate().
For example this line is really suspicious :
*i = (*i)-prev;
Yeah, that definitely looks wrong. How about this instead?
This patch seems fine to me, thanks !
Acked-by: Eric Dumazet eric.duma...@gmail.com
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible netfilter-related memory corruption in 2.6.37
Am 14.02.2011 17:29, schrieb Eric Dumazet: Le lundi 14 février 2011 à 17:24 +0100, Patrick McHardy a écrit : Also, I wonder if RCU rules are respected in nf_iterate(). For example this line is really suspicious : *i = (*i)-prev; Yeah, that definitely looks wrong. How about this instead? This patch seems fine to me, thanks ! Acked-by: Eric Dumazet eric.duma...@gmail.com THanks Eric, I've queued the patch for 2.6.38. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible netfilter-related memory corruption in 2.6.37
Le lundi 14 février 2011 à 17:37 +0100, Patrick McHardy a écrit : Am 14.02.2011 17:29, schrieb Eric Dumazet: Le lundi 14 février 2011 à 17:24 +0100, Patrick McHardy a écrit : Also, I wonder if RCU rules are respected in nf_iterate(). For example this line is really suspicious : *i = (*i)-prev; Yeah, that definitely looks wrong. How about this instead? This patch seems fine to me, thanks ! Acked-by: Eric Dumazet eric.duma...@gmail.com THanks Eric, I've queued the patch for 2.6.38. I am not sure, but I guess nf_reinject() needs a fix too ;) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Possible netfilter-related memory corruption in 2.6.37
Am 14.02.2011 17:48, schrieb Eric Dumazet: Le lundi 14 février 2011 à 17:37 +0100, Patrick McHardy a écrit : Am 14.02.2011 17:29, schrieb Eric Dumazet: Le lundi 14 février 2011 à 17:24 +0100, Patrick McHardy a écrit : Also, I wonder if RCU rules are respected in nf_iterate(). For example this line is really suspicious : *i = (*i)-prev; Yeah, that definitely looks wrong. How about this instead? This patch seems fine to me, thanks ! Acked-by: Eric Dumazet eric.duma...@gmail.com THanks Eric, I've queued the patch for 2.6.38. I am not sure, but I guess nf_reinject() needs a fix too ;) I agree. That one looks uglier though, I guess we'll have to iterate through all hooks to note the previous one. I'll take care of that once Dave has pulled the last fix since I already sent out the pull request. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
