* [PATCH 2.4] 2/2: Fix NAT helper locking
@ 2004-09-03 7:04 Harald Welte
2004-09-03 7:27 ` David S. Miller
0 siblings, 1 reply; 2+ messages in thread
From: Harald Welte @ 2004-09-03 7:04 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Netfilter Development Mailinglist
[-- Attachment #1: Type: text/plain, Size: 10276 bytes --]
Hi Dave!
This is the second of a two part patch.
This part fixes the locking in NAT helpers.
Please apply, Thanks.
# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
# 2004/08/08 12:49:20+02:00 kaber@coreworks.de
# [NETFILTER]: Fix deadlock condition in conntrack/nat-helpers
#
# There is a possible deadlock condition with conntrack/nat-helpers:
#
# CPU1:
# conntrack-helper:help: lock(private_lock)
# ip_conntrack_expect_related: write_lock(ip_conntrack_lock)
#
# CPU2:
# nat-core:do_bindings: read_lock(ip_conntrack_lock)
# nat-helper:help: lock(private_lock)
#
# The lock in the nat-helper is unneccessary because the expectation
# is never changed and is protected by ip_conntrack_lock.
#
# Signed-off-by: Patrick McHardy <kaber@trash.net>
# Signed-off-by: Harald Welte <laforge@netfilter.org>
#
# net/ipv4/netfilter/ip_nat_irc.c
# 2004/08/08 12:49:15+02:00 kaber@coreworks.de +1 -17
# [NETFILTER]: Fix deadlock condition in conntrack/nat-helpers
#
# net/ipv4/netfilter/ip_nat_ftp.c
# 2004/08/08 12:49:15+02:00 kaber@coreworks.de +1 -19
# [NETFILTER]: Fix deadlock condition in conntrack/nat-helpers
#
# net/ipv4/netfilter/ip_conntrack_irc.c
# 2004/08/08 12:49:15+02:00 kaber@coreworks.de +0 -8
# [NETFILTER]: Fix deadlock condition in conntrack/nat-helpers
#
# net/ipv4/netfilter/ip_conntrack_ftp.c
# 2004/08/08 12:49:15+02:00 kaber@coreworks.de +3 -6
# [NETFILTER]: Fix deadlock condition in conntrack/nat-helpers
#
# include/linux/netfilter_ipv4/ip_conntrack_irc.h
# 2004/08/08 12:49:15+02:00 kaber@coreworks.de +0 -5
# [NETFILTER]: Fix deadlock condition in conntrack/nat-helpers
#
# include/linux/netfilter_ipv4/ip_conntrack_ftp.h
# 2004/08/08 12:49:15+02:00 kaber@coreworks.de +0 -5
# [NETFILTER]: Fix deadlock condition in conntrack/nat-helpers
#
diff -Nru a/include/linux/netfilter_ipv4/ip_conntrack_ftp.h b/include/linux/netfilter_ipv4/ip_conntrack_ftp.h
--- a/include/linux/netfilter_ipv4/ip_conntrack_ftp.h 2004-08-08 12:49:45 +02:00
+++ b/include/linux/netfilter_ipv4/ip_conntrack_ftp.h 2004-08-08 12:49:45 +02:00
@@ -4,11 +4,6 @@
#ifdef __KERNEL__
-#include <linux/netfilter_ipv4/lockhelp.h>
-
-/* Protects ftp part of conntracks */
-DECLARE_LOCK_EXTERN(ip_ftp_lock);
-
#define FTP_PORT 21
#endif /* __KERNEL__ */
diff -Nru a/include/linux/netfilter_ipv4/ip_conntrack_irc.h b/include/linux/netfilter_ipv4/ip_conntrack_irc.h
--- a/include/linux/netfilter_ipv4/ip_conntrack_irc.h 2004-08-08 12:49:45 +02:00
+++ b/include/linux/netfilter_ipv4/ip_conntrack_irc.h 2004-08-08 12:49:45 +02:00
@@ -33,17 +33,12 @@
#ifdef __KERNEL__
-#include <linux/netfilter_ipv4/lockhelp.h>
-
#define IRC_PORT 6667
struct dccproto {
char* match;
int matchlen;
};
-
-/* Protects irc part of conntracks */
-DECLARE_LOCK_EXTERN(ip_irc_lock);
#endif /* __KERNEL__ */
diff -Nru a/net/ipv4/netfilter/ip_conntrack_ftp.c b/net/ipv4/netfilter/ip_conntrack_ftp.c
--- a/net/ipv4/netfilter/ip_conntrack_ftp.c 2004-08-08 12:49:45 +02:00
+++ b/net/ipv4/netfilter/ip_conntrack_ftp.c 2004-08-08 12:49:45 +02:00
@@ -11,7 +11,7 @@
#include <linux/netfilter_ipv4/ip_conntrack_helper.h>
#include <linux/netfilter_ipv4/ip_conntrack_ftp.h>
-DECLARE_LOCK(ip_ftp_lock);
+static DECLARE_LOCK(ip_ftp_lock);
struct module *ip_conntrack_ftp = THIS_MODULE;
#define MAX_PORTS 8
@@ -338,7 +338,6 @@
memset(&expect, 0, sizeof(expect));
/* Update the ftp info */
- LOCK_BH(&ip_ftp_lock);
if (htonl((array[0] << 24) | (array[1] << 16) | (array[2] << 8) | array[3])
== ct->tuplehash[dir].tuple.src.ip) {
exp->seq = ntohl(tcph->seq) + matchoff;
@@ -358,7 +357,8 @@
<lincoln@cesar.org.br> for reporting this potential
problem (DMZ machines opening holes to internal
networks, or the packet filter itself). */
- if (!loose) goto out;
+ if (!loose)
+ return NF_ACCEPT;
}
exp->tuple = ((struct ip_conntrack_tuple)
@@ -376,9 +376,6 @@
/* Ignore failure; should only happen with NAT */
ip_conntrack_expect_related(ct, &expect);
- out:
- UNLOCK_BH(&ip_ftp_lock);
-
return NF_ACCEPT;
}
diff -Nru a/net/ipv4/netfilter/ip_conntrack_irc.c b/net/ipv4/netfilter/ip_conntrack_irc.c
--- a/net/ipv4/netfilter/ip_conntrack_irc.c 2004-08-08 12:49:45 +02:00
+++ b/net/ipv4/netfilter/ip_conntrack_irc.c 2004-08-08 12:49:45 +02:00
@@ -29,7 +29,6 @@
#include <net/checksum.h>
#include <net/tcp.h>
-#include <linux/netfilter_ipv4/lockhelp.h>
#include <linux/netfilter_ipv4/ip_conntrack_helper.h>
#include <linux/netfilter_ipv4/ip_conntrack_irc.h>
@@ -61,7 +60,6 @@
};
#define MINMATCHLEN 5
-DECLARE_LOCK(ip_irc_lock);
struct module *ip_conntrack_irc = THIS_MODULE;
#if 0
@@ -208,8 +206,6 @@
memset(&expect, 0, sizeof(expect));
- LOCK_BH(&ip_irc_lock);
-
/* save position of address in dcc string,
* neccessary for NAT */
DEBUGP("tcph->seq = %u\n", tcph->seq);
@@ -236,8 +232,6 @@
ntohs(exp->tuple.dst.u.tcp.port));
ip_conntrack_expect_related(ct, &expect);
- UNLOCK_BH(&ip_irc_lock);
-
return NF_ACCEPT;
} /* for .. NUM_DCCPROTO */
} /* while data < ... */
@@ -314,8 +308,6 @@
ip_conntrack_helper_unregister(&irc_helpers[i]);
}
}
-
-EXPORT_SYMBOL(ip_irc_lock);
module_init(init);
module_exit(fini);
diff -Nru a/net/ipv4/netfilter/ip_nat_ftp.c b/net/ipv4/netfilter/ip_nat_ftp.c
--- a/net/ipv4/netfilter/ip_nat_ftp.c 2004-08-08 12:49:45 +02:00
+++ b/net/ipv4/netfilter/ip_nat_ftp.c 2004-08-08 12:49:45 +02:00
@@ -24,8 +24,6 @@
MODULE_PARM(ports, "1-" __MODULE_STRING(MAX_PORTS) "i");
#endif
-DECLARE_LOCK_EXTERN(ip_ftp_lock);
-
/* FIXME: Time out? --RR */
static unsigned int
@@ -48,8 +46,6 @@
DEBUGP("nat_expected: We have a connection!\n");
exp_ftp_info = &ct->master->help.exp_ftp_info;
- LOCK_BH(&ip_ftp_lock);
-
if (exp_ftp_info->ftptype == IP_CT_FTP_PORT
|| exp_ftp_info->ftptype == IP_CT_FTP_EPRT) {
/* PORT command: make connection go to the client. */
@@ -64,7 +60,6 @@
DEBUGP("nat_expected: PASV cmd. %u.%u.%u.%u->%u.%u.%u.%u\n",
NIPQUAD(newsrcip), NIPQUAD(newdstip));
}
- UNLOCK_BH(&ip_ftp_lock);
if (HOOK2MANIP(hooknum) == IP_NAT_MANIP_SRC)
newip = newsrcip;
@@ -100,8 +95,6 @@
{
char buffer[sizeof("nnn,nnn,nnn,nnn,nnn,nnn")];
- MUST_BE_LOCKED(&ip_ftp_lock);
-
sprintf(buffer, "%u,%u,%u,%u,%u,%u",
NIPQUAD(newip), port>>8, port&0xFF);
@@ -123,8 +116,6 @@
{
char buffer[sizeof("|1|255.255.255.255|65535|")];
- MUST_BE_LOCKED(&ip_ftp_lock);
-
sprintf(buffer, "|1|%u.%u.%u.%u|%u|", NIPQUAD(newip), port);
DEBUGP("calling ip_nat_mangle_tcp_packet\n");
@@ -145,8 +136,6 @@
{
char buffer[sizeof("|||65535|")];
- MUST_BE_LOCKED(&ip_ftp_lock);
-
sprintf(buffer, "|||%u|", port);
DEBUGP("calling ip_nat_mangle_tcp_packet\n");
@@ -178,7 +167,6 @@
u_int16_t port;
struct ip_conntrack_tuple newtuple;
- MUST_BE_LOCKED(&ip_ftp_lock);
DEBUGP("FTP_NAT: seq %u + %u in %u\n",
expect->seq, exp_ftp_info->len,
ntohl(tcph->seq));
@@ -257,15 +245,12 @@
}
datalen = (*pskb)->len - iph->ihl * 4 - tcph->doff * 4;
- LOCK_BH(&ip_ftp_lock);
/* If it's in the right range... */
if (between(exp->seq + exp_ftp_info->len,
ntohl(tcph->seq),
ntohl(tcph->seq) + datalen)) {
- if (!ftp_data_fixup(exp_ftp_info, ct, pskb, ctinfo, exp)) {
- UNLOCK_BH(&ip_ftp_lock);
+ if (!ftp_data_fixup(exp_ftp_info, ct, pskb, ctinfo, exp))
return NF_DROP;
- }
} else {
/* Half a match? This means a partial retransmisison.
It's a cracker being funky. */
@@ -275,11 +260,8 @@
ntohl(tcph->seq),
ntohl(tcph->seq) + datalen);
}
- UNLOCK_BH(&ip_ftp_lock);
return NF_DROP;
}
- UNLOCK_BH(&ip_ftp_lock);
-
return NF_ACCEPT;
}
diff -Nru a/net/ipv4/netfilter/ip_nat_irc.c b/net/ipv4/netfilter/ip_nat_irc.c
--- a/net/ipv4/netfilter/ip_nat_irc.c 2004-08-08 12:49:45 +02:00
+++ b/net/ipv4/netfilter/ip_nat_irc.c 2004-08-08 12:49:45 +02:00
@@ -46,9 +46,6 @@
MODULE_PARM_DESC(ports, "port numbers of IRC servers");
#endif
-/* protects irc part of conntracks */
-DECLARE_LOCK_EXTERN(ip_irc_lock);
-
/* FIXME: Time out? --RR */
static unsigned int
@@ -104,8 +101,6 @@
/* "4294967296 65635 " */
char buffer[18];
- MUST_BE_LOCKED(&ip_irc_lock);
-
DEBUGP("IRC_NAT: info (seq %u + %u) in %u\n",
expect->seq, exp_irc_info->len,
ntohl(tcph->seq));
@@ -113,11 +108,6 @@
newip = ct->tuplehash[IP_CT_DIR_REPLY].tuple.dst.ip;
/* Alter conntrack's expectations. */
-
- /* We can read expect here without conntrack lock, since it's
- only set in ip_conntrack_irc, with ip_irc_lock held
- writable */
-
t = expect->tuple;
t.dst.ip = newip;
for (port = exp_irc_info->port; port != 0; port++) {
@@ -187,15 +177,12 @@
DEBUGP("got beyond not touching\n");
datalen = (*pskb)->len - iph->ihl * 4 - tcph->doff * 4;
- LOCK_BH(&ip_irc_lock);
/* Check wether the whole IP/address pattern is carried in the payload */
if (between(exp->seq + exp_irc_info->len,
ntohl(tcph->seq),
ntohl(tcph->seq) + datalen)) {
- if (!irc_data_fixup(exp_irc_info, ct, pskb, ctinfo, exp)) {
- UNLOCK_BH(&ip_irc_lock);
+ if (!irc_data_fixup(exp_irc_info, ct, pskb, ctinfo, exp))
return NF_DROP;
- }
} else {
/* Half a match? This means a partial retransmisison.
It's a cracker being funky. */
@@ -206,11 +193,8 @@
ntohl(tcph->seq),
ntohl(tcph->seq) + datalen);
}
- UNLOCK_BH(&ip_irc_lock);
return NF_DROP;
}
- UNLOCK_BH(&ip_irc_lock);
-
return NF_ACCEPT;
}
--
- Harald Welte <laforge@netfilter.org> http://www.netfilter.org/
============================================================================
"Fragmentation is like classful addressing -- an interesting early
architectural error that shows how much experimentation was going
on while IP was being designed." -- Paul Vixie
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH 2.4] 2/2: Fix NAT helper locking
2004-09-03 7:04 [PATCH 2.4] 2/2: Fix NAT helper locking Harald Welte
@ 2004-09-03 7:27 ` David S. Miller
0 siblings, 0 replies; 2+ messages in thread
From: David S. Miller @ 2004-09-03 7:27 UTC (permalink / raw)
To: Harald Welte; +Cc: netdev, netfilter-devel
On Fri, 3 Sep 2004 09:04:55 +0200
Harald Welte <laforge@netfilter.org> wrote:
> This is the second of a two part patch.
>
> This part fixes the locking in NAT helpers.
These 2.4.x variants of the NAT locking fixes
are applied as well.
Thanks.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2004-09-03 7:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-09-03 7:04 [PATCH 2.4] 2/2: Fix NAT helper locking Harald Welte
2004-09-03 7:27 ` David S. Miller
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).