netdev
[Top] [All Lists]

[PATCH] [NETLINK] unify checkings for clean messages

To: "David S. Miller" <davem@xxxxxxxxxxxxx>
Subject: [PATCH] [NETLINK] unify checkings for clean messages
From: Pablo Neira <pablo@xxxxxxxxxxx>
Date: Fri, 11 Feb 2005 22:18:32 +0100
Cc: netdev@xxxxxxxxxxx
In-reply-to: <20050210173201.21da89e5.davem@xxxxxxxxxxxxx>
References: <420BF8C3.3050305@xxxxxxxxxxx> <20050210173201.21da89e5.davem@xxxxxxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040413 Debian/1.6-5
David S. Miller wrote:

Why don't you bypass all the cleanup diffs and just do the functionality
change instead?  When you mix whitespace and coding style cleanups
with real changes, it puts your real changes at risk if we think your
cleanups are ugly or bogus since you've created a patch dependency.

Ok, sorry for the nuisance. I'll try to avoid such things in future. Thanks davem.

Back to what I wanted to introduce...

The following patches introduce a new function that check that the netlink messages received are clean. Actually some people performs such checkings, some don't and others simply do some half of them. I think that we must unify the behaviour of a netlink socket when it has to reply to malformed messages.

01process_skb.patch:
introduces the new function called netlink_process_skb that does the sanity checkings for received messages.
02xfrm.patch:
     the modification to make xfrm_user use such new function.
03rtnetlink.patch:
     same thing for rtnetlink.

The 02 and 03 patches are straight forward conversions. If you are ok with this, I could post more patches to make other netlink sockets use this new function.

--
Pablo
===== include/linux/netlink.h 1.23 vs edited =====
--- 1.23/include/linux/netlink.h        2005-02-07 06:59:39 +01:00
+++ edited/include/linux/netlink.h      2005-02-11 21:25:38 +01:00
@@ -116,6 +116,7 @@
 #define NETLINK_CREDS(skb)     (&NETLINK_CB((skb)).creds)
 
 
+extern int netlink_process_skb(struct sk_buff *skb, int (*process_msg)(struct 
sk_buff *skb, struct nlmsghdr *nlh, int *err));
 extern struct sock *netlink_kernel_create(int unit, void (*input)(struct sock 
*sk, int len));
 extern void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err);
 extern int netlink_unicast(struct sock *ssk, struct sk_buff *skb, __u32 pid, 
int nonblock);
===== net/netlink/af_netlink.c 1.69 vs edited =====
--- 1.69/net/netlink/af_netlink.c       2005-01-21 21:25:32 +01:00
+++ edited/net/netlink/af_netlink.c     2005-02-11 21:30:34 +01:00
@@ -1201,6 +1201,42 @@
        netlink_unicast(in_skb->sk, skb, NETLINK_CB(in_skb).pid, MSG_DONTWAIT);
 }
 
+/* 
+ * Process one packet of messages.
+ * Malformed skbs with wrong lengths of messages are discarded silently.
+ */
+int netlink_process_skb(struct sk_buff *skb, 
+                       int (*process_msg)(struct sk_buff *skb,
+                                          struct nlmsghdr *nlh,
+                                          int *err))
+{
+       int err;
+       struct nlmsghdr * nlh;
+
+       while (skb->len >= NLMSG_SPACE(0)) {
+               u32 rlen;
+
+               nlh = (struct nlmsghdr *)skb->data;
+               if (nlh->nlmsg_len < sizeof(*nlh) || skb->len < nlh->nlmsg_len)
+                       return 0;
+               rlen = NLMSG_ALIGN(nlh->nlmsg_len);
+               if (rlen > skb->len)
+                       rlen = skb->len;
+               if (process_msg(skb, nlh, &err)) {
+                       /* Not error, but we must interrupt processing here:
+                        *   Note, that in this case we do not pull message
+                        *   from skb, it will be processed later.
+                        */
+                       if (err == 0)
+                               return -1;
+                       netlink_ack(skb, nlh, err);
+               } else if (nlh->nlmsg_flags & NLM_F_ACK)
+                       netlink_ack(skb, nlh, 0);
+               skb_pull(skb, rlen);
+       }
+
+       return 0;
+}
 
 #ifdef CONFIG_PROC_FS
 struct nl_seq_iter {
@@ -1456,6 +1492,7 @@
 
 MODULE_ALIAS_NETPROTO(PF_NETLINK);
 
+EXPORT_SYMBOL(netlink_process_skb);
 EXPORT_SYMBOL(netlink_ack);
 EXPORT_SYMBOL(netlink_broadcast);
 EXPORT_SYMBOL(netlink_dump_start);
===== net/xfrm/xfrm_user.c 1.52 vs edited =====
--- 1.52/net/xfrm/xfrm_user.c   2005-01-26 06:53:19 +01:00
+++ edited/net/xfrm/xfrm_user.c 2005-02-10 00:15:16 +01:00
@@ -980,33 +980,6 @@
        return -1;
 }
 
