Skip to content
Snippets Groups Projects
  1. Apr 11, 2014
    • David S. Miller's avatar
      net: Fix use after free by removing length arg from sk_data_ready callbacks. · 676d2369
      David S. Miller authored
      
      Several spots in the kernel perform a sequence like:
      
      	skb_queue_tail(&sk->s_receive_queue, skb);
      	sk->sk_data_ready(sk, skb->len);
      
      But at the moment we place the SKB onto the socket receive queue it
      can be consumed and freed up.  So this skb->len access is potentially
      to freed up memory.
      
      Furthermore, the skb->len can be modified by the consumer so it is
      possible that the value isn't accurate.
      
      And finally, no actual implementation of this callback actually uses
      the length argument.  And since nobody actually cared about it's
      value, lots of call sites pass arbitrary values in such as '0' and
      even '1'.
      
      So just remove the length argument from the callback, that way there
      is no confusion whatsoever and all of these use-after-free cases get
      fixed as a side effect.
      
      Based upon a patch by Eric Dumazet and his suggestion to audit this
      issue tree-wide.
      
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      676d2369
  2. Mar 27, 2014
  3. Jan 22, 2014
    • David S. Miller's avatar
      net: Fix some fallout from the etner_addr_copy() changes. · 9be68c1a
      David S. Miller authored
      
      net/appletalk/aarp.c: In function ‘__aarp_send_query’:
      net/appletalk/aarp.c:137:2: error: implicit declaration of function ‘ether_addr_copy’ [-Werror=implicit-function-declaration]
       ...
      net/atm/lec.c: In function ‘send_to_lecd’:
      net/atm/lec.c:524:3: warning: passing argument 1 of ‘ether_addr_copy’ from incompatible pointer type [enabled by default]
      In file included from net/atm/lec.c:17:0:
      include/linux/etherdevice.h:227:20: note: expected ‘u8 *’ but argument is of type ‘unsigned char (*)[6]’
       ...
      net/caif/caif_usb.c: In function ‘cfusbl_create’:
      net/caif/caif_usb.c:108:2: error: implicit declaration of function ‘ether_addr_copy’ [-Werror=implicit-function-declaration]
      
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      9be68c1a
    • Joe Perches's avatar
      atm: Use ether_addr_copy · 116e853f
      Joe Perches authored
      
      Use ether_addr_copy instead of memcpy(a, b, ETH_ALEN) to
      save some cycles on arm and powerpc.
      
      Signed-off-by: default avatarJoe Perches <joe@perches.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      116e853f
  4. Nov 21, 2013
    • Hannes Frederic Sowa's avatar
      net: rework recvmsg handler msg_name and msg_namelen logic · f3d33426
      Hannes Frederic Sowa authored
      
      This patch now always passes msg->msg_namelen as 0. recvmsg handlers must
      set msg_namelen to the proper size <= sizeof(struct sockaddr_storage)
      to return msg_name to the user.
      
      This prevents numerous uninitialized memory leaks we had in the
      recvmsg handlers and makes it harder for new code to accidentally leak
      uninitialized memory.
      
      Optimize for the case recvfrom is called with NULL as address. We don't
      need to copy the address at all, so set it to NULL before invoking the
      recvmsg handler. We can do so, because all the recvmsg handlers must
      cope with the case a plain read() is called on them. read() also sets
      msg_name to NULL.
      
      Also document these changes in include/linux/net.h as suggested by David
      Miller.
      
      Changes since RFC:
      
      Set msg->msg_name = NULL if user specified a NULL in msg_name but had a
      non-null msg_namelen in verify_iovec/verify_compat_iovec. This doesn't
      affect sendto as it would bail out earlier while trying to copy-in the
      address. It also more naturally reflects the logic by the callers of
      verify_iovec.
      
      With this change in place I could remove "
      if (!uaddr || msg_sys->msg_namelen == 0)
      	msg->msg_name = NULL
      ".
      
      This change does not alter the user visible error logic as we ignore
      msg_namelen as long as msg_name is NULL.
      
      Also remove two unnecessary curly brackets in ___sys_recvmsg and change
      comments to netdev style.
      
      Cc: David Miller <davem@davemloft.net>
      Suggested-by: default avatarEric Dumazet <eric.dumazet@gmail.com>
      Signed-off-by: default avatarHannes Frederic Sowa <hannes@stressinduktion.org>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      f3d33426
  5. May 29, 2013
  6. May 28, 2013
  7. Apr 09, 2013
    • Al Viro's avatar
      procfs: new helper - PDE_DATA(inode) · d9dda78b
      Al Viro authored
      
      The only part of proc_dir_entry the code outside of fs/proc
      really cares about is PDE(inode)->data.  Provide a helper
      for that; static inline for now, eventually will be moved
      to fs/proc, along with the knowledge of struct proc_dir_entry
      layout.
      
      Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
      d9dda78b
  8. Apr 07, 2013
  9. Mar 28, 2013
    • Simon Horman's avatar
      net: add ETH_P_802_3_MIN · e5c5d22e
      Simon Horman authored
      
      Add a new constant ETH_P_802_3_MIN, the minimum ethernet type for
      an 802.3 frame. Frames with a lower value in the ethernet type field
      are Ethernet II.
      
      Also update all the users of this value that David Miller and
      I could find to use the new constant.
      
      Also correct a bug in util.c. The comparison with ETH_P_802_3_MIN
      should be >= not >.
      
      As suggested by Jesse Gross.
      
      Compile tested only.
      
      Cc: David Miller <davem@davemloft.net>
      Cc: Jesse Gross <jesse@nicira.com>
      Cc: Karsten Keil <isdn@linux-pingi.de>
      Cc: John W. Linville <linville@tuxdriver.com>
      Cc: Johannes Berg <johannes@sipsolutions.net>
      Cc: Bart De Schuymer <bart.de.schuymer@pandora.be>
      Cc: Stephen Hemminger <stephen@networkplumber.org>
      Cc: Patrick McHardy <kaber@trash.net>
      Cc: Marcel Holtmann <marcel@holtmann.org>
      Cc: Gustavo Padovan <gustavo@padovan.org>
      Cc: Johan Hedberg <johan.hedberg@gmail.com>
      Cc: linux-bluetooth@vger.kernel.org
      Cc: netfilter-devel@vger.kernel.org
      Cc: bridge@lists.linux-foundation.org
      Cc: linux-wireless@vger.kernel.org
      Cc: linux1394-devel@lists.sourceforge.net
      Cc: linux-media@vger.kernel.org
      Cc: netdev@vger.kernel.org
      Cc: dev@openvswitch.org
      Acked-by: default avatarMauro Carvalho Chehab <mchehab@redhat.com>
      Acked-by: default avatarStefan Richter <stefanr@s5r6.in-berlin.de>
      Signed-off-by: default avatarSimon Horman <horms@verge.net.au>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      e5c5d22e
  10. Feb 28, 2013
    • Sasha Levin's avatar
      hlist: drop the node parameter from iterators · b67bfe0d
      Sasha Levin authored
      
      I'm not sure why, but the hlist for each entry iterators were conceived
      
              list_for_each_entry(pos, head, member)
      
      The hlist ones were greedy and wanted an extra parameter:
      
              hlist_for_each_entry(tpos, pos, head, member)
      
      Why did they need an extra pos parameter? I'm not quite sure. Not only
      they don't really need it, it also prevents the iterator from looking
      exactly like the list iterator, which is unfortunate.
      
      Besides the semantic patch, there was some manual work required:
      
       - Fix up the actual hlist iterators in linux/list.h
       - Fix up the declaration of other iterators based on the hlist ones.
       - A very small amount of places were using the 'node' parameter, this
       was modified to use 'obj->member' instead.
       - Coccinelle didn't handle the hlist_for_each_entry_safe iterator
       properly, so those had to be fixed up manually.
      
      The semantic patch which is mostly the work of Peter Senna Tschudin is here:
      
      @@
      iterator name hlist_for_each_entry, hlist_for_each_entry_continue, hlist_for_each_entry_from, hlist_for_each_entry_rcu, hlist_for_each_entry_rcu_bh, hlist_for_each_entry_continue_rcu_bh, for_each_busy_worker, ax25_uid_for_each, ax25_for_each, inet_bind_bucket_for_each, sctp_for_each_hentry, sk_for_each, sk_for_each_rcu, sk_for_each_from, sk_for_each_safe, sk_for_each_bound, hlist_for_each_entry_safe, hlist_for_each_entry_continue_rcu, nr_neigh_for_each, nr_neigh_for_each_safe, nr_node_for_each, nr_node_for_each_safe, for_each_gfn_indirect_valid_sp, for_each_gfn_sp, for_each_host;
      
      type T;
      expression a,c,d,e;
      identifier b;
      statement S;
      @@
      
      -T b;
          <+... when != b
      (
      hlist_for_each_entry(a,
      - b,
      c, d) S
      |
      hlist_for_each_entry_continue(a,
      - b,
      c) S
      |
      hlist_for_each_entry_from(a,
      - b,
      c) S
      |
      hlist_for_each_entry_rcu(a,
      - b,
      c, d) S
      |
      hlist_for_each_entry_rcu_bh(a,
      - b,
      c, d) S
      |
      hlist_for_each_entry_continue_rcu_bh(a,
      - b,
      c) S
      |
      for_each_busy_worker(a, c,
      - b,
      d) S
      |
      ax25_uid_for_each(a,
      - b,
      c) S
      |
      ax25_for_each(a,
      - b,
      c) S
      |
      inet_bind_bucket_for_each(a,
      - b,
      c) S
      |
      sctp_for_each_hentry(a,
      - b,
      c) S
      |
      sk_for_each(a,
      - b,
      c) S
      |
      sk_for_each_rcu(a,
      - b,
      c) S
      |
      sk_for_each_from
      -(a, b)
      +(a)
      S
      + sk_for_each_from(a) S
      |
      sk_for_each_safe(a,
      - b,
      c, d) S
      |
      sk_for_each_bound(a,
      - b,
      c) S
      |
      hlist_for_each_entry_safe(a,
      - b,
      c, d, e) S
      |
      hlist_for_each_entry_continue_rcu(a,
      - b,
      c) S
      |
      nr_neigh_for_each(a,
      - b,
      c) S
      |
      nr_neigh_for_each_safe(a,
      - b,
      c, d) S
      |
      nr_node_for_each(a,
      - b,
      c) S
      |
      nr_node_for_each_safe(a,
      - b,
      c, d) S
      |
      - for_each_gfn_sp(a, c, d, b) S
      + for_each_gfn_sp(a, c, d) S
      |
      - for_each_gfn_indirect_valid_sp(a, c, d, b) S
      + for_each_gfn_indirect_valid_sp(a, c, d) S
      |
      for_each_host(a,
      - b,
      c) S
      |
      for_each_host_safe(a,
      - b,
      c, d) S
      |
      for_each_mesh_entry(a,
      - b,
      c, d) S
      )
          ...+>
      
      [akpm@linux-foundation.org: drop bogus change from net/ipv4/raw.c]
      [akpm@linux-foundation.org: drop bogus hunk from net/ipv6/raw.c]
      [akpm@linux-foundation.org: checkpatch fixes]
      [akpm@linux-foundation.org: fix warnings]
      [akpm@linux-foudnation.org: redo intrusive kvm changes]
      Tested-by: default avatarPeter Senna Tschudin <peter.senna@gmail.com>
      Acked-by: default avatarPaul E. McKenney <paulmck@linux.vnet.ibm.com>
      Signed-off-by: default avatarSasha Levin <sasha.levin@oracle.com>
      Cc: Wu Fengguang <fengguang.wu@intel.com>
      Cc: Marcelo Tosatti <mtosatti@redhat.com>
      Cc: Gleb Natapov <gleb@redhat.com>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      b67bfe0d
  11. Feb 23, 2013
  12. Feb 18, 2013
  13. Dec 18, 2012
  14. Dec 02, 2012
  15. Nov 30, 2012
  16. Nov 28, 2012
  17. Nov 26, 2012
    • David Woodhouse's avatar
      atm: br2684: Fix excessive queue bloat · ae088d66
      David Woodhouse authored
      
      There's really no excuse for an additional wmem_default of buffering
      between the netdev queue and the ATM device. Two packets (one in-flight,
      and one ready to send) ought to be fine. It's not as if it should take
      long to get another from the netdev queue when we need it.
      
      If necessary we can make the queue space configurable later, but I don't
      think it's likely to be necessary.
      
      cf. commit 9d02daf7 (pppoatm: Fix
      excessive queue bloat) which did something very similar for PPPoATM.
      
      Note that there is a tremendously unlikely race condition which may
      result in qspace temporarily going negative. If a CPU running the
      br2684_pop() function goes off into the weeds for a long period of time
      after incrementing qspace to 1, but before calling netdev_wake_queue()...
      and another CPU ends up calling br2684_start_xmit() and *stopping* the
      queue again before the first CPU comes back, the netdev queue could
      end up being woken when qspace has already reached zero.
      
      An alternative approach to coping with this race would be to check in
      br2684_start_xmit() for qspace==0 and return NETDEV_TX_BUSY, but just
      using '> 0' and '< 1' for comparison instead of '== 0' and '!= 0' is
      simpler. It just warranted a mention of *why* we do it that way...
      
      Move the call to atmvcc->send() to happen *after* the accounting and
      potentially stopping the netdev queue, in br2684_xmit_vcc(). This matters
      if the ->send() call suffers an immediate failure, because it'll call
      br2684_pop() with the offending skb before returning. We want that to
      happen *after* we've done the initial accounting for the packet in
      question. Also make it return an appropriate success/failure indication
      while we're at it.
      
      Tested by running 'ping -l 1000 bottomless.aaisp.net.uk' from within my
      network, with only a single PPPoE-over-BR2684 link running. And after
      setting txqueuelen on the nas0 interface to something low (5, in fact).
      Before the patch, we'd see about 15 packets being queued and a resulting
      latency of ~56ms being reached. After the patch, we see only about 8,
      which is fairly much what we expect. And a max latency of ~36ms. On this
      OpenWRT box, wmem_default is 163840.
      
      Signed-off-by: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      Reviewed-by: default avatarKrzysztof Mazur <krzysiek@podlesie.net>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      ae088d66
  18. Aug 31, 2012
  19. Aug 16, 2012
  20. Jun 04, 2012
  21. May 16, 2012
  22. May 15, 2012
  23. May 10, 2012
    • Joe Perches's avatar
      atm: Convert compare_ether_addr to ether_addr_equal · 150238eb
      Joe Perches authored
      
      Use the new bool function ether_addr_equal to add
      some clarity and reduce the likelihood for misuse
      of compare_ether_addr for sorting.
      
      Done via cocci script:
      
      $ cat compare_ether_addr.cocci
      @@
      expression a,b;
      @@
      -	!compare_ether_addr(a, b)
      +	ether_addr_equal(a, b)
      
      @@
      expression a,b;
      @@
      -	compare_ether_addr(a, b)
      +	!ether_addr_equal(a, b)
      
      @@
      expression a,b;
      @@
      -	!ether_addr_equal(a, b) == 0
      +	ether_addr_equal(a, b)
      
      @@
      expression a,b;
      @@
      -	!ether_addr_equal(a, b) != 0
      +	!ether_addr_equal(a, b)
      
      @@
      expression a,b;
      @@
      -	ether_addr_equal(a, b) == 0
      +	!ether_addr_equal(a, b)
      
      @@
      expression a,b;
      @@
      -	ether_addr_equal(a, b) != 0
      +	ether_addr_equal(a, b)
      
      @@
      expression a,b;
      @@
      -	!!ether_addr_equal(a, b)
      +	ether_addr_equal(a, b)
      
      Signed-off-by: default avatarJoe Perches <joe@perches.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      150238eb
  24. Apr 15, 2012
  25. Apr 13, 2012
    • David Woodhouse's avatar
      pppoatm: Fix excessive queue bloat · 9d02daf7
      David Woodhouse authored
      
      We discovered that PPPoATM has an excessively deep transmit queue. A
      queue the size of the default socket send buffer (wmem_default) is
      maintained between the PPP generic core and the ATM device.
      
      Fix it to queue a maximum of *two* packets. The one the ATM device is
      currently working on, and one more for the ATM driver to process
      immediately in its TX done interrupt handler. The PPP core is designed
      to feed packets to the channel with minimal latency, so that really
      ought to be enough to keep the ATM device busy.
      
      While we're at it, fix the fact that we were triggering the wakeup
      tasklet on *every* pppoatm_pop() call. The comment saying "this is
      inefficient, but doing it right is too hard" turns out to be overly
      pessimistic... I think :)
      
      On machines like the Traverse Geos, with a slow Geode CPU and two
      high-speed ADSL2+ interfaces, there were reports of extremely high CPU
      usage which could partly be attributed to the extra wakeups.
      
      (The wakeup handling could actually be made a whole lot easier if we
       stop checking sk->sk_sndbuf altogether. Given that we now only queue
       *two* packets ever, one wonders what the point is. As it is, you could
       already deadlock the thing by setting the sk_sndbuf to a value lower
       than the MTU of the device, and it'd just block for ever.)
      
      Signed-off-by: default avatarDavid Woodhouse <David.Woodhouse@intel.com>
      Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      9d02daf7
  26. Apr 06, 2012
  27. Mar 28, 2012
  28. Mar 05, 2012
Loading