Re: [PATCH] Improve behaviour of Netlink Sockets

[Pablo Neira <pablo@eurodev.net>]

Hi Davem,

David S. Miller wrote:

> Pablo, there is a bug in your new code.  kmalloc() can
> fail, so you have to check the return value and handle
> that appropriately.
>  

thanks, I fixed, I shouldn't type at 5 a.m. If any other problem, please 
let me know.

regards,
Pablo

diff -u -r1.1.1.1 af_netlink.c
--- a/net/netlink/af_netlink.c	19 Aug 2004 14:52:01 -0000	1.1.1.1
+++ b/net/netlink/af_netlink.c	27 Aug 2004 10:49:20 -0000
@@ -46,6 +46,7 @@
 #include <linux/security.h>
 #include <net/sock.h>
 #include <net/scm.h>
+#include <linux/workqueue.h>
 
 #define Nprintk(a...)
 
@@ -69,6 +70,14 @@
 
 #define nlk_sk(__sk) ((struct netlink_opt *)(__sk)->sk_protinfo)
 
+struct netlink_work
+{
+	struct sock 		*sk;
+	int 			len;
+	struct work_struct 	work;
+};
+
+static struct workqueue_struct *netlink_wq;
 static struct hlist_head nl_table[MAX_LINKS];
 static DECLARE_WAIT_QUEUE_HEAD(nl_table_wait);
 static unsigned nl_nonroot[MAX_LINKS];
@@ -87,6 +96,16 @@
 
 static struct notifier_block *netlink_chain;
 
+/* netlink workqueue handler */
+static void netlink_wq_handler(void *data)
+{
+	struct netlink_work *work = data;
+	
+	work->sk->sk_data_ready(work->sk, work->len);
+	sock_put(work->sk);
+	kfree(work);
+}
+
 static void netlink_sock_destruct(struct sock *sk)
 {
 	skb_queue_purge(&sk->sk_receive_queue);
@@ -474,6 +493,8 @@
 	if (atomic_read(&sk->sk_rmem_alloc) > sk->sk_rcvbuf ||
 	    test_bit(0, &nlk->state)) {
 		DECLARE_WAITQUEUE(wait, current);
+		task_t *client;
+
 		if (!timeo) {
 			if (!nlk->pid)
 				netlink_overrun(sk);
@@ -482,6 +503,19 @@
 			return -EAGAIN;
 		}
 
+		if (nlk->pid) {
+			/* Kernel is sending information to user space
+			 * and socket buffer is full: Wake up user
+			 * process */
+			client = find_task_by_pid(nlk->pid);
+			if (!client) {
+				sock_put(sk);
+				kfree_skb(skb);
+				return -EAGAIN;
+			}
+			wake_up_process(client);
+		}
+
 		__set_current_state(TASK_INTERRUPTIBLE);
 		add_wait_queue(&nlk->wait, &wait);
 
@@ -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;
+		}
+		
+		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;
+		
+			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);