Skip to content
Snippets Groups Projects
  1. Feb 10, 2023
  2. Feb 09, 2023
  3. Feb 02, 2023
  4. Jan 23, 2023
  5. Jan 20, 2023
    • Tom Rini's avatar
      global: Finish CONFIG -> CFG migration · 6e7df1d1
      Tom Rini authored
      
      At this point, the remaining places where we have a symbol that is
      defined as CONFIG_... are in fairly odd locations. While as much dead
      code has been removed as possible, some of these locations are simply
      less obvious at first. In other cases, this code is used, but was
      defined in such a way as to have been missed by earlier checks.  Perform
      a rename of all such remaining symbols to be CFG_... rather than
      CONFIG_...
      
      Signed-off-by: default avatarTom Rini <trini@konsulko.com>
      Reviewed-by: Simon Glass's avatarSimon Glass <sjg@chromium.org>
      6e7df1d1
  6. Jan 11, 2023
  7. Dec 29, 2022
  8. Dec 22, 2022
  9. Dec 07, 2022
  10. Dec 05, 2022
  11. Dec 02, 2022
  12. Nov 28, 2022
    • Ying-Chun Liu (PaulLiu)'s avatar
      net: Add wget application · cfbae482
      Ying-Chun Liu (PaulLiu) authored and Tom Rini's avatar Tom Rini committed
      
      This commit adds a simple wget command that can download files
      from http server.
      
      The command syntax is
      wget ${loadaddr} <path of the file from server>
      
      Signed-off-by: default avatarDuncan Hare <DuncanCHare@yahoo.com>
      Signed-off-by: default avatarYing-Chun Liu (PaulLiu) <paul.liu@linaro.org>
      Reviewed-by: Simon Glass's avatarSimon Glass <sjg@chromium.org>
      Cc: Christian Gmeiner <christian.gmeiner@gmail.com>
      Cc: Joe Hershberger <joe.hershberger@ni.com>
      Cc: Michal Simek <michal.simek@xilinx.com>
      Cc: Ramon Fried <rfried.dev@gmail.com>
      Reviewed-by: default avatarRamon Fried <rfried.dev@gmail.com>
      cfbae482
    • Ying-Chun Liu (PaulLiu)'s avatar
      net: Add TCP protocol · a3bf193b
      Ying-Chun Liu (PaulLiu) authored and Tom Rini's avatar Tom Rini committed
      
      Currently file transfers are done using tftp or NFS both
      over udp. This requires a request to be sent from client
      (u-boot) to the boot server.
      
      The current standard is TCP with selective acknowledgment.
      
      Signed-off-by: default avatarDuncan Hare <DH@Synoia.com>
      Signed-off-by: default avatarDuncan Hare <DuncanCHare@yahoo.com>
      Signed-off-by: default avatarYing-Chun Liu (PaulLiu) <paul.liu@linaro.org>
      Reviewed-by: Simon Glass's avatarSimon Glass <sjg@chromium.org>
      Cc: Christian Gmeiner <christian.gmeiner@gmail.com>
      Cc: Joe Hershberger <joe.hershberger@ni.com>
      Cc: Michal Simek <michal.simek@xilinx.com>
      Cc: Ramon Fried <rfried.dev@gmail.com>
      Reviewed-by: default avatarRamon Fried <rfried.dev@gmail.com>
      a3bf193b
    • Rasmus Villemoes's avatar
      net: deal with fragment-overlapping-two-holes case · 06869686
      Rasmus Villemoes authored and Tom Rini's avatar Tom Rini committed
      
      With a suitable sequence of malicious packets, it's currently possible
      to get a hole descriptor to contain arbitrary attacker-controlled
      contents, and then with one more packet to use that as an arbitrary
      write vector.
      
      While one could possibly change the algorithm so we instead loop over
      all holes, and in each hole puts as much of the current fragment as
      belongs there (taking care to carefully update the hole list as
      appropriate), it's not worth the complexity: In real, non-malicious
      scenarios, one never gets overlapping fragments, and certainly not
      fragments that would be supersets of one another.
      
      So instead opt for this simple protection: Simply don't allow the
      eventual memcpy() to write beyond the last_byte of the current hole.
      
      Signed-off-by: default avatarRasmus Villemoes <rasmus.villemoes@prevas.dk>
      06869686
    • Rasmus Villemoes's avatar
      net: tftp: sanitize tftp block size, especially for TX · 087648b5
      Rasmus Villemoes authored and Tom Rini's avatar Tom Rini committed
      
      U-Boot does not support IP fragmentation on TX (and unless
      CONFIG_IP_DEFRAG is set, neither on RX). So the blocks we send must
      fit in a single ethernet packet.
      
      Currently, if tftpblocksize is set to something like 5000 and I
      tftpput a large enough file, U-Boot crashes because we overflow
      net_tx_packet (which only has room for 1500 bytes plus change).
      
      Similarly, if tftpblocksize is set to something larger than what we
      can actually receive (e.g. 50000, with NET_MAXDEFRAG being 16384), any
      tftp get just hangs because we never receive any packets.
      
      Signed-off-by: default avatarRasmus Villemoes <rasmus.villemoes@prevas.dk>
      Reviewed-by: default avatarRamon Fried <rfried.dev@gmail.com>
      087648b5
    • Rasmus Villemoes's avatar
      net: tftp: use IS_ENABLED(CONFIG_NET_TFTP_VARS) instead of #if · 4b8c44e3
      Rasmus Villemoes authored and Tom Rini's avatar Tom Rini committed
      
      Nothing inside this block depends on NET_TFTP_VARS to be set to parse
      correctly. Switch to C if() in preparation for adding code before
      this (to avoid a declaration-after-statement warning).
      
      Signed-off-by: default avatarRasmus Villemoes <rasmus.villemoes@prevas.dk>
      [trini: Update to cover CONFIG_TFTP_PORT case as well]
      Signed-off-by: default avatarTom Rini <trini@konsulko.com>
      4b8c44e3
    • Rasmus Villemoes's avatar
      net: fix ip_len in reassembled IP datagram · 06653c70
      Rasmus Villemoes authored and Tom Rini's avatar Tom Rini committed
      
      For some reason, the ip_len field in a reassembled IP datagram is set
      to just the size of the payload, but it should be set to the value it
      would have had if the datagram had never been fragmented in the first
      place, i.e. size of payload plus size of IP header.
      
      That latter value is currently returned correctly via the "len"
      variable. And before entering net_defragment(), len does have the
      value ntohs(ip->ip_len), so if we're not dealing with a
      fragment (so net_defragment leaves *len alone), that relationship of
      course also holds after the net_defragment() call.
      
      The only use I can find of ip->ip_len after the net_defragment call is
      the ntohs(ip->udp_len) > ntohs(ip->ip_len) sanity check - none of the
      functions that are passed the "ip" pointer themselves inspect ->ip_len
      but instead use the passed len.
      
      But that sanity check is a bit odd, since the RHS really should be
      "ntohs(ip->ip_len) - 20", i.e. the IP payload size.
      
      Now that we've fixed things so that len == ntohs(ip->ip_len) in all
      cases, change that sanity check to use len-20 as the RHS.
      
      Signed-off-by: default avatarRasmus Villemoes <rasmus.villemoes@prevas.dk>
      06653c70
    • Rasmus Villemoes's avatar
      net: (actually/better) deal with CVE-2022-{30790,30552} · 1817c382
      Rasmus Villemoes authored and Tom Rini's avatar Tom Rini committed
      
      I hit a strange problem with v2022.10: Sometimes my tftp transfer
      would seemingly just hang. It only happened for some files. Moreover,
      changing tftpblocksize from 65464 to 65460 or 65000 made it work again
      for all the files I tried. So I started suspecting it had something to
      do with the file sizes and in particular the way the tftp blocks get
      fragmented and reassembled.
      
      v2022.01 showed no problems with any of the files or any value of
      tftpblocksize.
      
      Looking at what had changed in net.c or tftp.c since January showed
      only one remotely interesting thing, b85d130e.
      
      So I fired up wireshark on my host to see if somehow one of the
      packets would be too small. But no, with both v2022.01 and v2022.10,
      the exact same sequence of packets were sent, all but the last of size
      1500, and the last being 1280 bytes.
      
      But then it struck me that 1280 is 5*256, so one of the two bytes
      on-the-wire is 0 and the other is 5, and when then looking at the code
      again the lack of endianness conversion becomes obvious. [ntohs is
      both applied to ip->ip_off just above, as well as to ip->ip_len just a
      little further down when the "len" is actually computed].
      
      IOWs the current code would falsely reject any packet which happens to
      be a multiple of 256 bytes in size, breaking tftp transfers somewhat
      randomly, and if it did get one of those "malicious" packets with
      ip_len set to, say, 27, it would be seen by this check as being 6912
      and hence not rejected.
      
      ====
      
      Now, just adding the missing ntohs() would make my initial problem go
      away, in that I can now download the file where the last fragment ends
      up being 1280 bytes. But there's another bug in the code and/or
      analysis: The right-hand side is too strict, in that it is ok for the
      last fragment not to have a multiple of 8 bytes as payload - it really
      must be ok, because nothing in the IP spec says that IP datagrams must
      have a multiple of 8 bytes as payload. And comments in the code also
      mention this.
      
      To fix that, replace the comparison with <= IP_HDR_SIZE and add
      another check that len is actually a multiple of 8 when the "more
      fragments" bit is set - which it necessarily is for the case where
      offset8 ends up being 0, since we're only called when
      
        (ip_off & (IP_OFFS | IP_FLAGS_MFRAG)).
      
      ====
      
      So, does this fix CVE-2022-30790 for real? It certainly correctly
      rejects the POC code which relies on sending a packet of size 27 with
      the MFRAG flag set. Can the attack be carried out with a size 27
      packet that doesn't set MFRAG (hence must set a non-zero fragment
      offset)? I dunno. If we get a packet without MFRAG, we update
      h->last_byte in the hole we've found to be start+len, hence we'd enter
      one of
      
      	if ((h >= thisfrag) && (h->last_byte <= start + len)) {
      
      or
      
      	} else if (h->last_byte <= start + len) {
      
      and thus won't reach any of the
      
      		/* overlaps with initial part of the hole: move this hole */
      		newh = thisfrag + (len / 8);
      
      		/* fragment sits in the middle: split the hole */
      		newh = thisfrag + (len / 8);
      
      IOW these division are now guaranteed to be exact, and thus I think
      the scenario in CVE-2022-30790 cannot happen anymore.
      
      ====
      
      However, there's a big elephant in the room, which has always been
      spelled out in the comments, and which makes me believe that one can
      still cause mayhem even with packets whose payloads are all 8-byte
      aligned:
      
          This code doesn't deal with a fragment that overlaps with two
          different holes (thus being a superset of a previously-received
          fragment).
      
      Suppose each character below represents 8 bytes, with D being already
      received data, H being a hole descriptor (struct hole), h being
      non-populated chunks, and P representing where the payload of a just
      received packet should go:
      
        DDDHhhhhDDDDHhhhDDDD
              PPPPPPPPP
      
      I'm pretty sure in this case we'd end up with h being the first hole,
      enter the simple
      
      	} else if (h->last_byte <= start + len) {
      		/* overlaps with final part of the hole: shorten this hole */
      		h->last_byte = start;
      
      case, and thus in the memcpy happily overwrite the second H with our
      chosen payload. This is probably worth fixing...
      
      Signed-off-by: default avatarRasmus Villemoes <rasmus.villemoes@prevas.dk>
      1817c382
    • Rasmus Villemoes's avatar
      net: compare received length to sizeof(ip_hdr), not sizeof(ip_udp_hdr) · ad359d89
      Rasmus Villemoes authored and Tom Rini's avatar Tom Rini committed
      
      While the code mostly/only handles UDP packets, it's possible for the
      last fragment of a fragmented UDP packet to be smaller than 28 bytes;
      it can be as small as 21 bytes (an IP header plus one byte of
      payload). So until we've performed the defragmentation step and thus
      know whether we're now holding a full packet, we should only check for
      the existence of the fields in the ip header, i.e. that there are at
      least 20 bytes present.
      
      In practice, we always seem to be handed a "len" of minimum 60 from the
      device layer, i.e. minimal ethernet frame length minus FCS, so this is
      mostly theoretical.
      
      After we've fetched the header's claimed length and used that to
      update the len variable, check that the header itself claims to be the
      minimal possible length.
      
      This is probably how CVE-2022-30552 should have been dealt with in the
      first place, because net_defragment() is not the only place that wants
      to know the size of the IP datagram payload: If we receive a
      non-fragmented ICMP packet, we pass "len" to receive_icmp() which in
      turn may pass it to ping_receive() which does
      
        compute_ip_checksum(icmph, len - IP_HDR_SIZE)
      
      and due to the signature of compute_ip_checksum(), that would then
      lead to accessing ~4G of address space, very likely leading to a
      crash.
      
      Signed-off-by: default avatarRasmus Villemoes <rasmus.villemoes@prevas.dk>
      ad359d89
    • Rasmus Villemoes's avatar
      net: improve check for no IP options · b0fcc48c
      Rasmus Villemoes authored and Tom Rini's avatar Tom Rini committed
      
      There's no reason we should accept an IP packet with a malformed IHL
      field. So ensure that it is exactly 5, not just <= 5.
      
      Signed-off-by: default avatarRasmus Villemoes <rasmus.villemoes@prevas.dk>
      Reviewed-by: default avatarRamon Fried <rfried.dev@gmail.com>
      b0fcc48c
  13. Nov 06, 2022
Loading