Remove all use of base::ByteSwapToLE* and apply spans

These are replaced with the bounds-safe byte conversion functions in
base::numerics.

Most of the sites doing so were trying to parse a byte stream, and we
have helpers for that now in base::numerics::U*FromLittleEndian, so
make use of those and turn code into using spans. Similarly,
constructing a byte stream in little endian from non-byte-sized
integers can use base::numerics::U*ToLittleEndian to be explicit.

Some code was doing invalid casts from bytes to non-byte-sized
integers which cause UB if the pointers are not correctly aligned,
so these are replaced with the use of byte spans. There were also
casts from structs to arrays of their fields, which is UB, so these
are also adjusted.

memcpy() is replaced with bounds-checked span::copy_from().

Bug: 40284755
Change-Id: I84594ff8ab4a2037e9d33bb6fe0d8ba60d5d7413
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5370942
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1276469}

base/containers/span.h

@@ -549,7 +549,9 @@ class GSL_POINTER span {
   // # Checks
   // The function CHECKs that the `other` span has the same size as itself and
   // will terminate otherwise.
-  constexpr void copy_from(span<const T, N> other) {
+  constexpr void copy_from(span<const T, N> other)
+    requires(!std::is_const_v<T>)
+  {
     CHECK_EQ(size_bytes(), other.size_bytes());
     // Verify non-overlapping in developer builds.
     //
@@ -959,7 +961,9 @@ class GSL_POINTER span<T, dynamic_extent, InternalPtrType> {
   // # Checks
   // The function CHECKs that the `other` span has the same size as itself and
   // will terminate otherwise.
-  constexpr void copy_from(span<const T> other) {
+  constexpr void copy_from(span<const T> other)
+    requires(!std::is_const_v<T>)
+  {
     CHECK_EQ(size_bytes(), other.size_bytes());
     // Verify non-overlapping in developer builds.
     //

base/files/file_util.cc

@@ -441,6 +441,11 @@ bool TruncateFile(FILE* file) {
   return true;
 }
 
+std::optional<uint64_t> ReadFile(const FilePath& filename,
+                                 span<uint8_t> buffer) {
+  return ReadFile(filename, base::as_writable_chars(buffer));
+}
+
 int ReadFile(const FilePath& filename, char* data, int max_size) {
   if (max_size < 0) {
     return -1;

base/files/file_util.h

@@ -562,6 +562,9 @@ BASE_EXPORT bool TruncateFile(FILE* file);
 // whose length exceeds INT_MAX.
 BASE_EXPORT std::optional<uint64_t> ReadFile(const FilePath& filename,
                                              span<char> buffer);
+BASE_EXPORT std::optional<uint64_t> ReadFile(const FilePath& filename,
+                                             span<uint8_t> buffer);
+
 // Same as above, but returns -1 on error.
 // TODO(https://crbug.com/1490484): Migrate callers to the span variant.
 BASE_EXPORT int ReadFile(const FilePath& filename, char* data, int max_size);

base/metrics/metrics_hashes.cc

@@ -12,25 +12,19 @@
 #include "base/containers/span.h"
 #include "base/hash/md5.h"
 #include "base/hash/sha1.h"
-#include "base/sys_byteorder.h"
+#include "base/numerics/byte_conversions.h"
 
 namespace base {
 namespace {
 
 // Converts the 8-byte prefix of an MD5 hash into a uint64_t value.
 inline uint64_t DigestToUInt64(const base::MD5Digest& digest) {
-  uint64_t value;
-  DCHECK_GE(sizeof(digest.a), sizeof(value));
-  memcpy(&value, digest.a, sizeof(value));
-  return base::NetToHost64(value);
+  return base::numerics::U64FromBigEndian(base::span(digest.a).first<8u>());
 }
 
 // Converts the 4-byte prefix of an MD5 hash into a uint32_t value.
 inline uint32_t DigestToUInt32(const base::MD5Digest& digest) {
-  uint32_t value;
-  DCHECK_GE(sizeof(digest.a), sizeof(value));
-  memcpy(&value, digest.a, sizeof(value));
-  return base::NetToHost32(value);
+  return base::numerics::U32FromBigEndian(base::span(digest.a).first<4u>());
 }
 
 }  // namespace
@@ -56,15 +50,8 @@ uint32_t HashMetricNameAs32Bits(std::string_view name) {
 uint32_t HashFieldTrialName(std::string_view name) {
   // SHA-1 is designed to produce a uniformly random spread in its output space,
   // even for nearly-identical inputs.
-  unsigned char sha1_hash[base::kSHA1Length];
-  base::SHA1HashBytes(reinterpret_cast<const unsigned char*>(name.data()),
-                      name.size(), sha1_hash);
-
-  uint32_t bits;
-  static_assert(sizeof(bits) < sizeof(sha1_hash), "more data required");
-  memcpy(&bits, sha1_hash, sizeof(bits));
-
-  return base::ByteSwapToLE32(bits);
+  SHA1Digest sha1_hash = base::SHA1HashSpan(base::as_byte_span(name));
+  return base::numerics::U32FromLittleEndian(base::span(sha1_hash).first<4u>());
 }
 
 }  // namespace base

base/sys_byteorder.h

@@ -22,30 +22,6 @@
 
 namespace base {
 
-// Converts the bytes in |x| from host order (endianness) to little endian, and
-// returns the result.
-inline constexpr uint16_t ByteSwapToLE16(uint16_t x) {
-#if defined(ARCH_CPU_LITTLE_ENDIAN)
-  return x;
-#else
-  return numerics::ByteSwap(x);
-#endif
-}
-inline constexpr uint32_t ByteSwapToLE32(uint32_t x) {
-#if defined(ARCH_CPU_LITTLE_ENDIAN)
-  return x;
-#else
-  return numerics::ByteSwap(x);
-#endif
-}
-inline constexpr uint64_t ByteSwapToLE64(uint64_t x) {
-#if defined(ARCH_CPU_LITTLE_ENDIAN)
-  return x;
-#else
-  return numerics::ByteSwap(x);
-#endif
-}
-
 // Converts the bytes in |x| from network to host order (endianness), and
 // returns the result.
 inline constexpr uint16_t NetToHost16(uint16_t x) {

base/sys_byteorder_unittest.cc

@@ -20,33 +20,6 @@ const uint64_t k64BitSwappedTestData = 0x11223344ddccbbaa;
 
 }  // namespace
 
-TEST(ByteOrderTest, ByteSwapToLE16) {
-  uint16_t le = base::ByteSwapToLE16(k16BitTestData);
-#if defined(ARCH_CPU_LITTLE_ENDIAN)
-  EXPECT_EQ(k16BitTestData, le);
-#else
-  EXPECT_EQ(k16BitSwappedTestData, le);
-#endif
-}
-
-TEST(ByteOrderTest, ByteSwapToLE32) {
-  uint32_t le = base::ByteSwapToLE32(k32BitTestData);
-#if defined(ARCH_CPU_LITTLE_ENDIAN)
-  EXPECT_EQ(k32BitTestData, le);
-#else
-  EXPECT_EQ(k32BitSwappedTestData, le);
-#endif
-}
-
-TEST(ByteOrderTest, ByteSwapToLE64) {
-  uint64_t le = base::ByteSwapToLE64(k64BitTestData);
-#if defined(ARCH_CPU_LITTLE_ENDIAN)
-  EXPECT_EQ(k64BitTestData, le);
-#else
-  EXPECT_EQ(k64BitSwappedTestData, le);
-#endif
-}
-
 TEST(ByteOrderTest, NetToHost16) {
   uint16_t h = base::NetToHost16(k16BitTestData);
 #if defined(ARCH_CPU_LITTLE_ENDIAN)

base/win/registry.h

@@ -209,7 +209,9 @@ class BASE_EXPORT RegistryValueIterator {
   // Advances to the next registry entry.
   void operator++();
 
+  // TODO(crbug.com/329476354): Provide a wcstring_view instead of a pointer.
   const wchar_t* Name() const { return name_.c_str(); }
+  // TODO(crbug.com/329476354): Provide a wcstring_view instead of a pointer.
   const wchar_t* Value() const { return value_.data(); }
   // ValueSize() is in bytes.
   DWORD ValueSize() const { return value_size_; }
@@ -231,6 +233,10 @@ class BASE_EXPORT RegistryValueIterator {
 
   // Current values.
   std::wstring name_;
+  // The vector always has a `0` at the end, after its `ValueSize() / 2u`
+  // elements (since ValueSize() is in bytes, but the vector is of 2-byte
+  // objects). This allows the value to always be read as a NUL-terminated
+  // string, even if it's holding another type of data.
   std::vector<wchar_t> value_;
   DWORD value_size_;
   DWORD type_;

chrome/test/BUILD.gn

@@ -6910,6 +6910,7 @@ test("unit_tests") {
     "//components/performance_manager/test_support:test_support_common",
     "//components/plus_addresses",
     "//components/plus_addresses:test_support",
+    "//components/policy:generated",
     "//components/policy/core/browser:test_support",
     "//components/policy/test_support:unittests",
     "//components/privacy_sandbox:privacy_sandbox",

components/policy/core/common/registry_dict.cc

@@ -257,11 +257,19 @@ void RegistryDict::ReadRegistry(HKEY hive, const std::wstring& root) {
       case REG_DWORD_LITTLE_ENDIAN:
       case REG_DWORD_BIG_ENDIAN:
         if (it.ValueSize() == sizeof(DWORD)) {
-          DWORD dword_value = *(reinterpret_cast<const DWORD*>(it.Value()));
-          if (it.Type() == REG_DWORD_BIG_ENDIAN)
-            dword_value = base::NetToHost32(dword_value);
-          else
-            dword_value = base::ByteSwapToLE32(dword_value);
+          auto value =
+              // TODO(crbug.com/40284755): it.Value() should return a
+              // wcstring_view which will be usable as a span directly. The
+              // ValueSize() here is the number of non-NUL *bytes* in the
+              // Value() string, so we cast the Value() to bytes which is what
+              // we want in the end anyway.
+              UNSAFE_BUFFERS(
+                  base::span(reinterpret_cast<const uint8_t*>(it.Value()),
+                             it.ValueSize()))
+                  .first<sizeof(DWORD)>();
+          DWORD dword_value = it.Type() == REG_DWORD_BIG_ENDIAN
+                                  ? base::numerics::U32FromBigEndian(value)
+                                  : base::numerics::U32FromLittleEndian(value);
           SetValue(name, base::Value(static_cast<int>(dword_value)));
           continue;
         }

components/variations/entropy_provider.cc

@@ -8,11 +8,12 @@
 #include <limits>
 
 #include "base/check_op.h"
+#include "base/containers/span.h"
 #include "base/hash/sha1.h"
+#include "base/numerics/byte_conversions.h"
 #include "base/rand_util.h"
 #include "base/strings/strcat.h"
 #include "base/strings/string_number_conversions.h"
-#include "base/sys_byteorder.h"
 #include "components/variations/variations_murmur_hash.h"
 
 namespace variations {
@@ -20,8 +21,7 @@ namespace variations {
 SHA1EntropyProvider::SHA1EntropyProvider(std::string_view entropy_source)
     : entropy_source_(entropy_source) {}
 
-SHA1EntropyProvider::~SHA1EntropyProvider() {
-}
+SHA1EntropyProvider::~SHA1EntropyProvider() = default;
 
 double SHA1EntropyProvider::GetEntropyForTrial(
     base::StringPiece trial_name,
@@ -39,15 +39,9 @@ double SHA1EntropyProvider::GetEntropyForTrial(
                             ? trial_name
                             : base::NumberToString(randomization_seed)});
 
-  unsigned char sha1_hash[base::kSHA1Length];
-  base::SHA1HashBytes(reinterpret_cast<const unsigned char*>(input.c_str()),
-                      input.size(),
-                      sha1_hash);
-
-  uint64_t bits;
-  static_assert(sizeof(bits) < sizeof(sha1_hash), "more data required");
-  memcpy(&bits, sha1_hash, sizeof(bits));
-  bits = base::ByteSwapToLE64(bits);
+  base::SHA1Digest sha1_hash = base::SHA1HashSpan(base::as_byte_span(input));
+  uint64_t bits =
+      base::numerics::U64FromLittleEndian(base::span(sha1_hash).first<8u>());
 
   return base::BitsToOpenEndedUnitInterval(bits);
 }

components/variations/variations_murmur_hash.cc

@@ -8,33 +8,35 @@
 
 #include "base/check_op.h"
 #include "base/compiler_specific.h"
-#include "base/sys_byteorder.h"
-#include "build/build_config.h"
-
-#if !(defined(ARCH_CPU_LITTLE_ENDIAN) || defined(ARCH_CPU_BIG_ENDIAN))
-#error "unknown endianness"
-#endif
+#include "base/containers/span.h"
+#include "base/numerics/byte_conversions.h"
 
 namespace variations {
 namespace internal {
 
 // static
 std::vector<uint32_t> VariationsMurmurHash::StringToLE32(
-    base::StringPiece data) {
+    base::StringPiece string) {
+  auto data = base::as_byte_span(string);
   const size_t data_size = data.size();
-  const size_t word_num = (data_size + 3) / 4;  // data_size / 4, rounding up
-  std::vector<uint32_t> words(word_num, 0);
-  DCHECK_GE(words.size() * sizeof(uint32_t), data_size * sizeof(char));
-  memcpy(words.data(), data.data(), data_size);
-
-#if defined(ARCH_CPU_BIG_ENDIAN)
-  // When packing chars into uint32_t, "abcd" may become 0x61626364 (big endian)
-  // or 0x64636261 (little endian). If big endian, swap everything, so we get
-  // the same values across platforms.
-  for (auto it = words.begin(); it != words.end(); ++it)
-    *it = base::ByteSwapToLE32(*it);
-#endif  // defined(ARCH_CPU_BIG_ENDIAN)
+  const size_t full_words = data_size / 4u;
+  // Include any partial word at the end of the `data` buffer.
+  const size_t total_words = (data_size + 3u) / 4u;
 
+  std::vector<uint32_t> words(total_words, 0u);
+  // Copy the words that are fully present in the `data` buffer.
+  for (size_t i = 0u; i < full_words; ++i) {
+    words[i] =
+        base::numerics::U32FromLittleEndian(data.subspan(4u * i).first<4u>());
+  }
+  // Copy the last partial-word from the end of the `data` buffer, padding
+  // the tail (MSBs) with 0.
+  if (total_words > full_words) {
+    const size_t rem = data_size % 4u;
+    std::array<uint8_t, 4u> bytes = {};
+    base::span(bytes).first(rem).copy_from(data.last(rem));
+    words[full_words] = base::numerics::U32FromLittleEndian(bytes);
+  }
   return words;
 }
 

fuchsia_web/runners/cast/cast_resolver.cc

@@ -18,8 +18,8 @@
 #include "base/files/file_util.h"
 #include "base/fuchsia/fuchsia_logging.h"
 #include "base/notreached.h"
+#include "base/numerics/byte_conversions.h"
 #include "base/strings/string_piece.h"
-#include "base/sys_byteorder.h"
 
 namespace {
 
@@ -51,17 +51,11 @@ uint64_t FetchAbiRevision() {
 
   // Read the Little Endian representation of the unsigned 64-bit integer ABI
   // revision from the file in the metadata directory.
-  uint64_t abi_revision_le = 0u;
-  int read_bytes = base::ReadFile(base::FilePath(kPkgAbiRevisionPath),
-                                  reinterpret_cast<char*>(&abi_revision_le),
-                                  sizeof(abi_revision_le));
-  CHECK_EQ(read_bytes, static_cast<int>(sizeof(abi_revision_le)));
-
-  // Swap the byte-order of `abi_revision_le` from little-endian to host-endian
-  // by using `ByteSwapToLE64()`. If the host is little-endian then this is a
-  // no-op, otherwise the byte order will be swapped, resulting in the correct
-  // big-endian/host-endian.
-  return base::ByteSwapToLE64(abi_revision_le);
+  std::array<uint8_t, 8u> abi_revision_le = {};
+  std::optional<uint64_t> read_bytes =
+      base::ReadFile(base::FilePath(kPkgAbiRevisionPath), abi_revision_le);
+  CHECK_EQ(read_bytes.value(), sizeof(abi_revision_le));
+  return base::numerics::U64FromLittleEndian(abi_revision_le);
 }
 
 }  // namespace

third_party/blink/public/common/privacy_budget/identifiability_internal_templates.h

@@ -5,11 +5,13 @@
 #ifndef THIRD_PARTY_BLINK_PUBLIC_COMMON_PRIVACY_BUDGET_IDENTIFIABILITY_INTERNAL_TEMPLATES_H_
 #define THIRD_PARTY_BLINK_PUBLIC_COMMON_PRIVACY_BUDGET_IDENTIFIABILITY_INTERNAL_TEMPLATES_H_
 
+#include <concepts>
 #include <cstdint>
 #include <cstring>
 #include <type_traits>
 
-#include "base/template_util.h"
+#include "base/containers/span.h"
+#include "base/numerics/byte_conversions.h"
 
 namespace blink {
 
@@ -47,25 +49,29 @@ using has_unique_object_representations = std::is_arithmetic<T>;
 // numbers. These are 80-bits wide, but in practice includes 6 bytes of padding
 // in order to extend the size to 16 bytes. The extra bytes are uninitialized
 // and will not contribute a stable digest.
-template <
-    typename T,
-    typename std::enable_if_t<std::is_same<T, std::remove_cvref_t<T>>::value &&
-                              std::is_trivially_copyable<T>::value &&
-                              has_unique_object_representations<T>::value &&
-                              sizeof(T) <= sizeof(int64_t)>* = nullptr>
-constexpr int64_t DigestOfObjectRepresentation(T in) {
-  // If |in| is small enough, the digest is itself. There's no point hashing
-  // this value since the identity has all the properties we are looking for
-  // in a digest.
-  if (std::is_integral<T>::value && std::is_signed<T>::value)
+template <typename T>
+constexpr int64_t DigestOfObjectRepresentation(T in)
+  requires(std::same_as<T, std::remove_cvref_t<T>> &&
+           std::is_trivially_copyable_v<T> &&
+           has_unique_object_representations<T>::value &&
+           sizeof(T) <= sizeof(int64_t))
+{
+  if constexpr (std::is_integral<T>::value &&
+                (std::is_signed<T>::value || sizeof(T) < sizeof(int64_t))) {
+    // If |in| is small enough, the digest is itself. There's no point hashing
+    // this value since the identity has all the properties we are looking for
+    // in a digest.
     return in;
-
-  if (std::is_integral<T>::value && sizeof(T) < sizeof(int64_t))
-    return in;
-
-  int64_t result = 0;
-  std::memcpy(&result, &in, sizeof(in));
-  return result;
+  } else {
+    // Otherwise we treat the native byte representation as the digest. If the
+    // type is a structure containing non-byte-sized integers, this would
+    // produce a different absolute output on BE and LE machines (though BE
+    // machines are not supported by Chromium).
+    std::array<uint8_t, 8u> bytes = {};
+    base::span(bytes).first<sizeof(T)>().copy_from(
+        base::byte_span_from_ref(in));
+    return static_cast<int64_t>(base::numerics::U64FromNativeEndian(bytes));
+  }
 }
 
 }  // namespace internal

third_party/blink/public/common/privacy_budget/identifiable_token_builder.h

@@ -101,8 +101,7 @@ class BLINK_COMMON_EXPORT IdentifiableTokenBuilder {
                 sizeof(T) <= sizeof(uint64_t)>* = nullptr>
   IdentifiableTokenBuilder& AddValue(T in) {
     AlignPartialBuffer();
-    int64_t clean_buffer =
-        base::ByteSwapToLE64(internal::DigestOfObjectRepresentation(in));
+    int64_t clean_buffer = internal::DigestOfObjectRepresentation(in);
     return AddBytes(base::make_span(
         reinterpret_cast<const uint8_t*>(&clean_buffer), sizeof(clean_buffer)));
   }

third_party/blink/tools/blinkpy/presubmit/audit_non_blink_usage.py

@@ -295,10 +295,10 @@ _CONFIG = [
             'base::SingleThreadTaskRunnerThreadMode',
 
             # Byte order
-            'base::ByteSwap',
-            'base::ReadBigEndian',
             'base::NetToHost(16|32|64)',
             'base::HostToNet(16|32|64)',
+            'base::numerics::U(8|16|32|64)(To|From)(Big|Little|Native)Endian'
+            'base::numerics::ByteSwap'
 
             # (Cryptographic) random number generation
             'base::RandUint64',

ui/base/x/x11_cursor_loader.cc

@@ -10,6 +10,7 @@
 #include <string>
 
 #include "base/compiler_specific.h"
+#include "base/containers/span.h"
 #include "base/environment.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
@@ -17,12 +18,12 @@
 #include "base/memory/ref_counted_memory.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/no_destructor.h"
+#include "base/numerics/byte_conversions.h"
 #include "base/sequence_checker.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
-#include "base/sys_byteorder.h"
 #include "base/task/task_traits.h"
 #include "base/task/thread_pool.h"
 #include "base/time/time.h"
@@ -528,21 +529,35 @@ void XCursorLoader::OnPropertyChanged(x11::Atom property,
 std::vector<XCursorLoader::Image> ParseCursorFile(
     scoped_refptr<base::RefCountedMemory> file,
     uint32_t preferred_size) {
-  constexpr uint32_t kMagic = 0x72756358;
-  constexpr uint32_t kImageType = 0xfffd0002;
+  constexpr uint32_t kMagic = 0x72756358u;
+  constexpr uint32_t kImageType = 0xfffd0002u;
 
-  const uint8_t* mem = file->data();
-  size_t offset = 0;
+  size_t offset = 0u;
 
-  auto ReadU32s = [&](void* dest, size_t len) {
-    DCHECK_EQ(len % 4, 0u);
-    if (offset >= file->size() || offset + len > file->size())
+  // Reads 32-bit values from `file` and writes them into the `dest` buffer.
+  auto ReadU32s = [&](base::span<uint8_t> dest) {
+    CHECK_EQ(dest.size() % 4u, 0u);
+    auto src = base::span(*file);
+    if (dest.size() > src.size() - offset) {
       return false;
-    const auto* src32 = reinterpret_cast<const uint32_t*>(mem + offset);
-    auto* dest32 = reinterpret_cast<uint32_t*>(dest);
-    for (size_t i = 0; i < len / 4; i++)
-      dest32[i] = base::ByteSwapToLE32(src32[i]);
-    offset += len;
+    }
+    for (size_t i = 0; i < dest.size(); i += 4u) {
+      uint32_t pixel = base::numerics::U32FromLittleEndian(
+          src.subspan(offset + i).first<4u>());
+      dest.subspan(i, 4u).copy_from(base::byte_span_from_ref(pixel));
+    }
+    offset += dest.size();
+    return true;
+  };
+  // Reads a single 32-bit value from `file` and writes it to `dest`.
+  auto ReadU32 = [&](uint32_t& dest) {
+    auto src = base::span(*file);
+    if (sizeof(dest) > src.size() - offset) {
+      return false;
+    }
+    dest = base::numerics::U32FromLittleEndian(
+        src.subspan(offset).first<sizeof(dest)>());
+    offset += sizeof(dest);
     return true;
   };
 
@@ -552,8 +567,13 @@ std::vector<XCursorLoader::Image> ParseCursorFile(
     uint32_t version;
     uint32_t ntoc;
   } header;
-  if (!ReadU32s(&header, sizeof(FileHeader)) || header.magic != kMagic)
+  if (!ReadU32(header.magic) ||    //
+      !ReadU32(header.header) ||   //
+      !ReadU32(header.version) ||  //
+      !ReadU32(header.ntoc) ||     //
+      header.magic != kMagic) {
     return {};
+  }
 
   struct TableOfContentsEntry {
     uint32_t type;
@@ -561,10 +581,13 @@ std::vector<XCursorLoader::Image> ParseCursorFile(
     uint32_t position;
   };
   std::vector<TableOfContentsEntry> toc;
-  for (uint32_t i = 0; i < header.ntoc; i++) {
+  for (uint32_t i = 0u; i < header.ntoc; i++) {
     TableOfContentsEntry entry;
-    if (!ReadU32s(&entry, sizeof(TableOfContentsEntry)))
+    if (!ReadU32(entry.type) ||     //
+        !ReadU32(entry.subtype) ||  //
+        !ReadU32(entry.position)) {
       return {};
+    }
     toc.push_back(entry);
   }
 
@@ -590,7 +613,10 @@ std::vector<XCursorLoader::Image> ParseCursorFile(
       uint32_t subtype;
       uint32_t version;
     } chunk_header;
-    if (!ReadU32s(&chunk_header, sizeof(ChunkHeader)) ||
+    if (!ReadU32(chunk_header.header) ||   //
+        !ReadU32(chunk_header.type) ||     //
+        !ReadU32(chunk_header.subtype) ||  //
+        !ReadU32(chunk_header.version) ||  //
         chunk_header.type != entry.type ||
         chunk_header.subtype != entry.subtype) {
       continue;
@@ -603,17 +629,31 @@ std::vector<XCursorLoader::Image> ParseCursorFile(
       uint32_t yhot;
       uint32_t delay;
     } image;
-    if (!ReadU32s(&image, sizeof(ImageHeader)))
+    if (!ReadU32(image.width) ||   //
+        !ReadU32(image.height) ||  //
+        !ReadU32(image.xhot) ||    //
+        !ReadU32(image.yhot) ||    //
+        !ReadU32(image.delay)) {
       continue;
+    }
     // Ignore unreasonably-sized cursors to prevent allocating too much
     // memory in the bitmap below.
-    if (image.width > 8192 || image.height > 8192) {
+    if (image.width > 8192u || image.height > 8192u) {
       continue;
     }
     SkBitmap bitmap;
     bitmap.allocN32Pixels(image.width, image.height);
-    if (!ReadU32s(bitmap.getPixels(), bitmap.computeByteSize()))
+    base::span<uint8_t> pixels =
+        // SAFETY: SkBitmap promises that getPixels() returns a pointer to
+        // at least as many bytes as computeByteSize().
+        //
+        // TODO(crbug.com/40284755): SkBitmap should provide a span-based
+        // API.
+        UNSAFE_BUFFERS(base::span(static_cast<uint8_t*>(bitmap.getPixels()),
+                                  bitmap.computeByteSize()));
+    if (!ReadU32s(pixels)) {
       continue;
+    }
     images.push_back(XCursorLoader::Image{bitmap,
                                           gfx::Point(image.xhot, image.yhot),
                                           base::Milliseconds(image.delay)});

ui/base/x/x11_cursor_loader_unittest.cc

@@ -13,12 +13,13 @@
 namespace ui {
 
 namespace {
-std::vector<XCursorLoader::Image> ParseFile(std::vector<uint32_t>* data,
+std::vector<XCursorLoader::Image> ParseFile(base::span<const uint32_t> data,
                                             uint32_t preferred_size) {
-  for (uint32_t& i : *data)
-    i = base::ByteSwapToLE32(i);
-  std::vector<uint8_t> vec(data->size() * sizeof(uint32_t));
-  memcpy(vec.data(), data->data(), vec.size());
+  std::vector<uint8_t> vec(data.size() * 4u);
+  for (size_t i = 0; i < data.size(); ++i) {
+    auto bytes = base::span(vec).subspan(i * 4u).first<4u>();
+    bytes.copy_from(base::numerics::U32ToLittleEndian(data[i]));
+  }
   return ParseCursorFile(base::RefCountedBytes::TakeVector(&vec),
                          preferred_size);
 }
@@ -64,7 +65,7 @@ TEST(XCursorLoaderTest, Basic) {
       // chunk data (ARGB image)
       0xff123456,
   };
-  auto images = ParseFile(&file, 1);
+  auto images = ParseFile(file, 1);
   ASSERT_EQ(images.size(), 1ul);
   EXPECT_EQ(images[0].frame_delay.InMilliseconds(), 123);
   EXPECT_EQ(images[0].bitmap.width(), 1);
@@ -180,7 +181,7 @@ TEST(XCursorLoaderTest, BestSize) {
       0xffffffff,
       0xffffffff,
   };
-  auto images = ParseFile(&file, 2);
+  auto images = ParseFile(file, 2);
   ASSERT_EQ(images.size(), 1ul);
   EXPECT_EQ(images[0].bitmap.width(), 2);
   EXPECT_EQ(images[0].bitmap.height(), 2);
@@ -253,7 +254,7 @@ TEST(XCursorLoaderTest, Animated) {
       // chunk data (ARGB image)
       0xff123456,
   };
-  auto images = ParseFile(&file, 1);
+  auto images = ParseFile(file, 1);
   ASSERT_EQ(images.size(), 2ul);
   EXPECT_EQ(images[0].frame_delay.InMilliseconds(), 500);
   EXPECT_EQ(images[1].frame_delay.InMilliseconds(), 500);

ui/base/x/x11_pointer_grab.cc

@@ -46,7 +46,7 @@ x11::GrabStatus GrabPointerImpl(x11::Window window,
                 x11::Input::XIEventMask::TouchBegin |
                 x11::Input::XIEventMask::TouchUpdate |
                 x11::Input::XIEventMask::TouchEnd;
-    static_assert(sizeof(mask) == 4, "");
+    static_assert(sizeof(mask) == 4);
 
     for (auto master_pointer :
          ui::DeviceDataManagerX11::GetInstance()->master_pointers()) {
@@ -59,7 +59,7 @@ x11::GrabStatus GrabPointerImpl(x11::Window window,
           .paired_device_mode = x11::GrabMode::Async,
           .owner_events = owner_events ? x11::Input::GrabOwner::Owner
                                        : x11::Input::GrabOwner::NoOwner,
-          .mask = {base::ByteSwapToLE32(static_cast<uint32_t>(mask))},
+          .mask = {static_cast<uint32_t>(mask)},
       };
       if (auto reply = connection->xinput().XIGrabDevice(req).Sync())
         result = reply->status;