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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160321213532.GA1818@salvia>
Date:	Mon, 21 Mar 2016 22:35:32 +0100
From:	Pablo Neira Ayuso <pablo@...filter.org>
To:	"Yigal Reiss (yreiss)" <yreiss@...co.com>
Cc:	"'netdev@...r.kernel.org'" <netdev@...r.kernel.org>,
	"netfilter-devel@...r.kernel.org" <netfilter-devel@...r.kernel.org>,
	"Florian Westphal (fw@...len.de)" <fw@...len.de>
Subject: Re: [PATCH net-next] change nfqueue failopen to apply also to
 receive message buffer in addition to queue size

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.

View attachment "x.patch" of type "text/x-diff" (629 bytes)