Browse Source

netfilter: xtables: fix reentrancy

commit f3c5c1bfd4308 (make ip_tables reentrant) introduced a race in
handling the stackptr restore, at the end of ipt_do_table()

We should do it before the call to xt_info_rdunlock_bh(), or we allow
cpu preemption and another cpu overwrites stackptr of original one.

A second fix is to change the underflow test to check the origptr value
instead of 0 to detect underflow, or else we allow a jump from different
hooks.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Jan Engelhardt <jengelh@medozas.de>
Signed-off-by: Patrick McHardy <kaber@trash.net>
Eric Dumazet 14 years ago
parent
commit
db856674ac
2 changed files with 4 additions and 4 deletions
  1. 2 2
      net/ipv4/netfilter/ip_tables.c
  2. 2 2
      net/ipv6/netfilter/ip6_tables.c

+ 2 - 2
net/ipv4/netfilter/ip_tables.c

@@ -387,7 +387,7 @@ ipt_do_table(struct sk_buff *skb,
 					verdict = (unsigned)(-v) - 1;
 					verdict = (unsigned)(-v) - 1;
 					break;
 					break;
 				}
 				}
-				if (*stackptr == 0) {
+				if (*stackptr <= origptr) {
 					e = get_entry(table_base,
 					e = get_entry(table_base,
 					    private->underflow[hook]);
 					    private->underflow[hook]);
 					pr_debug("Underflow (this is normal) "
 					pr_debug("Underflow (this is normal) "
@@ -427,10 +427,10 @@ ipt_do_table(struct sk_buff *skb,
 			/* Verdict */
 			/* Verdict */
 			break;
 			break;
 	} while (!acpar.hotdrop);
 	} while (!acpar.hotdrop);
-	xt_info_rdunlock_bh();
 	pr_debug("Exiting %s; resetting sp from %u to %u\n",
 	pr_debug("Exiting %s; resetting sp from %u to %u\n",
 		 __func__, *stackptr, origptr);
 		 __func__, *stackptr, origptr);
 	*stackptr = origptr;
 	*stackptr = origptr;
+	xt_info_rdunlock_bh();
 #ifdef DEBUG_ALLOW_ALL
 #ifdef DEBUG_ALLOW_ALL
 	return NF_ACCEPT;
 	return NF_ACCEPT;
 #else
 #else

+ 2 - 2
net/ipv6/netfilter/ip6_tables.c

@@ -410,7 +410,7 @@ ip6t_do_table(struct sk_buff *skb,
 					verdict = (unsigned)(-v) - 1;
 					verdict = (unsigned)(-v) - 1;
 					break;
 					break;
 				}
 				}
-				if (*stackptr == 0)
+				if (*stackptr <= origptr)
 					e = get_entry(table_base,
 					e = get_entry(table_base,
 					    private->underflow[hook]);
 					    private->underflow[hook]);
 				else
 				else
@@ -441,8 +441,8 @@ ip6t_do_table(struct sk_buff *skb,
 			break;
 			break;
 	} while (!acpar.hotdrop);
 	} while (!acpar.hotdrop);
 
 
-	xt_info_rdunlock_bh();
 	*stackptr = origptr;
 	*stackptr = origptr;
+	xt_info_rdunlock_bh();
 
 
 #ifdef DEBUG_ALLOW_ALL
 #ifdef DEBUG_ALLOW_ALL
 	return NF_ACCEPT;
 	return NF_ACCEPT;