Hi, - Thanks for your comments; I was not aware that this issues occur in other places too; attached here is another patch, fixing where applicable in ip6_queue and nfnetlink_queue. 1) in net/ipv6/netfilter/ip6_queue.c - No need to perform data_len = 0 in the switch command, since data_len is initialized to 0 in the beginning of the ipq_build_packet_message() method - We can reach nlmsg_failure only from one place; skb is sure to be NULL when getting there; since skb is NULL, there is no need to check this fact and call kfree_skb(). 2) in net/netfilter/nfnetlink_queue.c: - No need to perform data_len = 0 in the switch command, since data_len is initialized to 0 in the beginning of the nfqnl_build_packet_message() method (Note: here, as opposed to previous patch, nlmsg_failure must check skb and free it if it is not NULL, so the call to kfree_skb() is needed , so it is not removed) Regards, Rami Rosen Signed-off-by: Rami Rosen <ramirose@xxxxxxxxx> On Mon, Jun 2, 2008 at 12:46 PM, Patrick McHardy <kaber@xxxxxxxxx> wrote: > David Miller wrote: >> >> Forwarding to netfilter-devel where this belongs... > > Thanks. > >> In this patch, these three fixes were made in >> net/ipv4/netfilter/ip_queue.c: >> >> 1) No need to perform data_len = 0 in the switch command, since >> data_len >> is initialized to 0 in the beginning of the method >> ,ipq_build_packet_message(). >> >> 2) We can reach nlmsg_failure only from one place; skb is sure to >> be NULL >> when getting there; since skb is NULL, there is no need to check >> this fact >> and call kfree_skb(). >> >> 3) Add #ifdef CONFIG_PROC_FS when removing the VFS entry, >> proc_net_remove(&init_net, IPQ_PROC_FS_NAME); >> >> >> Regards, >> Rami Rosen >> >> >> Signed-off-by: Rami Rosen <ramirose@xxxxxxxxx> > > 1) also affects ip6_queue and nfnetlink_queue > 2) also affects ip6_queue > 3) is unnecessary since proc_net_remove is a NOP without > CONFIG_PROC_FS > > Please update your patch to also change ip6_queue and > nfnetlink_queue where applicable. Thanks. > > >
diff --git a/net/ipv6/netfilter/ip6_queue.c b/net/ipv6/netfilter/ip6_queue.c index 2eff3ae..1b8815f 100644 --- a/net/ipv6/netfilter/ip6_queue.c +++ b/net/ipv6/netfilter/ip6_queue.c @@ -159,7 +159,6 @@ ipq_build_packet_message(struct nf_queue_entry *entry, int *errp) case IPQ_COPY_META: case IPQ_COPY_NONE: size = NLMSG_SPACE(sizeof(*pmsg)); - data_len = 0; break; case IPQ_COPY_PACKET: @@ -226,8 +225,6 @@ ipq_build_packet_message(struct nf_queue_entry *entry, int *errp) return skb; nlmsg_failure: - if (skb) - kfree_skb(skb); *errp = -EINVAL; printk(KERN_ERR "ip6_queue: error creating packet message\n"); return NULL; diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c index 3447025..04e9c96 100644 --- a/net/netfilter/nfnetlink_queue.c +++ b/net/netfilter/nfnetlink_queue.c @@ -243,7 +243,6 @@ nfqnl_build_packet_message(struct nfqnl_instance *queue, switch ((enum nfqnl_config_mode)queue->copy_mode) { case NFQNL_COPY_META: case NFQNL_COPY_NONE: - data_len = 0; break; case NFQNL_COPY_PACKET: