From 93b2d4a208eeb19bff95e98c34503ce4b03cd0f9 Mon Sep 17 00:00:00 2001 From: "David S. Miller" Date: Sun, 17 Feb 2008 18:35:07 -0800 Subject: Revert "[RTNETLINK]: Send a single notification on device state changes." This reverts commit 45b503548210fe6f23e92b856421c2a3f05fd034. It break locking around dev->link_mode as well as cause other bootup problems. Signed-off-by: David S. Miller --- net/core/rtnetlink.c | 36 ++++++++++-------------------------- 1 file changed, 10 insertions(+), 26 deletions(-) (limited to 'net/core') diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index ecb02afd52dc..61ac8d06292c 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -504,7 +504,7 @@ int rtnl_put_cacheinfo(struct sk_buff *skb, struct dst_entry *dst, u32 id, EXPORT_SYMBOL_GPL(rtnl_put_cacheinfo); -static int set_operstate(struct net_device *dev, unsigned char transition, bool send_notification) +static void set_operstate(struct net_device *dev, unsigned char transition) { unsigned char operstate = dev->operstate; @@ -527,12 +527,8 @@ static int set_operstate(struct net_device *dev, unsigned char transition, bool write_lock_bh(&dev_base_lock); dev->operstate = operstate; write_unlock_bh(&dev_base_lock); - - if (send_notification) - netdev_state_change(dev); - return 1; - } else - return 0; + netdev_state_change(dev); + } } static void copy_rtnl_link_stats(struct rtnl_link_stats *a, @@ -826,7 +822,6 @@ static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm, if (tb[IFLA_BROADCAST]) { nla_memcpy(dev->broadcast, tb[IFLA_BROADCAST], dev->addr_len); send_addr_notify = 1; - modified = 1; } if (ifm->ifi_flags || ifm->ifi_change) { @@ -839,23 +834,16 @@ static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm, dev_change_flags(dev, flags); } - if (tb[IFLA_TXQLEN]) { - if (dev->tx_queue_len != nla_get_u32(tb[IFLA_TXQLEN])) { - dev->tx_queue_len = nla_get_u32(tb[IFLA_TXQLEN]); - modified = 1; - } - } + if (tb[IFLA_TXQLEN]) + dev->tx_queue_len = nla_get_u32(tb[IFLA_TXQLEN]); if (tb[IFLA_OPERSTATE]) - modified |= set_operstate(dev, nla_get_u8(tb[IFLA_OPERSTATE]), false); + set_operstate(dev, nla_get_u8(tb[IFLA_OPERSTATE])); if (tb[IFLA_LINKMODE]) { - if (dev->link_mode != nla_get_u8(tb[IFLA_LINKMODE])) { - write_lock_bh(&dev_base_lock); - dev->link_mode = nla_get_u8(tb[IFLA_LINKMODE]); - write_lock_bh(&dev_base_lock); - modified = 1; - } + write_lock_bh(&dev_base_lock); + dev->link_mode = nla_get_u8(tb[IFLA_LINKMODE]); + write_unlock_bh(&dev_base_lock); } err = 0; @@ -869,10 +857,6 @@ errout: if (send_addr_notify) call_netdevice_notifiers(NETDEV_CHANGEADDR, dev); - - if (modified) - netdev_state_change(dev); - return err; } @@ -990,7 +974,7 @@ struct net_device *rtnl_create_link(struct net *net, char *ifname, if (tb[IFLA_TXQLEN]) dev->tx_queue_len = nla_get_u32(tb[IFLA_TXQLEN]); if (tb[IFLA_OPERSTATE]) - set_operstate(dev, nla_get_u8(tb[IFLA_OPERSTATE]), true); + set_operstate(dev, nla_get_u8(tb[IFLA_OPERSTATE])); if (tb[IFLA_LINKMODE]) dev->link_mode = nla_get_u8(tb[IFLA_LINKMODE]); -- cgit From 9ff566074689e3aed1488780b97714ec43ba361d Mon Sep 17 00:00:00 2001 From: "David S. Miller" Date: Sun, 17 Feb 2008 18:39:54 -0800 Subject: Revert "[NDISC]: Fix race in generic address resolution" This reverts commit 69cc64d8d92bf852f933e90c888dfff083bd4fc9. It causes recursive locking in IPV6 because unlike other neighbour layer clients, it even needs neighbour cache entries to send neighbour soliciation messages :-( We'll have to find another way to fix this race. Signed-off-by: David S. Miller --- net/core/neighbour.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'net/core') diff --git a/net/core/neighbour.c b/net/core/neighbour.c index 7bb6a9a1256d..a16cf1ec5e5e 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -834,12 +834,18 @@ static void neigh_timer_handler(unsigned long arg) } if (neigh->nud_state & (NUD_INCOMPLETE | NUD_PROBE)) { struct sk_buff *skb = skb_peek(&neigh->arp_queue); - + /* keep skb alive even if arp_queue overflows */ + if (skb) + skb_get(skb); + write_unlock(&neigh->lock); neigh->ops->solicit(neigh, skb); atomic_inc(&neigh->probes); - } + if (skb) + kfree_skb(skb); + } else { out: - write_unlock(&neigh->lock); + write_unlock(&neigh->lock); + } if (notify) neigh_update_notify(neigh); -- cgit From 12aa343add3eced38a44bdb612b35fdf634d918c Mon Sep 17 00:00:00 2001 From: "Jorge Boncompte [DTI2]" Date: Tue, 19 Feb 2008 14:17:04 -0800 Subject: [NET]: Messed multicast lists after dev_mc_sync/unsync Commit a0a400d79e3dd7843e7e81baa3ef2957bdc292d0 ("[NET]: dev_mcast: add multicast list synchronization helpers") from you introduced a new field "da_synced" to struct dev_addr_list that is not properly initialized to 0. So when any of the current users (8021q, macvlan, mac80211) calls dev_mc_sync/unsync they mess the address list for both devices. The attached patch fixed it for me and avoid future problems. Signed-off-by: Jorge Boncompte [DTI2] Signed-off-by: Patrick McHardy Signed-off-by: David S. Miller --- net/core/dev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/core') diff --git a/net/core/dev.c b/net/core/dev.c index 6cfc1238c4a6..95161054c4d1 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -2900,7 +2900,7 @@ int __dev_addr_add(struct dev_addr_list **list, int *count, } } - da = kmalloc(sizeof(*da), GFP_ATOMIC); + da = kzalloc(sizeof(*da), GFP_ATOMIC); if (da == NULL) return -ENOMEM; memcpy(da->da_addr, addr, alen); -- cgit From 76e87306c2de2a581ec939cf9c97cd18d053f90e Mon Sep 17 00:00:00 2001 From: Thomas Graf Date: Tue, 19 Feb 2008 16:12:08 -0800 Subject: [RTNL]: Add missing link netlink attribute policy definitions IFLA_LINK is no longer a write-only attribute on the kernel side and must thus be validated. Same goes for the newly introduced IFLA_LINKINFO. Fixes undefined behaviour if either of the attributes are not well formed. Signed-off-by: Thomas Graf Signed-off-by: David S. Miller --- net/core/rtnetlink.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'net/core') diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 61ac8d06292c..e170179cc66f 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -689,10 +689,12 @@ const struct nla_policy ifla_policy[IFLA_MAX+1] = { [IFLA_BROADCAST] = { .type = NLA_BINARY, .len = MAX_ADDR_LEN }, [IFLA_MAP] = { .len = sizeof(struct rtnl_link_ifmap) }, [IFLA_MTU] = { .type = NLA_U32 }, + [IFLA_LINK] = { .type = NLA_U32 }, [IFLA_TXQLEN] = { .type = NLA_U32 }, [IFLA_WEIGHT] = { .type = NLA_U32 }, [IFLA_OPERSTATE] = { .type = NLA_U8 }, [IFLA_LINKMODE] = { .type = NLA_U8 }, + [IFLA_LINKINFO] = { .type = NLA_NESTED }, [IFLA_NET_NS_PID] = { .type = NLA_U32 }, }; -- cgit From da12f7356da1dfb97f1c6c418f828b7ce442fef9 Mon Sep 17 00:00:00 2001 From: "Denis V. Lunev" Date: Wed, 20 Feb 2008 00:26:16 -0800 Subject: [NETNS]: Namespace leak in pneigh_lookup. release_net is missed on the error path in pneigh_lookup. Signed-off-by: Denis V. Lunev Signed-off-by: David S. Miller --- net/core/neighbour.c | 1 + 1 file changed, 1 insertion(+) (limited to 'net/core') diff --git a/net/core/neighbour.c b/net/core/neighbour.c index a16cf1ec5e5e..4062b88bfc1b 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -507,6 +507,7 @@ struct pneigh_entry * pneigh_lookup(struct neigh_table *tbl, if (tbl->pconstructor && tbl->pconstructor(n)) { if (dev) dev_put(dev); + release_net(net); kfree(n); n = NULL; goto out; -- cgit From 1840bb13c22f5b8fd2e242e36c8d6ea3f312be67 Mon Sep 17 00:00:00 2001 From: Thomas Graf Date: Sat, 23 Feb 2008 19:54:36 -0800 Subject: [RTNL]: Validate hardware and broadcast address attribute for RTM_NEWLINK RTM_NEWLINK allows for already existing links to be modified. For this purpose do_setlink() is called which expects address attributes with a payload length of at least dev->addr_len. This patch adds the necessary validation for the RTM_NEWLINK case. The address length for links to be created is not checked for now as the actual attribute length is used when copying the address to the netdevice structure. It might make sense to report an error if less than addr_len bytes are provided but enforcing this might break drivers trying to be smart with not transmitting all zero addresses. Signed-off-by: Thomas Graf Signed-off-by: David S. Miller --- net/core/rtnetlink.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) (limited to 'net/core') diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index e170179cc66f..2bd9c5f7627d 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -722,6 +722,21 @@ static struct net *get_net_ns_by_pid(pid_t pid) return net; } +static int validate_linkmsg(struct net_device *dev, struct nlattr *tb[]) +{ + if (dev) { + if (tb[IFLA_ADDRESS] && + nla_len(tb[IFLA_ADDRESS]) < dev->addr_len) + return -EINVAL; + + if (tb[IFLA_BROADCAST] && + nla_len(tb[IFLA_BROADCAST]) < dev->addr_len) + return -EINVAL; + } + + return 0; +} + static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm, struct nlattr **tb, char *ifname, int modified) { @@ -894,12 +909,7 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg) goto errout; } - if (tb[IFLA_ADDRESS] && - nla_len(tb[IFLA_ADDRESS]) < dev->addr_len) - goto errout_dev; - - if (tb[IFLA_BROADCAST] && - nla_len(tb[IFLA_BROADCAST]) < dev->addr_len) + if ((err = validate_linkmsg(dev, tb)) < 0) goto errout_dev; err = do_setlink(dev, ifm, tb, ifname, 0); @@ -1020,6 +1030,9 @@ replay: else dev = NULL; + if ((err = validate_linkmsg(dev, tb)) < 0) + return err; + if (tb[IFLA_LINKINFO]) { err = nla_parse_nested(linkinfo, IFLA_INFO_MAX, tb[IFLA_LINKINFO], ifla_info_policy); -- cgit From bc4bf5f38cf0a623e6a29f52ec80bfcc56a373c6 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Sat, 23 Feb 2008 19:57:02 -0800 Subject: [NEIGH]: Fix race between neighbor lookup and table's hash_rnd update. The neigh_hash_grow() may update the tbl->hash_rnd value, which is used in all tbl->hash callbacks to calculate the hashval. Two lookup routines may race with this, since they call the ->hash callback without the tbl->lock held. Since the hash_rnd is changed with this lock write-locked moving the calls to ->hash under this lock read-locked closes this gap. Signed-off-by: Pavel Emelyanov Signed-off-by: David S. Miller --- net/core/neighbour.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'net/core') diff --git a/net/core/neighbour.c b/net/core/neighbour.c index 4062b88bfc1b..2328acbd16cd 100644 --- a/net/core/neighbour.c +++ b/net/core/neighbour.c @@ -358,11 +358,12 @@ struct neighbour *neigh_lookup(struct neigh_table *tbl, const void *pkey, { struct neighbour *n; int key_len = tbl->key_len; - u32 hash_val = tbl->hash(pkey, dev); + u32 hash_val; NEIGH_CACHE_STAT_INC(tbl, lookups); read_lock_bh(&tbl->lock); + hash_val = tbl->hash(pkey, dev); for (n = tbl->hash_buckets[hash_val & tbl->hash_mask]; n; n = n->next) { if (dev == n->dev && !memcmp(n->primary_key, pkey, key_len)) { neigh_hold(n); @@ -379,11 +380,12 @@ struct neighbour *neigh_lookup_nodev(struct neigh_table *tbl, struct net *net, { struct neighbour *n; int key_len = tbl->key_len; - u32 hash_val = tbl->hash(pkey, NULL); + u32 hash_val; NEIGH_CACHE_STAT_INC(tbl, lookups); read_lock_bh(&tbl->lock); + hash_val = tbl->hash(pkey, NULL); for (n = tbl->hash_buckets[hash_val & tbl->hash_mask]; n; n = n->next) { if (!memcmp(n->primary_key, pkey, key_len) && (net == n->dev->nd_net)) { -- cgit