netfilter: nfnetlink_queue: assert monotonic packet ids

Packet identifier is currently setup in nfqnl_build_packet_message(),
using one atomic_inc_return().

Problem is that since several cpus might concurrently call
nfqnl_enqueue_packet() for the same queue, we can deliver packets to
consumer in non monotonic way (packet N+1 being delivered after packet
N)

This patch moves the packet id setup from nfqnl_build_packet_message()
to nfqnl_enqueue_packet() to guarantee correct delivery order.

This also removes one atomic operation.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Florian Westphal <fw@strlen.de>
CC: Pablo Neira Ayuso <pablo@netfilter.org>
CC: Eric Leblond <eric@regit.org>
Signed-off-by: Patrick McHardy <kaber@trash.net>

diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index c645b87915b845aae8918e4b25f9c302266b2c10..3b2af8cb7de98d2ac4dfb5773fb7cbf157495794 100644 (file)
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -58,7 +58,7 @@ struct nfqnl_instance {
  */
        spinlock_t      lock;
        unsigned int    queue_total;
-       atomic_t        id_sequence;            /* 'sequence' of pkt ids */
+       unsigned int    id_sequence;            /* 'sequence' of pkt ids */
        struct list_head queue_list;            /* packets in queue */
 };
 
@@ -213,13 +213,15 @@ nfqnl_flush(struct nfqnl_instance *queue, nfqnl_cmpfn cmpfn, unsigned long data)
 
 static struct sk_buff *
 nfqnl_build_packet_message(struct nfqnl_instance *queue,
-                          struct nf_queue_entry *entry)
+                          struct nf_queue_entry *entry,
+                          __be32 **packet_id_ptr)
 {
        sk_buff_data_t old_tail;
        size_t size;
        size_t data_len = 0;
        struct sk_buff *skb;
-       struct nfqnl_msg_packet_hdr pmsg;
+       struct nlattr *nla;
+       struct nfqnl_msg_packet_hdr *pmsg;
        struct nlmsghdr *nlh;
        struct nfgenmsg *nfmsg;
        struct sk_buff *entskb = entry->skb;
@@ -272,12 +274,11 @@ nfqnl_build_packet_message(struct nfqnl_instance *queue,
        nfmsg->version = NFNETLINK_V0;
        nfmsg->res_id = htons(queue->queue_num);
 
-       entry->id = atomic_inc_return(&queue->id_sequence);
-       pmsg.packet_id          = htonl(entry->id);
-       pmsg.hw_protocol        = entskb->protocol;
-       pmsg.hook               = entry->hook;
-
-       NLA_PUT(skb, NFQA_PACKET_HDR, sizeof(pmsg), &pmsg);
+       nla = __nla_reserve(skb, NFQA_PACKET_HDR, sizeof(*pmsg));
+       pmsg = nla_data(nla);
+       pmsg->hw_protocol       = entskb->protocol;
+       pmsg->hook              = entry->hook;
+       *packet_id_ptr          = &pmsg->packet_id;
 
        indev = entry->indev;
        if (indev) {
@@ -388,6 +389,7 @@ nfqnl_enqueue_packet(struct nf_queue_entry *entry, unsigned int queuenum)
        struct sk_buff *nskb;
        struct nfqnl_instance *queue;
        int err = -ENOBUFS;
+       __be32 *packet_id_ptr;
 
        /* rcu_read_lock()ed by nf_hook_slow() */
        queue = instance_lookup(queuenum);
@@ -401,7 +403,7 @@ nfqnl_enqueue_packet(struct nf_queue_entry *entry, unsigned int queuenum)
                goto err_out;
        }
 
-       nskb = nfqnl_build_packet_message(queue, entry);
+       nskb = nfqnl_build_packet_message(queue, entry, &packet_id_ptr);
        if (nskb == NULL) {
                err = -ENOMEM;
                goto err_out;
@@ -420,6 +422,8 @@ nfqnl_enqueue_packet(struct nf_queue_entry *entry, unsigned int queuenum)
                                 queue->queue_total);
                goto err_out_free_nskb;
        }
+       entry->id = ++queue->id_sequence;
+       *packet_id_ptr = htonl(entry->id);
 
        /* nfnetlink_unicast will either free the nskb or add it to a socket */
        err = nfnetlink_unicast(nskb, &init_net, queue->peer_pid, MSG_DONTWAIT);
@@ -852,7 +856,7 @@ static int seq_show(struct seq_file *s, void *v)
                          inst->peer_pid, inst->queue_total,
                          inst->copy_mode, inst->copy_range,
                          inst->queue_dropped, inst->queue_user_dropped,
-                         atomic_read(&inst->id_sequence), 1);
+                         inst->id_sequence, 1);
 }
 
 static const struct seq_operations nfqnl_seq_ops = {