From 4aa3b2ee1945ed082430ae1fb988d60eef64ca07 Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Wed, 18 Feb 2009 15:28:46 +0100 Subject: [PATCH 1/6] netfilter: nf_conntrack_ipv6: fix nf_log_packet message in icmpv6 conntrack This patch fixes a trivial typo that was adding a new line at end of the nf_log_packet() prefix. It also make the logging conditionnal by adding a LOG_INVALID test. Signed-off-by: Eric Leblond Signed-off-by: Patrick McHardy --- net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c index c323643ffcf96..72dbb6d1a6b3e 100644 --- a/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c +++ b/net/ipv6/netfilter/nf_conntrack_proto_icmpv6.c @@ -201,8 +201,9 @@ icmpv6_error(struct net *net, struct sk_buff *skb, unsigned int dataoff, if (net->ct.sysctl_checksum && hooknum == NF_INET_PRE_ROUTING && nf_ip6_checksum(skb, hooknum, dataoff, IPPROTO_ICMPV6)) { - nf_log_packet(PF_INET6, 0, skb, NULL, NULL, NULL, - "nf_ct_icmpv6: ICMPv6 checksum failed\n"); + if (LOG_INVALID(net, IPPROTO_ICMPV6)) + nf_log_packet(PF_INET6, 0, skb, NULL, NULL, NULL, + "nf_ct_icmpv6: ICMPv6 checksum failed "); return -NF_ACCEPT; } From 5ca431f9ae8db8c6edb9c64bebe6d6521077afd6 Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Wed, 18 Feb 2009 15:29:23 +0100 Subject: [PATCH 2/6] netfilter: nfnetlink_log: fix per-rule qthreshold override In NFLOG the per-rule qthreshold should overrides per-instance only it is set. With current code, the per-rule qthreshold is 1 if not set and it overrides the per-instance qthreshold. This patch modifies the default xt_NFLOG threshold from 1 to 0. Thus a value of 0 means there is no per-rule setting and the instance parameter has to apply. Signed-off-by: Eric Leblond Signed-off-by: Patrick McHardy --- include/linux/netfilter/xt_NFLOG.h | 2 +- net/netfilter/nfnetlink_log.c | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/include/linux/netfilter/xt_NFLOG.h b/include/linux/netfilter/xt_NFLOG.h index cdcd0ed58f7ac..4b36aeb46a102 100644 --- a/include/linux/netfilter/xt_NFLOG.h +++ b/include/linux/netfilter/xt_NFLOG.h @@ -2,7 +2,7 @@ #define _XT_NFLOG_TARGET #define XT_NFLOG_DEFAULT_GROUP 0x1 -#define XT_NFLOG_DEFAULT_THRESHOLD 1 +#define XT_NFLOG_DEFAULT_THRESHOLD 0 #define XT_NFLOG_MASK 0x0 diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c index fa49dc7fe100f..580b837e44ddc 100644 --- a/net/netfilter/nfnetlink_log.c +++ b/net/netfilter/nfnetlink_log.c @@ -590,8 +590,10 @@ nfulnl_log_packet(u_int8_t pf, qthreshold = inst->qthreshold; /* per-rule qthreshold overrides per-instance */ - if (qthreshold > li->u.ulog.qthreshold) - qthreshold = li->u.ulog.qthreshold; + if (li->u.ulog.qthreshold) + if (qthreshold > li->u.ulog.qthreshold) + qthreshold = li->u.ulog.qthreshold; + switch (inst->copy_mode) { case NFULNL_COPY_META: From 2c6764b743f9d25dd0806a417f06920dcbd0f599 Mon Sep 17 00:00:00 2001 From: Eric Leblond Date: Wed, 18 Feb 2009 15:29:49 +0100 Subject: [PATCH 3/6] netfilter: nfnetlink_log: fix timeout handling NFLOG timeout was computed in timer by doing: flushtimeout*HZ/100 Default value of flushtimeout was HZ (for 1 second delay). This was wrong for non 100HZ computer. This patch modify the default delay by using 100 instead of HZ. Signed-off-by: Eric Leblond Signed-off-by: Patrick McHardy --- net/netfilter/nfnetlink_log.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c index 580b837e44ddc..c712e9fc6bba2 100644 --- a/net/netfilter/nfnetlink_log.c +++ b/net/netfilter/nfnetlink_log.c @@ -39,7 +39,7 @@ #endif #define NFULNL_NLBUFSIZ_DEFAULT NLMSG_GOODSIZE -#define NFULNL_TIMEOUT_DEFAULT HZ /* every second */ +#define NFULNL_TIMEOUT_DEFAULT 100 /* every second */ #define NFULNL_QTHRESH_DEFAULT 100 /* 100 packets */ #define NFULNL_COPY_RANGE_MAX 0xFFFF /* max packet size is limited by 16-bit struct nfattr nfa_len field */ From 5962fc6d5fff09c8e6fb8cadcb18327a0f4277f7 Mon Sep 17 00:00:00 2001 From: Patrick McHardy Date: Wed, 18 Feb 2009 15:30:34 +0100 Subject: [PATCH 4/6] netfilter: nf_conntrack: don't try to deliver events for untracked connections The untracked conntrack actually does usually have events marked for delivery as its not special-cased in that part of the code. Skip the actual delivery since it impacts performance noticeably. Signed-off-by: Patrick McHardy --- include/net/netfilter/nf_conntrack_core.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h index e78afe7f28e35..c25068e385169 100644 --- a/include/net/netfilter/nf_conntrack_core.h +++ b/include/net/netfilter/nf_conntrack_core.h @@ -59,7 +59,7 @@ static inline int nf_conntrack_confirm(struct sk_buff *skb) struct nf_conn *ct = (struct nf_conn *)skb->nfct; int ret = NF_ACCEPT; - if (ct) { + if (ct && ct != &nf_conntrack_untracked) { if (!nf_ct_is_confirmed(ct) && !nf_ct_is_dying(ct)) ret = __nf_conntrack_confirm(skb); nf_ct_deliver_cached_events(ct); From eb132205ca2f7ad44d8c8c482815b6911200b6a0 Mon Sep 17 00:00:00 2001 From: Jan Engelhardt Date: Wed, 18 Feb 2009 16:42:19 +0100 Subject: [PATCH 5/6] netfilter: make proc/net/ip* print names from foreign NFPROTO When extensions were moved to the NFPROTO_UNSPEC wildcard in ab4f21e6fb1c09b13c4c3cb8357babe8223471bd, they disappeared from the procfs files. Signed-off-by: Jan Engelhardt Signed-off-by: Patrick McHardy --- net/netfilter/x_tables.c | 199 ++++++++++++++++++++++++++++----------- 1 file changed, 142 insertions(+), 57 deletions(-) diff --git a/net/netfilter/x_tables.c b/net/netfilter/x_tables.c index bfbf521f6ea5a..5baccfa5a0deb 100644 --- a/net/netfilter/x_tables.c +++ b/net/netfilter/x_tables.c @@ -827,59 +827,143 @@ static const struct file_operations xt_table_ops = { .release = seq_release_net, }; -static void *xt_match_seq_start(struct seq_file *seq, loff_t *pos) +/* + * Traverse state for ip{,6}_{tables,matches} for helping crossing + * the multi-AF mutexes. + */ +struct nf_mttg_trav { + struct list_head *head, *curr; + uint8_t class, nfproto; +}; + +enum { + MTTG_TRAV_INIT, + MTTG_TRAV_NFP_UNSPEC, + MTTG_TRAV_NFP_SPEC, + MTTG_TRAV_DONE, +}; + +static void *xt_mttg_seq_next(struct seq_file *seq, void *v, loff_t *ppos, + bool is_target) { - struct proc_dir_entry *pde = (struct proc_dir_entry *)seq->private; - u_int16_t af = (unsigned long)pde->data; + static const uint8_t next_class[] = { + [MTTG_TRAV_NFP_UNSPEC] = MTTG_TRAV_NFP_SPEC, + [MTTG_TRAV_NFP_SPEC] = MTTG_TRAV_DONE, + }; + struct nf_mttg_trav *trav = seq->private; + + switch (trav->class) { + case MTTG_TRAV_INIT: + trav->class = MTTG_TRAV_NFP_UNSPEC; + mutex_lock(&xt[NFPROTO_UNSPEC].mutex); + trav->head = trav->curr = is_target ? + &xt[NFPROTO_UNSPEC].target : &xt[NFPROTO_UNSPEC].match; + break; + case MTTG_TRAV_NFP_UNSPEC: + trav->curr = trav->curr->next; + if (trav->curr != trav->head) + break; + mutex_unlock(&xt[NFPROTO_UNSPEC].mutex); + mutex_lock(&xt[trav->nfproto].mutex); + trav->head = trav->curr = is_target ? + &xt[trav->nfproto].target : &xt[trav->nfproto].match; + trav->class = next_class[trav->class]; + break; + case MTTG_TRAV_NFP_SPEC: + trav->curr = trav->curr->next; + if (trav->curr != trav->head) + break; + /* fallthru, _stop will unlock */ + default: + return NULL; + } - mutex_lock(&xt[af].mutex); - return seq_list_start(&xt[af].match, *pos); + if (ppos != NULL) + ++*ppos; + return trav; } -static void *xt_match_seq_next(struct seq_file *seq, void *v, loff_t *pos) +static void *xt_mttg_seq_start(struct seq_file *seq, loff_t *pos, + bool is_target) { - struct proc_dir_entry *pde = (struct proc_dir_entry *)seq->private; - u_int16_t af = (unsigned long)pde->data; + struct nf_mttg_trav *trav = seq->private; + unsigned int j; - return seq_list_next(v, &xt[af].match, pos); + trav->class = MTTG_TRAV_INIT; + for (j = 0; j < *pos; ++j) + if (xt_mttg_seq_next(seq, NULL, NULL, is_target) == NULL) + return NULL; + return trav; } -static void xt_match_seq_stop(struct seq_file *seq, void *v) +static void xt_mttg_seq_stop(struct seq_file *seq, void *v) { - struct proc_dir_entry *pde = seq->private; - u_int16_t af = (unsigned long)pde->data; + struct nf_mttg_trav *trav = seq->private; + + switch (trav->class) { + case MTTG_TRAV_NFP_UNSPEC: + mutex_unlock(&xt[NFPROTO_UNSPEC].mutex); + break; + case MTTG_TRAV_NFP_SPEC: + mutex_unlock(&xt[trav->nfproto].mutex); + break; + } +} - mutex_unlock(&xt[af].mutex); +static void *xt_match_seq_start(struct seq_file *seq, loff_t *pos) +{ + return xt_mttg_seq_start(seq, pos, false); } -static int xt_match_seq_show(struct seq_file *seq, void *v) +static void *xt_match_seq_next(struct seq_file *seq, void *v, loff_t *ppos) { - struct xt_match *match = list_entry(v, struct xt_match, list); + return xt_mttg_seq_next(seq, v, ppos, false); +} - if (strlen(match->name)) - return seq_printf(seq, "%s\n", match->name); - else - return 0; +static int xt_match_seq_show(struct seq_file *seq, void *v) +{ + const struct nf_mttg_trav *trav = seq->private; + const struct xt_match *match; + + switch (trav->class) { + case MTTG_TRAV_NFP_UNSPEC: + case MTTG_TRAV_NFP_SPEC: + if (trav->curr == trav->head) + return 0; + match = list_entry(trav->curr, struct xt_match, list); + return (*match->name == '\0') ? 0 : + seq_printf(seq, "%s\n", match->name); + } + return 0; } static const struct seq_operations xt_match_seq_ops = { .start = xt_match_seq_start, .next = xt_match_seq_next, - .stop = xt_match_seq_stop, + .stop = xt_mttg_seq_stop, .show = xt_match_seq_show, }; static int xt_match_open(struct inode *inode, struct file *file) { + struct seq_file *seq; + struct nf_mttg_trav *trav; int ret; - ret = seq_open(file, &xt_match_seq_ops); - if (!ret) { - struct seq_file *seq = file->private_data; + trav = kmalloc(sizeof(*trav), GFP_KERNEL); + if (trav == NULL) + return -ENOMEM; - seq->private = PDE(inode); + ret = seq_open(file, &xt_match_seq_ops); + if (ret < 0) { + kfree(trav); + return ret; } - return ret; + + seq = file->private_data; + seq->private = trav; + trav->nfproto = (unsigned long)PDE(inode)->data; + return 0; } static const struct file_operations xt_match_ops = { @@ -887,62 +971,63 @@ static const struct file_operations xt_match_ops = { .open = xt_match_open, .read = seq_read, .llseek = seq_lseek, - .release = seq_release, + .release = seq_release_private, }; static void *xt_target_seq_start(struct seq_file *seq, loff_t *pos) { - struct proc_dir_entry *pde = (struct proc_dir_entry *)seq->private; - u_int16_t af = (unsigned long)pde->data; - - mutex_lock(&xt[af].mutex); - return seq_list_start(&xt[af].target, *pos); + return xt_mttg_seq_start(seq, pos, true); } -static void *xt_target_seq_next(struct seq_file *seq, void *v, loff_t *pos) +static void *xt_target_seq_next(struct seq_file *seq, void *v, loff_t *ppos) { - struct proc_dir_entry *pde = (struct proc_dir_entry *)seq->private; - u_int16_t af = (unsigned long)pde->data; - - return seq_list_next(v, &xt[af].target, pos); -} - -static void xt_target_seq_stop(struct seq_file *seq, void *v) -{ - struct proc_dir_entry *pde = seq->private; - u_int16_t af = (unsigned long)pde->data; - - mutex_unlock(&xt[af].mutex); + return xt_mttg_seq_next(seq, v, ppos, true); } static int xt_target_seq_show(struct seq_file *seq, void *v) { - struct xt_target *target = list_entry(v, struct xt_target, list); - - if (strlen(target->name)) - return seq_printf(seq, "%s\n", target->name); - else - return 0; + const struct nf_mttg_trav *trav = seq->private; + const struct xt_target *target; + + switch (trav->class) { + case MTTG_TRAV_NFP_UNSPEC: + case MTTG_TRAV_NFP_SPEC: + if (trav->curr == trav->head) + return 0; + target = list_entry(trav->curr, struct xt_target, list); + return (*target->name == '\0') ? 0 : + seq_printf(seq, "%s\n", target->name); + } + return 0; } static const struct seq_operations xt_target_seq_ops = { .start = xt_target_seq_start, .next = xt_target_seq_next, - .stop = xt_target_seq_stop, + .stop = xt_mttg_seq_stop, .show = xt_target_seq_show, }; static int xt_target_open(struct inode *inode, struct file *file) { + struct seq_file *seq; + struct nf_mttg_trav *trav; int ret; - ret = seq_open(file, &xt_target_seq_ops); - if (!ret) { - struct seq_file *seq = file->private_data; + trav = kmalloc(sizeof(*trav), GFP_KERNEL); + if (trav == NULL) + return -ENOMEM; - seq->private = PDE(inode); + ret = seq_open(file, &xt_target_seq_ops); + if (ret < 0) { + kfree(trav); + return ret; } - return ret; + + seq = file->private_data; + seq->private = trav; + trav->nfproto = (unsigned long)PDE(inode)->data; + return 0; } static const struct file_operations xt_target_ops = { @@ -950,7 +1035,7 @@ static const struct file_operations xt_target_ops = { .open = xt_target_open, .read = seq_read, .llseek = seq_lseek, - .release = seq_release, + .release = seq_release_private, }; #define FORMAT_TABLES "_tables_names" From 325fb5b4d26038cba665dd0d8ee09555321061f0 Mon Sep 17 00:00:00 2001 From: Josef Drexler Date: Tue, 24 Feb 2009 14:53:12 +0100 Subject: [PATCH 6/6] netfilter: xt_recent: fix proc-file addition/removal of IPv4 addresses Fix regression introduded by commit 079aa88 (netfilter: xt_recent: IPv6 support): From http://bugzilla.kernel.org/show_bug.cgi?id=12753: Problem Description: An uninitialized buffer causes IPv4 addresses added manually (via the +IP command to the proc interface) to never match any packets. Similarly, the -IP command fails to remove IPv4 addresses. Details: In the function recent_entry_lookup, the xt_recent module does comparisons of the entire nf_inet_addr union value, both for IPv4 and IPv6 addresses. For addresses initialized from actual packets the remaining 12 bytes not occupied by the IPv4 are zeroed so this works correctly. However when setting the nf_inet_addr addr variable in the recent_mt_proc_write function, only the IPv4 bytes are initialized and the remaining 12 bytes contain garbage. Hence addresses added in this way never match any packets, unless these uninitialized 12 bytes happened to be zero by coincidence. Similarly, addresses cannot consistently be removed using the proc interface due to mismatch of the garbage bytes (although it will sometimes work to remove an address that was added manually). Reading the /proc/net/xt_recent/ entries hides this problem because this only uses the first 4 bytes when displaying IPv4 addresses. Steps to reproduce: $ iptables -I INPUT -m recent --rcheck -j LOG $ echo +169.254.156.239 > /proc/net/xt_recent/DEFAULT $ cat /proc/net/xt_recent/DEFAULT src=169.254.156.239 ttl: 0 last_seen: 119910 oldest_pkt: 1 119910 [At this point no packets from 169.254.156.239 are being logged.] $ iptables -I INPUT -s 169.254.156.239 -m recent --set $ cat /proc/net/xt_recent/DEFAULT src=169.254.156.239 ttl: 0 last_seen: 119910 oldest_pkt: 1 119910 src=169.254.156.239 ttl: 255 last_seen: 126184 oldest_pkt: 4 125434, 125684, 125934, 126184 [At this point, adding the address via an iptables rule, packets are being logged correctly.] $ echo -169.254.156.239 > /proc/net/xt_recent/DEFAULT $ cat /proc/net/xt_recent/DEFAULT src=169.254.156.239 ttl: 0 last_seen: 119910 oldest_pkt: 1 119910 src=169.254.156.239 ttl: 255 last_seen: 126992 oldest_pkt: 10 125434, 125684, 125934, 126184, 126434, 126684, 126934, 126991, 126991, 126992 $ echo -169.254.156.239 > /proc/net/xt_recent/DEFAULT $ cat /proc/net/xt_recent/DEFAULT src=169.254.156.239 ttl: 0 last_seen: 119910 oldest_pkt: 1 119910 src=169.254.156.239 ttl: 255 last_seen: 126992 oldest_pkt: 10 125434, 125684, 125934, 126184, 126434, 126684, 126934, 126991, 126991, 126992 [Removing the address via /proc interface failed evidently.] Possible solutions: - initialize the addr variable in recent_mt_proc_write - compare only 4 bytes for IPv4 addresses in recent_entry_lookup Signed-off-by: Patrick McHardy --- net/netfilter/xt_recent.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/netfilter/xt_recent.c b/net/netfilter/xt_recent.c index fe80b614a4003..791e030ea9031 100644 --- a/net/netfilter/xt_recent.c +++ b/net/netfilter/xt_recent.c @@ -542,7 +542,7 @@ recent_mt_proc_write(struct file *file, const char __user *input, struct recent_entry *e; char buf[sizeof("+b335:1d35:1e55:dead:c0de:1715:5afe:c0de")]; const char *c = buf; - union nf_inet_addr addr; + union nf_inet_addr addr = {}; u_int16_t family; bool add, succ;