-static int xfrm_user_rcv_skb(struct sk_buff *skb)
-{
-       int err;
-       struct nlmsghdr *nlh;
-
-       while (skb->len >= NLMSG_SPACE(0)) {
-               u32 rlen;
-
-               nlh = (struct nlmsghdr *) skb->data;
-               if (nlh->nlmsg_len < sizeof(*nlh) ||
-                   skb->len < nlh->nlmsg_len)
-                       return 0;
-               rlen = NLMSG_ALIGN(nlh->nlmsg_len);
-               if (rlen > skb->len)
-                       rlen = skb->len;
-               if (xfrm_user_rcv_msg(skb, nlh, &err) < 0) {
-                       if (err == 0)
-                               return -1;
-                       netlink_ack(skb, nlh, err);
-               } else if (nlh->nlmsg_flags & NLM_F_ACK)
-                       netlink_ack(skb, nlh, 0);
-               skb_pull(skb, rlen);
-       }
-
-       return 0;
-}
-
 static void xfrm_netlink_rcv(struct sock *sk, int len)
 {
        do {
@@ -1015,7 +988,7 @@
                down(&xfrm_cfg_sem);
 
                while ((skb = skb_dequeue(&sk->sk_receive_queue)) != NULL) {
-                       if (xfrm_user_rcv_skb(skb)) {
+                       if (netlink_process_skb(skb, xfrm_user_rcv_msg)) {
                                if (skb->len)
                                        skb_queue_head(&sk->sk_receive_queue,
                                                       skb);
===== net/core/rtnetlink.c 1.33 vs edited =====
--- 1.33/net/core/rtnetlink.c   2005-01-10 22:42:22 +01:00
+++ edited/net/core/rtnetlink.c 2005-02-10 00:14:59 +01:00
@@ -570,41 +570,6 @@
        return -1;
 }
 
-/* 
- * Process one packet of messages.
- * Malformed skbs with wrong lengths of messages are discarded silently.
- */
-
-static inline int rtnetlink_rcv_skb(struct sk_buff *skb)
-{
-       int err;
-       struct nlmsghdr * nlh;
-
-       while (skb->len >= NLMSG_SPACE(0)) {
-               u32 rlen;
-
-               nlh = (struct nlmsghdr *)skb->data;
-               if (nlh->nlmsg_len < sizeof(*nlh) || skb->len < nlh->nlmsg_len)
-                       return 0;
-               rlen = NLMSG_ALIGN(nlh->nlmsg_len);
-               if (rlen > skb->len)
-                       rlen = skb->len;
-               if (rtnetlink_rcv_msg(skb, nlh, &err)) {
-                       /* Not error, but we must interrupt processing here:
-                        *   Note, that in this case we do not pull message
-                        *   from skb, it will be processed later.
-                        */
-                       if (err == 0)
-                               return -1;
-                       netlink_ack(skb, nlh, err);
-               } else if (nlh->nlmsg_flags&NLM_F_ACK)
-                       netlink_ack(skb, nlh, 0);
-               skb_pull(skb, rlen);
-       }
-
-       return 0;
-}
-
 /*
  *  rtnetlink input queue processing routine:
  *     - try to acquire shared lock. If it is failed, defer processing.
@@ -622,7 +587,7 @@
                        return;
 
                while ((skb = skb_dequeue(&sk->sk_receive_queue)) != NULL) {
-                       if (rtnetlink_rcv_skb(skb)) {
+                       if (netlink_process_skb(skb, rtnetlink_rcv_msg)) {
                                if (skb->len)
                                        skb_queue_head(&sk->sk_receive_queue,
                                                       skb);
<Prev in Thread] Current Thread [Next in Thread>