From f90a0a74b864fdc46737614f03b8868f4f31e3bf Mon Sep 17 00:00:00 2001 From: Thomas Graf Date: Tue, 3 May 2005 14:29:00 -0700 Subject: [RTNETLINK] Fix & cleanup rtm_min/rtm_max Converts rtm_min and rtm_max arrays to use c99 designated initializers for easier insertion of new message families. RTM_GETMULTICAST and RTM_GETANYCAST did not have the minimal message size specified which means that the netlink message was parsed for routing attributes starting from the header. Adds the proper minimal message sizes for these messages (netlink header + common rtnetlink header) to fix this issue. Signed-off-by: Thomas Graf Signed-off-by: David S. Miller --- net/core/rtnetlink.c | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) (limited to 'net/core/rtnetlink.c') diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index d8c198e42f9..58a98180626 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -88,28 +88,31 @@ struct rtnetlink_link * rtnetlink_links[NPROTO]; static const int rtm_min[(RTM_MAX+1-RTM_BASE)/4] = { - NLMSG_LENGTH(sizeof(struct ifinfomsg)), - NLMSG_LENGTH(sizeof(struct ifaddrmsg)), - NLMSG_LENGTH(sizeof(struct rtmsg)), - NLMSG_LENGTH(sizeof(struct ndmsg)), - NLMSG_LENGTH(sizeof(struct rtmsg)), - NLMSG_LENGTH(sizeof(struct tcmsg)), - NLMSG_LENGTH(sizeof(struct tcmsg)), - NLMSG_LENGTH(sizeof(struct tcmsg)), - NLMSG_LENGTH(sizeof(struct tcamsg)) + [RTM_FAM(RTM_NEWLINK)] = NLMSG_LENGTH(sizeof(struct ifinfomsg)), + [RTM_FAM(RTM_NEWADDR)] = NLMSG_LENGTH(sizeof(struct ifaddrmsg)), + [RTM_FAM(RTM_NEWROUTE)] = NLMSG_LENGTH(sizeof(struct rtmsg)), + [RTM_FAM(RTM_NEWNEIGH)] = NLMSG_LENGTH(sizeof(struct ndmsg)), + [RTM_FAM(RTM_NEWRULE)] = NLMSG_LENGTH(sizeof(struct rtmsg)), + [RTM_FAM(RTM_NEWQDISC)] = NLMSG_LENGTH(sizeof(struct tcmsg)), + [RTM_FAM(RTM_NEWTCLASS)] = NLMSG_LENGTH(sizeof(struct tcmsg)), + [RTM_FAM(RTM_NEWTFILTER)] = NLMSG_LENGTH(sizeof(struct tcmsg)), + [RTM_FAM(RTM_NEWACTION)] = NLMSG_LENGTH(sizeof(struct tcamsg)), + [RTM_FAM(RTM_NEWPREFIX)] = NLMSG_LENGTH(sizeof(struct rtgenmsg)), + [RTM_FAM(RTM_GETMULTICAST)] = NLMSG_LENGTH(sizeof(struct rtgenmsg)), + [RTM_FAM(RTM_GETANYCAST)] = NLMSG_LENGTH(sizeof(struct rtgenmsg)), }; static const int rta_max[(RTM_MAX+1-RTM_BASE)/4] = { - IFLA_MAX, - IFA_MAX, - RTA_MAX, - NDA_MAX, - RTA_MAX, - TCA_MAX, - TCA_MAX, - TCA_MAX, - TCAA_MAX + [RTM_FAM(RTM_NEWLINK)] = IFLA_MAX, + [RTM_FAM(RTM_NEWADDR)] = IFA_MAX, + [RTM_FAM(RTM_NEWROUTE)] = RTA_MAX, + [RTM_FAM(RTM_NEWNEIGH)] = NDA_MAX, + [RTM_FAM(RTM_NEWRULE)] = RTA_MAX, + [RTM_FAM(RTM_NEWQDISC)] = TCA_MAX, + [RTM_FAM(RTM_NEWTCLASS)] = TCA_MAX, + [RTM_FAM(RTM_NEWTFILTER)] = TCA_MAX, + [RTM_FAM(RTM_NEWACTION)] = TCAA_MAX, }; void __rta_fill(struct sk_buff *skb, int attrtype, int attrlen, const void *data) -- cgit v1.2.2 From db46edc6d3b66bf708a8f23a9aa89f63a49ebe33 Mon Sep 17 00:00:00 2001 From: Thomas Graf Date: Tue, 3 May 2005 14:29:39 -0700 Subject: [RTNETLINK] Cleanup rtnetlink_link tables Converts remaining rtnetlink_link tables to use c99 designated initializers to make greping a little bit easier. Signed-off-by: Thomas Graf Signed-off-by: David S. Miller --- net/core/rtnetlink.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'net/core/rtnetlink.c') diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 58a98180626..5fb70cfa108 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -86,7 +86,7 @@ struct sock *rtnl; struct rtnetlink_link * rtnetlink_links[NPROTO]; -static const int rtm_min[(RTM_MAX+1-RTM_BASE)/4] = +static const int rtm_min[RTM_NR_FAMILIES] = { [RTM_FAM(RTM_NEWLINK)] = NLMSG_LENGTH(sizeof(struct ifinfomsg)), [RTM_FAM(RTM_NEWADDR)] = NLMSG_LENGTH(sizeof(struct ifaddrmsg)), @@ -102,7 +102,7 @@ static const int rtm_min[(RTM_MAX+1-RTM_BASE)/4] = [RTM_FAM(RTM_GETANYCAST)] = NLMSG_LENGTH(sizeof(struct rtgenmsg)), }; -static const int rta_max[(RTM_MAX+1-RTM_BASE)/4] = +static const int rta_max[RTM_NR_FAMILIES] = { [RTM_FAM(RTM_NEWLINK)] = IFLA_MAX, [RTM_FAM(RTM_NEWADDR)] = IFA_MAX, @@ -641,7 +641,7 @@ static void rtnetlink_rcv(struct sock *sk, int len) } while (rtnl && rtnl->sk_receive_queue.qlen); } -static struct rtnetlink_link link_rtnetlink_table[RTM_MAX-RTM_BASE+1] = +static struct rtnetlink_link link_rtnetlink_table[RTM_NR_MSGTYPES] = { [RTM_GETLINK - RTM_BASE] = { .dumpit = rtnetlink_dump_ifinfo }, [RTM_SETLINK - RTM_BASE] = { .doit = do_setlink }, -- cgit v1.2.2 From 2a0a6ebee1d68552152ae8d4aeda91d806995dec Mon Sep 17 00:00:00 2001 From: Herbert Xu Date: Tue, 3 May 2005 14:55:09 -0700 Subject: [NETLINK]: Synchronous message processing. Let's recap the problem. The current asynchronous netlink kernel message processing is vulnerable to these attacks: 1) Hit and run: Attacker sends one or more messages and then exits before they're processed. This may confuse/disable the next netlink user that gets the netlink address of the attacker since it may receive the responses to the attacker's messages. Proposed solutions: a) Synchronous processing. b) Stream mode socket. c) Restrict/prohibit binding. 2) Starvation: Because various netlink rcv functions were written to not return until all messages have been processed on a socket, it is possible for these functions to execute for an arbitrarily long period of time. If this is successfully exploited it could also be used to hold rtnl forever. Proposed solutions: a) Synchronous processing. b) Stream mode socket. Firstly let's cross off solution c). It only solves the first problem and it has user-visible impacts. In particular, it'll break user space applications that expect to bind or communicate with specific netlink addresses (pid's). So we're left with a choice of synchronous processing versus SOCK_STREAM for netlink. For the moment I'm sticking with the synchronous approach as suggested by Alexey since it's simpler and I'd rather spend my time working on other things. However, it does have a number of deficiencies compared to the stream mode solution: 1) User-space to user-space netlink communication is still vulnerable. 2) Inefficient use of resources. This is especially true for rtnetlink since the lock is shared with other users such as networking drivers. The latter could hold the rtnl while communicating with hardware which causes the rtnetlink user to wait when it could be doing other things. 3) It is still possible to DoS all netlink users by flooding the kernel netlink receive queue. The attacker simply fills the receive socket with a single netlink message that fills up the entire queue. The attacker then continues to call sendmsg with the same message in a loop. Point 3) can be countered by retransmissions in user-space code, however it is pretty messy. In light of these problems (in particular, point 3), we should implement stream mode netlink at some point. In the mean time, here is a patch that implements synchronous processing. Signed-off-by: Herbert Xu Signed-off-by: David S. Miller --- net/core/rtnetlink.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) (limited to 'net/core/rtnetlink.c') diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 5fb70cfa108..6e1ab1e34b2 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -609,26 +609,31 @@ static inline int rtnetlink_rcv_skb(struct sk_buff *skb) /* * rtnetlink input queue processing routine: - * - try to acquire shared lock. If it is failed, defer processing. + * - process as much as there was in the queue upon entry. * - feed skbs to rtnetlink_rcv_skb, until it refuse a message, - * that will occur, when a dump started and/or acquisition of - * exclusive lock failed. + * that will occur, when a dump started. */ static void rtnetlink_rcv(struct sock *sk, int len) { + unsigned int qlen = skb_queue_len(&sk->sk_receive_queue); + do { struct sk_buff *skb; - if (rtnl_shlock_nowait()) - return; + rtnl_lock(); + + if (qlen > skb_queue_len(&sk->sk_receive_queue)) + qlen = skb_queue_len(&sk->sk_receive_queue); - while ((skb = skb_dequeue(&sk->sk_receive_queue)) != NULL) { + while (qlen--) { + skb = skb_dequeue(&sk->sk_receive_queue); if (rtnetlink_rcv_skb(skb)) { - if (skb->len) + if (skb->len) { skb_queue_head(&sk->sk_receive_queue, skb); - else + qlen++; + } else kfree_skb(skb); break; } @@ -638,7 +643,7 @@ static void rtnetlink_rcv(struct sock *sk, int len) up(&rtnl_sem); netdev_run_todo(); - } while (rtnl && rtnl->sk_receive_queue.qlen); + } while (qlen); } static struct rtnetlink_link link_rtnetlink_table[RTM_NR_MSGTYPES] = -- cgit v1.2.2 From 09e14305982efc2f3b509d3c50ef5dcbff64a998 Mon Sep 17 00:00:00 2001 From: "David S. Miller" Date: Tue, 3 May 2005 15:30:05 -0700 Subject: [NETLINK]: Fix infinite loops in synchronous netlink changes. The qlen should continue to decrement, even if we pop partially processed SKBs back onto the receive queue. Signed-off-by: David S. Miller --- net/core/rtnetlink.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'net/core/rtnetlink.c') diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 6e1ab1e34b2..75b6d33b529 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -626,14 +626,13 @@ static void rtnetlink_rcv(struct sock *sk, int len) if (qlen > skb_queue_len(&sk->sk_receive_queue)) qlen = skb_queue_len(&sk->sk_receive_queue); - while (qlen--) { + for (; qlen; qlen--) { skb = skb_dequeue(&sk->sk_receive_queue); if (rtnetlink_rcv_skb(skb)) { - if (skb->len) { + if (skb->len) skb_queue_head(&sk->sk_receive_queue, skb); - qlen++; - } else + else kfree_skb(skb); break; } -- cgit v1.2.2 From 0f4821e7b93fe72e89b8ff393bd8e705bd178aa5 Mon Sep 17 00:00:00 2001 From: "David S. Miller" Date: Tue, 3 May 2005 16:15:59 -0700 Subject: [XFRM/RTNETLINK]: Decrement qlen properly in {xfrm_,rt}netlink_rcv(). If we free up a partially processed packet because it's skb->len dropped to zero, we need to decrement qlen because we are dropping out of the top-level loop so it will do the decrement for us. Spotted by Herbert Xu. Signed-off-by: David S. Miller --- net/core/rtnetlink.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'net/core/rtnetlink.c') diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 75b6d33b529..00caf4b318b 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -632,8 +632,10 @@ static void rtnetlink_rcv(struct sock *sk, int len) if (skb->len) skb_queue_head(&sk->sk_receive_queue, skb); - else + else { kfree_skb(skb); + qlen--; + } break; } kfree_skb(skb); -- cgit v1.2.2