1. 13 2月, 2008 10 次提交
    • D
      [NDISC]: Fix race in generic address resolution · 69cc64d8
      David S. Miller 提交于
      Frank Blaschka provided the bug report and the initial suggested fix
      for this bug.  He also validated this version of this fix.
      
      The problem is that the access to neigh->arp_queue is inconsistent, we
      grab references when dropping the lock lock to call
      neigh->ops->solicit() but this does not prevent other threads of
      control from trying to send out that packet at the same time causing
      corruptions because both code paths believe they have exclusive access
      to the skb.
      
      The best option seems to be to hold the write lock on neigh->lock
      during the ->solicit() call.  I looked at all of the ndisc_ops
      implementations and this seems workable.  The only case that needs
      special care is the IPV4 ARP implementation of arp_solicit().  It
      wants to take neigh->lock as a reader to protect the header entry in
      neigh->ha during the emission of the soliciation.  We can simply
      remove the read lock calls to take care of that since holding the lock
      as a writer at the caller providers a superset of the protection
      afforded by the existing read locking.
      
      The rest of the ->solicit() implementations don't care whether the
      neigh is locked or not.
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      69cc64d8
    • D
      hci_ldisc: fix null pointer deref · 3611f4d2
      David Newall 提交于
      Arjan:
      
        With the help of kerneloops.org I've spotted a nice little interaction
        between the TTY layer and the bluetooth code, however the tty layer is not
        something I'm all too familiar with so I rather ask than brute-force fix the
        code incorrectly.
      
        The raw details are at:
        http://www.kerneloops.org/search.php?search=uart_flush_buffer
      
        What happens is that, on closing the bluetooth tty, the tty layer goes
        into the release_dev() function, which first does a bunch of stuff, then
        sets the file->private_data to NULL, does some more stuff and then calls the
        ldisc close function.  Which in this case, is hci_uart_tty_close().
      
        Now, hci_uart_tty_close() calls hci_uart_close() which clears some
        internal bit, and then calls hci_uart_flush()...  which calls back to the
        tty layers' uart_flush_buffer() function.  (in drivers/bluetooth/hci_tty.c
        around line 194) Which then WARN_ON()'s because that's not allowed/supposed
        to be called this late in the shutdown of the port....
      
        Should the bluetooth driver even call this flush function at all??
      
      David:
      
        This seems to be what happens: Hci_uart_close() flushes using
        hci_uart_flush().  Subsequently, in hci_dev_do_close(), (one step in
        hci_unregister_dev()), hci_uart_flush() is called again.  The comment in
        uart_flush_buffer(), relating to the WARN_ON(), indicates you can't flush
        after the port is closed; which sounds reasonable.  I think hci_uart_close()
        should set hdev->flush to NULL before returning.  Hci_dev_do_close() does
        check for this.  The code path is rather involved and I'm not entirely clear
        of all steps, but I think that's what should be done.
      Signed-off-by: NAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      3611f4d2
    • J
      [AX25] ax25_ds_timer: use mod_timer instead of add_timer · e848b583
      Jarek Poplawski 提交于
      This patch changes current use of: init_timer(), add_timer()
      and del_timer() to setup_timer() with mod_timer(), which
      should be safer anyway.
      Reported-by: NJann Traschewski <jann@gmx.de>
      Signed-off-by: NJarek Poplawski <jarkao2@gmail.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      e848b583
    • J
      [AX25] ax25_timer: use mod_timer instead of add_timer · 21fab4a8
      Jarek Poplawski 提交于
      According to one of Jann's OOPS reports it looks like
      BUG_ON(timer_pending(timer)) triggers during add_timer()
      in ax25_start_t1timer(). This patch changes current use
      of: init_timer(), add_timer() and del_timer() to
      setup_timer() with mod_timer(), which should be safer
      anyway.
      Reported-by: NJann Traschewski <jann@gmx.de>
      Signed-off-by: NJarek Poplawski <jarkao2@gmail.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      21fab4a8
    • J
      [AX25] ax25_route: make ax25_route_lock BH safe · 4de211f1
      Jarek Poplawski 提交于
      > =================================
      > [ INFO: inconsistent lock state ]
      > 2.6.24-dg8ngn-p02 #1
      > ---------------------------------
      > inconsistent {softirq-on-W} -> {in-softirq-R} usage.
      > linuxnet/3046 [HC0[0]:SC1[2]:HE1:SE0] takes:
      >  (ax25_route_lock){--.+}, at: [<f8a0cfb7>] ax25_get_route+0x18/0xb7 [ax25]
      > {softirq-on-W} state was registered at:
      ...
      
      This lockdep report shows that ax25_route_lock is taken for reading in
      softirq context, and for writing in process context with BHs enabled.
      So, to make this safe, all write_locks in ax25_route.c are changed to
      _bh versions.
      
      Reported-by: Jann Traschewski <jann@gmx.de>,
      Signed-off-by: NJarek Poplawski <jarkao2@gmail.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      4de211f1
    • J
      [AX25] af_ax25: remove sock lock in ax25_info_show() · 1105b5d1
      Jarek Poplawski 提交于
      This lockdep warning:
      
      > =======================================================
      > [ INFO: possible circular locking dependency detected ]
      > 2.6.24 #3
      > -------------------------------------------------------
      > swapper/0 is trying to acquire lock:
      >  (ax25_list_lock){-+..}, at: [<f91dd3b1>] ax25_destroy_socket+0x171/0x1f0 [ax25]
      >
      > but task is already holding lock:
      >  (slock-AF_AX25){-+..}, at: [<f91dbabc>] ax25_std_heartbeat_expiry+0x1c/0xe0 [ax25]
      >
      > which lock already depends on the new lock.
      ...
      
      shows that ax25_list_lock and slock-AF_AX25 are taken in different
      order: ax25_info_show() takes slock (bh_lock_sock(ax25->sk)) while
      ax25_list_lock is held, so reversely to other functions. To fix this
      the sock lock should be moved to ax25_info_start(), and there would
      be still problem with breaking ax25_list_lock (it seems this "proper"
      order isn't optimal yet). But, since it's only for reading proc info
      it seems this is not necessary (e.g.  ax25_send_to_raw() does similar
      reading without this lock too).
      
      So, this patch removes sock lock to avoid deadlock possibility; there
      is also used sock_i_ino() function, which reads sk_socket under proper
      read lock. Additionally printf format of this i_ino is changed to %lu.
      Reported-by: NBernard Pidoux F6BVP <f6bvp@free.fr>
      Signed-off-by: NJarek Poplawski <jarkao2@gmail.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      1105b5d1
    • S
      fib_trie: /proc/net/route performance improvement · 8315f5d8
      Stephen Hemminger 提交于
      Use key/offset caching to change /proc/net/route (use by iputils route)
      from O(n^2) to O(n). This improves performance from 30sec with 160,000
      routes to 1sec.
      Signed-off-by: NStephen Hemminger <shemminger@vyatta.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      8315f5d8
    • S
      fib_trie: handle empty tree · ec28cf73
      Stephen Hemminger 提交于
      This fixes possible problems when trie_firstleaf() returns NULL
      to trie_leafindex().
      Signed-off-by: NStephen Hemminger <shemminger@vyatta.com>
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      ec28cf73
    • D
      [IPV4]: Remove IP_TOS setting privilege checks. · e4f8b5d4
      David S. Miller 提交于
      Various RFCs have all sorts of things to say about the CS field of the
      DSCP value.  In particular they try to make the distinction between
      values that should be used by "user applications" and things like
      routing daemons.
      
      This seems to have influenced the CAP_NET_ADMIN check which exists for
      IP_TOS socket option settings, but in fact it has an off-by-one error
      so it wasn't allowing CS5 which is meant for "user applications" as
      well.
      
      Further adding to the inconsistency and brokenness here, IPV6 does not
      validate the DSCP values specified for the IPV6_TCLASS socket option.
      
      The real actual uses of these TOS values are system specific in the
      final analysis, and these RFC recommendations are just that, "a
      recommendation".  In fact the standards very purposefully use
      "SHOULD" and "SHOULD NOT" when describing how these values can be
      used.
      
      In the final analysis the only clean way to provide consistency here
      is to remove the CAP_NET_ADMIN check.  The alternatives just don't
      work out:
      
      1) If we add the CAP_NET_ADMIN check to ipv6, this can break existing
         setups.
      
      2) If we just fix the off-by-one error in the class comparison in
         IPV4, certain DSCP values can be used in IPV6 but not IPV4 by
         default.  So people will just ask for a sysctl asking to
         override that.
      
      I checked several other freely available kernel trees and they
      do not make any privilege checks in this area like we do.  For
      the BSD stacks, this goes back all the way to Stevens Volume 2
      and beyond.
      Signed-off-by: NDavid S. Miller <davem@davemloft.net>
      e4f8b5d4
    • D
  2. 12 2月, 2008 30 次提交