Re: [PATCH] Improve behaviour of Netlink Sockets

[Patrick McHardy <kaber@trash.net>]

Pablo Neira wrote:

> If any other problem, please let me know.
>
> ------------------------------------------------------------------------
>
> @@ -521,8 +555,24 @@
> #endif
>
> 	skb_queue_tail(&sk->sk_receive_queue, skb);
> -	sk->sk_data_ready(sk, len);
> -	sock_put(sk);
> +	if (!nlk->pid) {
> +		struct netlink_work *nlwork = 
> +			kmalloc(sizeof(struct netlink_work), GFP_KERNEL);
> +
> +		if (!nlwork) {
> +			sock_put(sk);
> +			return -EAGAIN;
>  

What happens if we queue the packet, fail here and no further
packets are sent ? The socket will never get waken up and the
packet will be stuck forever, right ?

> +		}
> +		
> +		INIT_WORK(&nlwork->work, netlink_wq_handler, nlwork);
> +		nlwork->sk = sk;
> +		nlwork->len = len;
> +		queue_work(netlink_wq, &nlwork->work);
> +	} else {
> +		sk->sk_data_ready(sk, len);
> +		sock_put(sk);
> +	}
> +
> 	return len;
> }
>
> @@ -569,7 +619,21 @@
>                 skb_orphan(skb);
> 		skb_set_owner_r(skb, sk);
> 		skb_queue_tail(&sk->sk_receive_queue, skb);
> -		sk->sk_data_ready(sk, skb->len);
> +
> +		if (!nlk->pid) {
> +			struct netlink_work *nlwork = 
> +				kmalloc(sizeof(struct netlink_work), GFP_KERNEL);
> +
> +			if (!nlwork)
> +				return -1;
>  
Same here.

> +		
> +			INIT_WORK(&nlwork->work, netlink_wq_handler, nlwork);
> +			nlwork->sk = sk;
> +			nlwork->len = skb->len;
> +			queue_work(netlink_wq, &nlwork->work);
> +		} else 
> +			sk->sk_data_ready(sk, skb->len);
> +
> 		return 0;
> 	}
> 	return -1;
> @@ -615,13 +679,14 @@
> 			netlink_overrun(sk);
> 			/* Clone failed. Notify ALL listeners. */
> 			failure = 1;
> +			sock_put(sk);
> 		} else if (netlink_broadcast_deliver(sk, skb2)) {
> 			netlink_overrun(sk);
> +			sock_put(sk);
> 		} else {
> 			delivered = 1;
> 			skb2 = NULL;
> 		}
> -		sock_put(sk);
> 	}
>
> 	netlink_unlock_table();
> @@ -1198,6 +1263,9 @@
> #endif
> 	/* The netlink device handler may be needed early. */ 
> 	rtnetlink_init();
> +	
> +	/* Create a work queue to handle callbacks to modules */
> +	netlink_wq = create_workqueue("netlink");
> 	return 0;
> }
>
> @@ -1205,6 +1273,7 @@
> {
>        sock_unregister(PF_NETLINK);
>        proc_net_remove("netlink");
> +       destroy_workqueue(netlink_wq);
> }
>
> core_initcall(netlink_proto_init);
>  

-
: send the line "unsubscribe linux-net" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html