diff mbox

[net-next] change nfqueue failopen to apply also to receive message buffer in addition to queue size

Message ID 20160321213532.GA1818@salvia
State Awaiting Upstream, archived
Delegated to: David Miller
Headers show

Commit Message

Pablo Neira Ayuso March 21, 2016, 9:35 p.m. UTC 
On Mon, Mar 21, 2016 at 11:23:43AM +0000, Yigal Reiss (yreiss) wrote:
> @@ -582,10 +585,17 @@ __nfqnl_enqueue_packet(struct net *net, struct nfqnl_instance *queue,
>  	*packet_id_ptr = htonl(entry->id);
>  
>  	/* nfnetlink_unicast will either free the nskb or add it to a socket */
> -	err = nfnetlink_unicast(nskb, net, queue->peer_portid, MSG_DONTWAIT);
> +	err = nfnetlink_unicast_nofree(nskb, net, queue->peer_portid, MSG_DONTWAIT);

This keeps nskb around, this skbuff contains the netlink message, not
the network packet itself that is located in entry->skb.

>  	if (err < 0) {
> -		queue->queue_user_dropped++;
> -		goto err_out_unlock;
> +		if (queue->flags & NFQA_CFG_F_FAIL_OPEN) {
> +		        queue->nobuf_failopened++;
> +		        failopen = 1;
> +			err = 0;

In case we couldn't deliver due to socket buffer overrun, if the
NFQA_CFG_F_FAIL_OPEN flag is set, you set failopen to 1.

> +		}
> +		else {
> +		    queue->queue_user_dropped++;
> +		}
> +		goto err_out_free_nskb;

And finally, jump to err_out_free_nskb.

>  	}
>  
>  	__enqueue_entry(queue, entry);
> @@ -595,7 +605,6 @@ __nfqnl_enqueue_packet(struct net *net, struct nfqnl_instance *queue,
>  
>  err_out_free_nskb:
>  	kfree_skb(nskb);

Which just releases the netlink skbuff.

> -err_out_unlock:
>  	spin_unlock_bh(&queue->lock);
>  	if (failopen)
>  		nf_reinject(entry, NF_ACCEPT);

And reinjects the packet.

So isn't the more simple patch that I'm attaching achieving what you need?

Let me know, thanks.

Comments

Yigal Reiss (yreiss) March 23, 2016, 11:40 a.m. UTC | #1 
On Monday, March 21, 2016 11:36 PM, Pablo Neira Ayuso wrote:
> So isn't the more simple patch that I'm attaching achieving what you need?
Yes. I applied the patch and it works as expected. Indeed much more simple.

I intend to use this patch and would like it to eventually get into the formal kernel. Do you intend to pursue this into mainline kernel?
Pablo Neira Ayuso March 23, 2016, 11:58 a.m. UTC | #2 
On Wed, Mar 23, 2016 at 11:40:14AM +0000, Yigal Reiss (yreiss) wrote:
> On Monday, March 21, 2016 11:36 PM, Pablo Neira Ayuso wrote:
> > So isn't the more simple patch that I'm attaching achieving what you need?
>
> Yes. I applied the patch and it works as expected. Indeed much more
> simple.
> 
> I intend to use this patch and would like it to eventually get into
> the formal kernel. Do you intend to pursue this into mainline
> kernel?

Just made a formal submission of the patch.
diff mbox

Patch

diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index 7542999..cb5b630 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -582,7 +582,12 @@  __nfqnl_enqueue_packet(struct net *net, struct nfqnl_instance *queue,
 	/* nfnetlink_unicast will either free the nskb or add it to a socket */
 	err = nfnetlink_unicast(nskb, net, queue->peer_portid, MSG_DONTWAIT);
 	if (err < 0) {
-		queue->queue_user_dropped++;
+		if (queue->flags & NFQA_CFG_F_FAIL_OPEN) {
+			failopen = 1;
+			err = 0;
+		} else {
+			queue->queue_user_dropped++;
+		}
 		goto err_out_unlock;
 	}