From 67a4cce4a89718d252b61aaf58882c69c0e2f6e3 Mon Sep 17 00:00:00 2001 From: "John W. Linville" Date: Fri, 12 Oct 2007 16:40:37 -0400 Subject: [PATCH] mac80211: make ieee802_11_parse_elems return void Some APs send management frames with junk padding after the last IE. We already account for a similar problem with some Apple Airport devices, but at least one device is known to send more than a single extra byte. The device in question is the Draytek Vigor2900: http://www.draytek.com.au/products/Vigor2900.php The junk in question looks like an IE that runs off the end of the frame. This cause us to return ParseFailed. Since the frame in question is an association response, this causes us to fail to associate with this AP. The return code from ieee802_11_parse_elems is superfluous. All callers still check for the presence of the specific IEs that interest them anyway. So, remove the return code so the parse never "fails". Acked-by: Michael Wu Signed-off-by: John W. Linville --- net/mac80211/ieee80211_sta.c | 55 ++++++++------------------------------------ 1 file changed, 9 insertions(+), 46 deletions(-) diff --git a/net/mac80211/ieee80211_sta.c b/net/mac80211/ieee80211_sta.c index db81aef6177a..f7ffeec3913f 100644 --- a/net/mac80211/ieee80211_sta.c +++ b/net/mac80211/ieee80211_sta.c @@ -108,14 +108,11 @@ struct ieee802_11_elems { u8 wmm_param_len; }; -enum ParseRes { ParseOK = 0, ParseUnknown = 1, ParseFailed = -1 }; - -static enum ParseRes ieee802_11_parse_elems(u8 *start, size_t len, - struct ieee802_11_elems *elems) +static void ieee802_11_parse_elems(u8 *start, size_t len, + struct ieee802_11_elems *elems) { size_t left = len; u8 *pos = start; - int unknown = 0; memset(elems, 0, sizeof(*elems)); @@ -126,15 +123,8 @@ static enum ParseRes ieee802_11_parse_elems(u8 *start, size_t len, elen = *pos++; left -= 2; - if (elen > left) { -#if 0 - if (net_ratelimit()) - printk(KERN_DEBUG "IEEE 802.11 element parse " - "failed (id=%d elen=%d left=%d)\n", - id, elen, left); -#endif - return ParseFailed; - } + if (elen > left) + return; switch (id) { case WLAN_EID_SSID: @@ -201,28 +191,15 @@ static enum ParseRes ieee802_11_parse_elems(u8 *start, size_t len, elems->ext_supp_rates_len = elen; break; default: -#if 0 - printk(KERN_DEBUG "IEEE 802.11 element parse ignored " - "unknown element (id=%d elen=%d)\n", - id, elen); -#endif - unknown++; break; } left -= elen; pos += elen; } - - /* Do not trigger error if left == 1 as Apple Airport base stations - * send AssocResps that are one spurious byte too long. */ - - return unknown ? ParseUnknown : ParseOK; } - - static int ecw2cw(int ecw) { int cw = 1; @@ -931,12 +908,7 @@ static void ieee80211_auth_challenge(struct net_device *dev, printk(KERN_DEBUG "%s: replying to auth challenge\n", dev->name); pos = mgmt->u.auth.variable; - if (ieee802_11_parse_elems(pos, len - (pos - (u8 *) mgmt), &elems) - == ParseFailed) { - printk(KERN_DEBUG "%s: failed to parse Auth(challenge)\n", - dev->name); - return; - } + ieee802_11_parse_elems(pos, len - (pos - (u8 *) mgmt), &elems); if (!elems.challenge) { printk(KERN_DEBUG "%s: no challenge IE in shared key auth " "frame\n", dev->name); @@ -1230,12 +1202,7 @@ static void ieee80211_rx_mgmt_assoc_resp(struct net_device *dev, aid &= ~(BIT(15) | BIT(14)); pos = mgmt->u.assoc_resp.variable; - if (ieee802_11_parse_elems(pos, len - (pos - (u8 *) mgmt), &elems) - == ParseFailed) { - printk(KERN_DEBUG "%s: failed to parse AssocResp\n", - dev->name); - return; - } + ieee802_11_parse_elems(pos, len - (pos - (u8 *) mgmt), &elems); if (!elems.supp_rates) { printk(KERN_DEBUG "%s: no SuppRates element in AssocResp\n", @@ -1459,7 +1426,7 @@ static void ieee80211_rx_bss_info(struct net_device *dev, struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr); struct ieee802_11_elems elems; size_t baselen; - int channel, invalid = 0, clen; + int channel, clen; struct ieee80211_sta_bss *bss; struct sta_info *sta; struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev); @@ -1505,9 +1472,7 @@ static void ieee80211_rx_bss_info(struct net_device *dev, #endif /* CONFIG_MAC80211_IBSS_DEBUG */ } - if (ieee802_11_parse_elems(mgmt->u.beacon.variable, len - baselen, - &elems) == ParseFailed) - invalid = 1; + ieee802_11_parse_elems(mgmt->u.beacon.variable, len - baselen, &elems); if (sdata->type == IEEE80211_IF_TYPE_IBSS && elems.supp_rates && memcmp(mgmt->bssid, sdata->u.sta.bssid, ETH_ALEN) == 0 && @@ -1724,9 +1689,7 @@ static void ieee80211_rx_mgmt_beacon(struct net_device *dev, if (baselen > len) return; - if (ieee802_11_parse_elems(mgmt->u.beacon.variable, len - baselen, - &elems) == ParseFailed) - return; + ieee802_11_parse_elems(mgmt->u.beacon.variable, len - baselen, &elems); if (elems.erp_info && elems.erp_info_len >= 1) ieee80211_handle_erp_ie(dev, elems.erp_info[0]); -- cgit v1.2.2 From 9b013e05e0289c190a53d78ca029e2f21c0e4485 Mon Sep 17 00:00:00 2001 From: Olof Johansson Date: Thu, 18 Oct 2007 21:48:39 -0700 Subject: [NET]: Fix bug in sk_filter race cures. Looks like this might be causing problems, at least for me on ppc. This happened during a normal boot, right around first interface config/dhcp run.. cpu 0x0: Vector: 300 (Data Access) at [c00000000147b820] pc: c000000000435e5c: .sk_filter_delayed_uncharge+0x1c/0x60 lr: c0000000004360d0: .sk_attach_filter+0x170/0x180 sp: c00000000147baa0 msr: 9000000000009032 dar: 4 dsisr: 40000000 current = 0xc000000004780fa0 paca = 0xc000000000650480 pid = 1295, comm = dhclient3 0:mon> t [c00000000147bb20] c0000000004360d0 .sk_attach_filter+0x170/0x180 [c00000000147bbd0] c000000000418988 .sock_setsockopt+0x788/0x7f0 [c00000000147bcb0] c000000000438a74 .compat_sys_setsockopt+0x4e4/0x5a0 [c00000000147bd90] c00000000043955c .compat_sys_socketcall+0x25c/0x2b0 [c00000000147be30] c000000000007508 syscall_exit+0x0/0x40 --- Exception: c01 (System Call) at 000000000ff618d8 SP (fffdf040) is in userspace 0:mon> I.e. null pointer deref at sk_filter_delayed_uncharge+0x1c: 0:mon> di $.sk_filter_delayed_uncharge c000000000435e40 7c0802a6 mflr r0 c000000000435e44 fbc1fff0 std r30,-16(r1) c000000000435e48 7c8b2378 mr r11,r4 c000000000435e4c ebc2cdd0 ld r30,-12848(r2) c000000000435e50 f8010010 std r0,16(r1) c000000000435e54 f821ff81 stdu r1,-128(r1) c000000000435e58 380300a4 addi r0,r3,164 c000000000435e5c 81240004 lwz r9,4(r4) That's the deref of fp: static void sk_filter_delayed_uncharge(struct sock *sk, struct sk_filter *fp) { unsigned int size = sk_filter_len(fp); ... That is called from sk_attach_filter(): ... rcu_read_lock_bh(); old_fp = rcu_dereference(sk->sk_filter); rcu_assign_pointer(sk->sk_filter, fp); rcu_read_unlock_bh(); sk_filter_delayed_uncharge(sk, old_fp); return 0; ... So, looks like rcu_dereference() returned NULL. I don't know the filter code at all, but it seems like it might be a valid case? sk_detach_filter() seems to handle a NULL sk_filter, at least. So, this needs review by someone who knows the filter, but it fixes the problem for me: Signed-off-by: Olof Johansson Signed-off-by: David S. Miller --- net/core/filter.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/core/filter.c b/net/core/filter.c index 1f0068eae501..e0a06942c025 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -447,7 +447,8 @@ int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk) rcu_assign_pointer(sk->sk_filter, fp); rcu_read_unlock_bh(); - sk_filter_delayed_uncharge(sk, old_fp); + if (old_fp) + sk_filter_delayed_uncharge(sk, old_fp); return 0; } -- cgit v1.2.2 From 85ef3e5cad3dfc6bf84bf2487e9f26a01983c196 Mon Sep 17 00:00:00 2001 From: Randy Dunlap Date: Thu, 18 Oct 2007 21:56:38 -0700 Subject: [NET]: QoS/Sched as menuconfig Convert "QoS and/or fair queueing" to menuconfig. This makes it easy for someone to disable all sub-options with one config symbol. Signed-off-by: Randy Dunlap Signed-off-by: David S. Miller --- net/sched/Kconfig | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/net/sched/Kconfig b/net/sched/Kconfig index 92435a882fac..9c15c4888d12 100644 --- a/net/sched/Kconfig +++ b/net/sched/Kconfig @@ -2,9 +2,7 @@ # Traffic control configuration. # -menu "QoS and/or fair queueing" - -config NET_SCHED +menuconfig NET_SCHED bool "QoS and/or fair queueing" select NET_SCH_FIFO ---help--- @@ -41,9 +39,6 @@ config NET_SCHED The available schedulers are listed in the following questions; you can say Y to as many as you like. If unsure, say N now. -config NET_SCH_FIFO - bool - if NET_SCHED comment "Queueing/Scheduling" @@ -500,4 +495,5 @@ config NET_CLS_IND endif # NET_SCHED -endmenu +config NET_SCH_FIFO + bool -- cgit v1.2.2 From be702d5e38e2e7e554604b223794f87c12fa6811 Mon Sep 17 00:00:00 2001 From: Patrick McHardy Date: Thu, 18 Oct 2007 21:58:19 -0700 Subject: [PACKET]: Kill unused pg_vec_endpage() function The conversion to vm_insert_page() left this unused function behind, remove it. Signed-off-by: Patrick McHardy Signed-off-by: David S. Miller --- net/packet/af_packet.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index e11000a8e950..d0936506b731 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c @@ -1623,11 +1623,6 @@ static struct vm_operations_struct packet_mmap_ops = { .close =packet_mm_close, }; -static inline struct page *pg_vec_endpage(char *one_pg_vec, unsigned int order) -{ - return virt_to_page(one_pg_vec + (PAGE_SIZE << order) - 1); -} - static void free_pg_vec(char **pg_vec, unsigned int order, unsigned int len) { int i; -- cgit v1.2.2 From a25de534f89c515c82d3553c42d3bb02c2d1a7da Mon Sep 17 00:00:00 2001 From: Anton Arapov Date: Thu, 18 Oct 2007 22:00:17 -0700 Subject: [INET]: Justification for local port range robustness. There is a justifying patch for Stephen's patches. Stephen's patches disallows using a port range of one single port and brakes the meaning of the 'remaining' variable, in some places it has different meaning. My patch gives back the sense of 'remaining' variable. It should mean how many ports are remaining and nothing else. Also my patch allows using a single port. I sure we must be able to use mentioned port range, this does not restricted by documentation and does not brake current behavior. usefull links: Patches posted by Stephen Hemminger http://marc.info/?l=linux-netdev&m=119206106218187&w=2 http://marc.info/?l=linux-netdev&m=119206109918235&w=2 Andrew Morton's comment http://marc.info/?l=linux-kernel&m=119248225007737&w=2 1. Allows using a port range of one single port. 2. Gives back sense of 'remaining' variable. Signed-off-by: Anton Arapov Acked-by: Stephen Hemminger Signed-off-by: David S. Miller --- drivers/infiniband/core/cma.c | 5 +++-- net/ipv4/inet_connection_sock.c | 2 +- net/ipv4/inet_hashtables.c | 2 +- net/ipv4/sysctl_net_ipv4.c | 4 ++-- net/ipv4/udp.c | 5 +++-- net/ipv6/inet6_hashtables.c | 2 +- 6 files changed, 11 insertions(+), 9 deletions(-) diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c index 93644f82592c..d08fb30768bc 100644 --- a/drivers/infiniband/core/cma.c +++ b/drivers/infiniband/core/cma.c @@ -2797,11 +2797,12 @@ static void cma_remove_one(struct ib_device *device) static int cma_init(void) { - int ret, low, high; + int ret, low, high, remaining; get_random_bytes(&next_port, sizeof next_port); inet_get_local_port_range(&low, &high); - next_port = ((unsigned int) next_port % (high - low)) + low; + remaining = (high - low) + 1; + next_port = ((unsigned int) next_port % remaining) + low; cma_wq = create_singlethread_workqueue("rdma_cm"); if (!cma_wq) diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c index 3cef12835c4b..8fb6ca23700a 100644 --- a/net/ipv4/inet_connection_sock.c +++ b/net/ipv4/inet_connection_sock.c @@ -93,7 +93,7 @@ int inet_csk_get_port(struct inet_hashinfo *hashinfo, int remaining, rover, low, high; inet_get_local_port_range(&low, &high); - remaining = high - low; + remaining = (high - low) + 1; rover = net_random() % remaining + low; do { diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c index fac6398e4367..16eecc7046a3 100644 --- a/net/ipv4/inet_hashtables.c +++ b/net/ipv4/inet_hashtables.c @@ -286,7 +286,7 @@ int inet_hash_connect(struct inet_timewait_death_row *death_row, struct inet_timewait_sock *tw = NULL; inet_get_local_port_range(&low, &high); - remaining = high - low; + remaining = (high - low) + 1; local_bh_disable(); for (i = 1; i <= remaining; i++) { diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c index c78acc1a7f11..ffddd2b45352 100644 --- a/net/ipv4/sysctl_net_ipv4.c +++ b/net/ipv4/sysctl_net_ipv4.c @@ -122,7 +122,7 @@ static int ipv4_local_port_range(ctl_table *table, int write, struct file *filp, ret = proc_dointvec_minmax(&tmp, write, filp, buffer, lenp, ppos); if (write && ret == 0) { - if (range[1] <= range[0]) + if (range[1] < range[0]) ret = -EINVAL; else set_local_port_range(range); @@ -150,7 +150,7 @@ static int ipv4_sysctl_local_port_range(ctl_table *table, int __user *name, ret = sysctl_intvec(&tmp, name, nlen, oldval, oldlenp, newval, newlen); if (ret == 0 && newval && newlen) { - if (range[1] <= range[0]) + if (range[1] < range[0]) ret = -EINVAL; else set_local_port_range(range); diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index cb9fc58efb2f..35d2b0e9e10b 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c @@ -147,13 +147,14 @@ int __udp_lib_get_port(struct sock *sk, unsigned short snum, write_lock_bh(&udp_hash_lock); if (!snum) { - int i, low, high; + int i, low, high, remaining; unsigned rover, best, best_size_so_far; inet_get_local_port_range(&low, &high); + remaining = (high - low) + 1; best_size_so_far = UINT_MAX; - best = rover = net_random() % (high - low) + low; + best = rover = net_random() % remaining + low; /* 1st pass: look for empty (or shortest) hash chain */ for (i = 0; i < UDP_HTABLE_SIZE; i++) { diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c index 1c2c27655435..d6f1026f1943 100644 --- a/net/ipv6/inet6_hashtables.c +++ b/net/ipv6/inet6_hashtables.c @@ -261,7 +261,7 @@ int inet6_hash_connect(struct inet_timewait_death_row *death_row, struct inet_timewait_sock *tw = NULL; inet_get_local_port_range(&low, &high); - remaining = high - low; + remaining = (high - low) + 1; local_bh_disable(); for (i = 1; i <= remaining; i++) { -- cgit v1.2.2 From ce0e32e65f70337e0732c97499b643205fa8ea31 Mon Sep 17 00:00:00 2001 From: Herbert Xu Date: Thu, 18 Oct 2007 22:37:58 -0700 Subject: [NET]: Fix possible dev_deactivate race condition The function dev_deactivate is supposed to only return when all outstanding transmissions have completed. Unfortunately it is possible for store operations in the driver's transmit function to only become visible after dev_deactivate returns. This patch fixes this by taking the queue lock after we see the end of the queue run. This ensures that all effects of any previous transmit calls are visible. If however we detect that there is another queue run occuring, then we'll warn about it because this should never happen as we have pointed dev->qdisc to noop_qdisc within the same queue lock earlier in the functino. Signed-off-by: Herbert Xu Signed-off-by: David S. Miller --- net/sched/sch_generic.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c index e01d57692c9a..fa1a6f45dc41 100644 --- a/net/sched/sch_generic.c +++ b/net/sched/sch_generic.c @@ -556,6 +556,7 @@ void dev_deactivate(struct net_device *dev) { struct Qdisc *qdisc; struct sk_buff *skb; + int running; spin_lock_bh(&dev->queue_lock); qdisc = dev->qdisc; @@ -571,12 +572,31 @@ void dev_deactivate(struct net_device *dev) dev_watchdog_down(dev); - /* Wait for outstanding dev_queue_xmit calls. */ + /* Wait for outstanding qdisc-less dev_queue_xmit calls. */ synchronize_rcu(); /* Wait for outstanding qdisc_run calls. */ - while (test_bit(__LINK_STATE_QDISC_RUNNING, &dev->state)) - yield(); + do { + while (test_bit(__LINK_STATE_QDISC_RUNNING, &dev->state)) + yield(); + + /* + * Double-check inside queue lock to ensure that all effects + * of the queue run are visible when we return. + */ + spin_lock_bh(&dev->queue_lock); + running = test_bit(__LINK_STATE_QDISC_RUNNING, &dev->state); + spin_unlock_bh(&dev->queue_lock); + + /* + * The running flag should never be set at this point because + * we've already set dev->qdisc to noop_qdisc *inside* the same + * pair of spin locks. That is, if any qdisc_run starts after + * our initial test it should see the noop_qdisc and then + * clear the RUNNING bit before dropping the queue lock. So + * if it is set here then we've found a bug. + */ + } while (WARN_ON_ONCE(running)); } void dev_init_scheduler(struct net_device *dev) -- cgit v1.2.2