Re: [PATCH] peer_pid checking in ip_queue

[Patrick McHardy <kaber@trash.net>]

Hi Pablo,

Pablo Neira wrote:
> Hi,
> 
> I patched ip_queue.c to modify the default behaviour if peer_pid is 0. 
> In this case, there's no program in user space to receive the skbuff via 
> netlink, but ip_queue will allocate space for the skbuff and after 
> checking if peer_pid is 0, it will destroy this skbuff allocated. Am I 
> missing anything?

peer_pid is protected by queue_lock, you move it out of the locked
section. This leads to a small race which can cause message loss.
Not sure if it's bad though as netlink is unreliable itself.

Regards
Patrick

> 
> best regards,
> Pablo
> 
> P.S: BTW, Harald, I noticed that I had some problems with my smtp server 
> and some email got lost, I sent you an email with a "digest" of all my 
> recent patches, did you receive it? If not, please let me know.
> 
> 
> ------------------------------------------------------------------------
> 
> --- linux-2.6.3-old/net/ipv4/netfilter/ip_queue.c	2004-02-18 04:59:59.000000000 +0100
> +++ linux-2.6.3/net/ipv4/netfilter/ip_queue.c	2004-03-23 10:31:16.000000000 +0100
> @@ -277,6 +277,9 @@
>  	struct sk_buff *nskb;
>  	struct ipq_queue_entry *entry;
>  
> +	if (!peer_pid)
> +		return -EINVAL;
> +
>  	if (copy_mode == IPQ_COPY_NONE)
>  		return -EAGAIN;
>  
> @@ -303,9 +306,6 @@
>  		
>  	write_lock_bh(&queue_lock);
>  	
> -	if (!peer_pid)
> -		goto err_out_free_nskb; 
> -
>   	/* netlink_unicast will either free the nskb or attach it to a socket */ 
>  	status = netlink_unicast(ipqnl, nskb, peer_pid, MSG_DONTWAIT);
>  	if (status < 0)
> @@ -318,9 +318,6 @@
>  	write_unlock_bh(&queue_lock);
>  	return status;
>  
> -err_out_free_nskb:
> -	kfree_skb(nskb); 
> -	
>  err_out_unlock:
>  	write_unlock_bh(&queue_lock);
>