Browse Source

chore: cherry-pick 4 changes from Release-2-M121 and Release-3-M121 (#41375)

* chore: cherry-pick 4 changes from Release-2-M121 and Release-3-M121

* chore: update patches

---------

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Pedro Pontes 1 year ago
parent
commit
426cd1d68c

+ 3 - 0
patches/chromium/.patches

@@ -154,3 +154,6 @@ cherry-pick-91a02d67a83d.patch
 cherry-pick-8755f76bec32.patch
 cherry-pick-1f8bec968902.patch
 cherry-pick-4a98f9e304be.patch
+fix_racy_iterator_use_in_node_addconnection.patch
+fix_a_crash_when_a_bmp_image_contains_an_unnecessary_eof_code.patch
+m120_ipcz_fix_a_few_weak_asserts.patch

+ 106 - 0
patches/chromium/fix_a_crash_when_a_bmp_image_contains_an_unnecessary_eof_code.patch

@@ -0,0 +1,106 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: John Stiles <[email protected]>
+Date: Thu, 1 Feb 2024 20:40:55 +0000
+Subject: Fix a crash when a BMP image contains an unnecessary EOF code.
+
+Previously, this would try to perform color correction on a row
+one past the end of the image data.
+
+(cherry picked from commit 4bdd8d61bebbba9fab77fa86a8f66b305995199b)
+
+Bug: 1521893
+Change-Id: I425437005b9ef400138556705616095857d2cf0d
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5241305
+Auto-Submit: John Stiles <[email protected]>
+Commit-Queue: John Stiles <[email protected]>
+Reviewed-by: Peter Kasting <[email protected]>
+Cr-Original-Commit-Position: refs/heads/main@{#1253633}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5259699
+Commit-Queue: Peter Kasting <[email protected]>
+Cr-Commit-Position: refs/branch-heads/6099@{#1915}
+Cr-Branched-From: e6ee4500f7d6549a9ac1354f8d056da49ef406be-refs/heads/main@{#1217362}
+
+diff --git a/third_party/blink/renderer/platform/blink_platform_unittests_bundle_data.filelist b/third_party/blink/renderer/platform/blink_platform_unittests_bundle_data.filelist
+index 42d0414da143f509a89551c8b09762949f8011c2..49e238e38fef374c698fcdc1d4b50d4ed7670c3d 100644
+--- a/third_party/blink/renderer/platform/blink_platform_unittests_bundle_data.filelist
++++ b/third_party/blink/renderer/platform/blink_platform_unittests_bundle_data.filelist
+@@ -403,6 +403,7 @@
+ ../../web_tests/images/resources/truncated.webp
+ ../../web_tests/images/resources/truncated2.webp
+ ../../web_tests/images/resources/twitter_favicon.ico
++../../web_tests/images/resources/unnecessary-eof.bmp
+ ../../web_tests/images/resources/webp-animated-icc-xmp.webp
+ ../../web_tests/images/resources/webp-animated-large.webp
+ ../../web_tests/images/resources/webp-animated-no-blend.webp
+diff --git a/third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_decoder_test.cc b/third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_decoder_test.cc
+index 85e3591ac707297944b81cc4673b75d308417bd1..6c867a76a44c57ae818e7a00df30d046077b4d4c 100644
+--- a/third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_decoder_test.cc
++++ b/third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_decoder_test.cc
+@@ -98,6 +98,19 @@ TEST(BMPImageDecoderTest, crbug752898) {
+   decoder->DecodeFrameBufferAtIndex(0);
+ }
+ 
++// Verify that decoding an image with an unnecessary EOF marker does not crash.
++TEST(BMPImageDecoderTest, allowEOFWhenPastEndOfImage) {
++  static constexpr char kBmpFile[] = "/images/resources/unnecessary-eof.bmp";
++  scoped_refptr<SharedBuffer> data = ReadFile(kBmpFile);
++  ASSERT_TRUE(data.get());
++
++  std::unique_ptr<ImageDecoder> decoder = CreateBMPDecoder();
++  decoder->SetData(data.get(), true);
++  ImageFrame* frame = decoder->DecodeFrameBufferAtIndex(0);
++  EXPECT_EQ(ImageFrame::kFrameComplete, frame->GetStatus());
++  EXPECT_FALSE(decoder->Failed());
++}
++
+ class BMPImageDecoderCorpusTest : public ImageDecoderBaseTest {
+  public:
+   BMPImageDecoderCorpusTest() : ImageDecoderBaseTest("bmp") {}
+diff --git a/third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_reader.cc b/third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_reader.cc
+index 6b6bcb2e8fce05062e80b84283fd635f9d4bf008..b7bbbf51a1b419d1641db5e2f062cfefb2eae554 100644
+--- a/third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_reader.cc
++++ b/third_party/blink/renderer/platform/image-decoders/bmp/bmp_image_reader.cc
+@@ -886,7 +886,8 @@ BMPImageReader::ProcessingResult BMPImageReader::ProcessRLEData() {
+     // the image.
+     const uint8_t count = ReadUint8(0);
+     const uint8_t code = ReadUint8(1);
+-    if ((count || (code != 1)) && PastEndOfImage(0)) {
++    const bool is_past_end_of_image = PastEndOfImage(0);
++    if ((count || (code != 1)) && is_past_end_of_image) {
+       return kFailure;
+     }
+ 
+@@ -911,7 +912,9 @@ BMPImageReader::ProcessingResult BMPImageReader::ProcessRLEData() {
+                             : (coord_.y() > 0))) {
+             buffer_->SetHasAlpha(true);
+           }
+-          ColorCorrectCurrentRow();
++          if (!is_past_end_of_image) {
++            ColorCorrectCurrentRow();
++          }
+           // There's no need to move |coord_| here to trigger the caller
+           // to call SetPixelsChanged().  If the only thing that's changed
+           // is the alpha state, that will be properly written into the
+@@ -1136,6 +1139,13 @@ void BMPImageReader::ColorCorrectCurrentRow() {
+   if (!transform) {
+     return;
+   }
++  int decoder_width = parent_->Size().width();
++  // Enforce 0 ≤ current row < bitmap height.
++  CHECK_GE(coord_.y(), 0);
++  CHECK_LT(coord_.y(), buffer_->Bitmap().height());
++  // Enforce decoder width == bitmap width exactly. (The bitmap rowbytes might
++  // add a bit of padding, but we are only converting one row at a time.)
++  CHECK_EQ(decoder_width, buffer_->Bitmap().width());
+   ImageFrame::PixelData* const row = buffer_->GetAddr(0, coord_.y());
+   const skcms_PixelFormat fmt = XformColorFormat();
+   const skcms_AlphaFormat alpha =
+@@ -1144,7 +1154,7 @@ void BMPImageReader::ColorCorrectCurrentRow() {
+           : skcms_AlphaFormat_Unpremul;
+   const bool success =
+       skcms_Transform(row, fmt, alpha, transform->SrcProfile(), row, fmt, alpha,
+-                      transform->DstProfile(), parent_->Size().width());
++                      transform->DstProfile(), decoder_width);
+   DCHECK(success);
+   buffer_->SetPixelsChanged(true);
+ }

+ 46 - 0
patches/chromium/fix_racy_iterator_use_in_node_addconnection.patch

@@ -0,0 +1,46 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Peter=20Bostr=C3=B6m?= <[email protected]>
+Date: Thu, 1 Feb 2024 21:24:43 +0000
+Subject: Fix racy iterator use in Node::AddConnection
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Before this fix an iterator to `connections_` which requires a lock
+would be dereferenced outside an unlock operation because the `it` taken
+from the map isn't understood as guarded by the same lock.
+
+This takes a Ref<NodeLink> before unlocking which'll keep the link
+reference alive even if `connections_` is concurrently modified and the
+entry removed (or replaced).
+
+(cherry picked from commit 1f2cbf5833d7f00d3fcbfd1f3ef0c1aff10c04cd)
+
+Bug: 1523704
+Change-Id: I6f6fe4e34ec2c8268d4e7f33965a13e3b10f9f92
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5254025
+Commit-Queue: Peter Boström <[email protected]>
+Reviewed-by: Ken Rockot <[email protected]>
+Commit-Queue: Ken Rockot <[email protected]>
+Auto-Submit: Peter Boström <[email protected]>
+Cr-Original-Commit-Position: refs/heads/main@{#1254709}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5260163
+Cr-Commit-Position: refs/branch-heads/6099@{#1916}
+Cr-Branched-From: e6ee4500f7d6549a9ac1354f8d056da49ef406be-refs/heads/main@{#1217362}
+
+diff --git a/third_party/ipcz/src/ipcz/node.cc b/third_party/ipcz/src/ipcz/node.cc
+index a7162f9017d86d3ab3369d04add98527189dd33f..3a8eef652cc3381b608d89cfb6acd006d4ba378b 100644
+--- a/third_party/ipcz/src/ipcz/node.cc
++++ b/third_party/ipcz/src/ipcz/node.cc
+@@ -168,9 +168,10 @@ bool Node::AddConnection(const NodeName& remote_node_name,
+       // handling an incoming NodeConnector message, we can err on the side of
+       // caution (i.e. less re-entrancy in event handlers) by treating every
+       // case like an API call.
++      const Ref<NodeLink> link = it->second.link;
+       mutex_.Unlock();
+       const OperationContext context{OperationContext::kAPICall};
+-      DropConnection(context, *it->second.link);
++      DropConnection(context, *link);
+       mutex_.Lock();
+     }
+ 

+ 87 - 0
patches/chromium/m120_ipcz_fix_a_few_weak_asserts.patch

@@ -0,0 +1,87 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Ken Rockot <[email protected]>
+Date: Tue, 30 Jan 2024 21:36:01 +0000
+Subject: ipcz: Fix a few weak asserts
+
+DriverMemory cloning should not weakly assert success, as it can fail in
+real production scenarios. Now Clone() will return an invalid
+DriverMemory object if it fails to duplicate the internal handle.
+Existing callers of Clone() are already durable to an invalid output, so
+this change results in graceful failures instead of undefined behavior.
+
+This also replaces some weak asserts in DriverTransport creation with
+hardening asserts. We may want to fail more gracefully if these end
+up crashing a lot, but it seems unlikely.
+
+(cherry picked from commit 4bd18c5a3a7a935716bbed197fba6d45a1122894)
+
+Fixed: 1521571
+Change-Id: Id764b33ead8bbba58e61b3270920c839479eaa4a
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5240312
+Commit-Queue: Ken Rockot <[email protected]>
+Reviewed-by: Alex Gough <[email protected]>
+Cr-Original-Commit-Position: refs/heads/main@{#1252882}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5250958
+Bot-Commit: Rubber Stamper <[email protected]>
+Auto-Submit: Ken Rockot <[email protected]>
+Cr-Commit-Position: refs/branch-heads/6099@{#1905}
+Cr-Branched-From: e6ee4500f7d6549a9ac1354f8d056da49ef406be-refs/heads/main@{#1217362}
+
+diff --git a/third_party/ipcz/src/ipcz/driver_memory.cc b/third_party/ipcz/src/ipcz/driver_memory.cc
+index f8761985b78409fdb5420456661b0d227030cc8f..3bdc3aaf52d166a7691b5f28ebc86cc47600f7cc 100644
+--- a/third_party/ipcz/src/ipcz/driver_memory.cc
++++ b/third_party/ipcz/src/ipcz/driver_memory.cc
+@@ -30,10 +30,11 @@ DriverMemory::DriverMemory(const IpczDriver& driver, size_t num_bytes)
+     : size_(num_bytes) {
+   ABSL_ASSERT(num_bytes > 0);
+   IpczDriverHandle handle;
+-  IpczResult result =
++  const IpczResult result =
+       driver.AllocateSharedMemory(num_bytes, IPCZ_NO_FLAGS, nullptr, &handle);
+-  ABSL_ASSERT(result == IPCZ_RESULT_OK);
+-  memory_ = DriverObject(driver, handle);
++  if (result == IPCZ_RESULT_OK) {
++    memory_ = DriverObject(driver, handle);
++  }
+ }
+ 
+ DriverMemory::DriverMemory(DriverMemory&& other) = default;
+@@ -43,12 +44,14 @@ DriverMemory& DriverMemory::operator=(DriverMemory&& other) = default;
+ DriverMemory::~DriverMemory() = default;
+ 
+ DriverMemory DriverMemory::Clone() {
+-  ABSL_ASSERT(is_valid());
++  ABSL_HARDENING_ASSERT(is_valid());
+ 
+   IpczDriverHandle handle;
+-  IpczResult result = memory_.driver()->DuplicateSharedMemory(
++  const IpczResult result = memory_.driver()->DuplicateSharedMemory(
+       memory_.handle(), 0, nullptr, &handle);
+-  ABSL_ASSERT(result == IPCZ_RESULT_OK);
++  if (result != IPCZ_RESULT_OK) {
++    return DriverMemory();
++  }
+ 
+   return DriverMemory(DriverObject(*memory_.driver(), handle));
+ }
+diff --git a/third_party/ipcz/src/ipcz/driver_transport.cc b/third_party/ipcz/src/ipcz/driver_transport.cc
+index 096f1b3bed3cfbe0074b074edba21bcfceacd897..dbeb69a0a881a82c9360118a017942ec6eb920f8 100644
+--- a/third_party/ipcz/src/ipcz/driver_transport.cc
++++ b/third_party/ipcz/src/ipcz/driver_transport.cc
+@@ -68,14 +68,14 @@ DriverTransport::Pair DriverTransport::CreatePair(
+   IpczDriverHandle target_transport0 = IPCZ_INVALID_DRIVER_HANDLE;
+   IpczDriverHandle target_transport1 = IPCZ_INVALID_DRIVER_HANDLE;
+   if (transport0) {
+-    ABSL_ASSERT(transport1);
++    ABSL_HARDENING_ASSERT(transport1);
+     target_transport0 = transport0->driver_object().handle();
+     target_transport1 = transport1->driver_object().handle();
+   }
+   IpczResult result = driver.CreateTransports(
+       target_transport0, target_transport1, IPCZ_NO_FLAGS, nullptr,
+       &new_transport0, &new_transport1);
+-  ABSL_ASSERT(result == IPCZ_RESULT_OK);
++  ABSL_HARDENING_ASSERT(result == IPCZ_RESULT_OK);
+   auto first =
+       MakeRefCounted<DriverTransport>(DriverObject(driver, new_transport0));
+   auto second =

+ 1 - 0
patches/webrtc/.patches

@@ -2,3 +2,4 @@ fix_fallback_to_x11_capturer_on_wayland.patch
 fix_mark_pipewire_capturer_as_failed_after_session_is_closed.patch
 fix_check_pipewire_init_before_creating_generic_capturer.patch
 pipewire_capturer_make_restore_tokens_re-usable_more_than_one_time.patch
+tighten_som_dchecks_to_checks_in_vp9_packetization.patch

+ 103 - 0
patches/webrtc/tighten_som_dchecks_to_checks_in_vp9_packetization.patch

@@ -0,0 +1,103 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= <[email protected]>
+Date: Fri, 19 Jan 2024 16:59:01 +0100
+Subject: Tighten som DCHECKs to CHECKs in VP9 packetization.
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+(cherry picked from commit 6a992129fb0dede4a8fbdaf5de43abaf43c20299)
+
+No-Try: True
+Bug: chromium:1518991, chromium:1518994
+Change-Id: I47f68ba6aaf4874fd952332bf213e3a1e0389268
+Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/335241
+Auto-Submit: Erik Språng <[email protected]>
+Reviewed-by: Danil Chapovalov <[email protected]>
+Commit-Queue: Danil Chapovalov <[email protected]>
+Cr-Original-Commit-Position: refs/heads/main@{#41580}
+Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/338640
+Reviewed-by: Mirko Bonadei <[email protected]>
+Commit-Queue: Erik Språng <[email protected]>
+Cr-Commit-Position: refs/branch-heads/6167@{#6}
+Cr-Branched-From: ece5cb83715dea85617114b6d4e981fdee2623ba-refs/heads/main@{#41315}
+
+diff --git a/modules/rtp_rtcp/source/rtp_format_vp9.cc b/modules/rtp_rtcp/source/rtp_format_vp9.cc
+index 15e059e85c8968c8ed72efa6b17ac998b5597f45..9ad4aa97c34aabe761739045662adc6374e3dc69 100644
+--- a/modules/rtp_rtcp/source/rtp_format_vp9.cc
++++ b/modules/rtp_rtcp/source/rtp_format_vp9.cc
+@@ -94,8 +94,8 @@ size_t RefIndicesLength(const RTPVideoHeaderVP9& hdr) {
+   if (!hdr.inter_pic_predicted || !hdr.flexible_mode)
+     return 0;
+ 
+-  RTC_DCHECK_GT(hdr.num_ref_pics, 0U);
+-  RTC_DCHECK_LE(hdr.num_ref_pics, kMaxVp9RefPics);
++  RTC_CHECK_GT(hdr.num_ref_pics, 0U);
++  RTC_CHECK_LE(hdr.num_ref_pics, kMaxVp9RefPics);
+   return hdr.num_ref_pics;
+ }
+ 
+@@ -123,9 +123,9 @@ size_t SsDataLength(const RTPVideoHeaderVP9& hdr) {
+   if (!hdr.ss_data_available)
+     return 0;
+ 
+-  RTC_DCHECK_GT(hdr.num_spatial_layers, 0U);
+-  RTC_DCHECK_LE(hdr.num_spatial_layers, kMaxVp9NumberOfSpatialLayers);
+-  RTC_DCHECK_LE(hdr.gof.num_frames_in_gof, kMaxVp9FramesInGof);
++  RTC_CHECK_GT(hdr.num_spatial_layers, 0U);
++  RTC_CHECK_LE(hdr.num_spatial_layers, kMaxVp9NumberOfSpatialLayers);
++  RTC_CHECK_LE(hdr.gof.num_frames_in_gof, kMaxVp9FramesInGof);
+   size_t length = 1;  // V
+   if (hdr.spatial_layer_resolution_present) {
+     length += 4 * hdr.num_spatial_layers;  // Y
+@@ -136,7 +136,7 @@ size_t SsDataLength(const RTPVideoHeaderVP9& hdr) {
+   // N_G
+   length += hdr.gof.num_frames_in_gof;  // T, U, R
+   for (size_t i = 0; i < hdr.gof.num_frames_in_gof; ++i) {
+-    RTC_DCHECK_LE(hdr.gof.num_ref_pics[i], kMaxVp9RefPics);
++    RTC_CHECK_LE(hdr.gof.num_ref_pics[i], kMaxVp9RefPics);
+     length += hdr.gof.num_ref_pics[i];  // R times
+   }
+   return length;
+@@ -248,9 +248,9 @@ bool WriteRefIndices(const RTPVideoHeaderVP9& vp9,
+ //      +-+-+-+-+-+-+-+-+              -|           -|
+ //
+ bool WriteSsData(const RTPVideoHeaderVP9& vp9, rtc::BitBufferWriter* writer) {
+-  RTC_DCHECK_GT(vp9.num_spatial_layers, 0U);
+-  RTC_DCHECK_LE(vp9.num_spatial_layers, kMaxVp9NumberOfSpatialLayers);
+-  RTC_DCHECK_LE(vp9.gof.num_frames_in_gof, kMaxVp9FramesInGof);
++  RTC_CHECK_GT(vp9.num_spatial_layers, 0U);
++  RTC_CHECK_LE(vp9.num_spatial_layers, kMaxVp9NumberOfSpatialLayers);
++  RTC_CHECK_LE(vp9.gof.num_frames_in_gof, kMaxVp9FramesInGof);
+   bool g_bit = vp9.gof.num_frames_in_gof > 0;
+ 
+   RETURN_FALSE_ON_ERROR(writer->WriteBits(vp9.num_spatial_layers - 1, 3));
+@@ -288,6 +288,8 @@ bool WriteSsData(const RTPVideoHeaderVP9& vp9, rtc::BitBufferWriter* writer) {
+ // current API to invoke SVC is not flexible enough.
+ RTPVideoHeaderVP9 RemoveInactiveSpatialLayers(
+     const RTPVideoHeaderVP9& original_header) {
++  RTC_CHECK_LE(original_header.num_spatial_layers,
++               kMaxVp9NumberOfSpatialLayers);
+   RTPVideoHeaderVP9 hdr(original_header);
+   if (original_header.first_active_layer == 0)
+     return hdr;
+@@ -314,7 +316,7 @@ RtpPacketizerVp9::RtpPacketizerVp9(rtc::ArrayView<const uint8_t> payload,
+       header_size_(PayloadDescriptorLengthMinusSsData(hdr_)),
+       first_packet_extra_header_size_(SsDataLength(hdr_)),
+       remaining_payload_(payload) {
+-  RTC_DCHECK_EQ(hdr_.first_active_layer, 0);
++  RTC_CHECK_EQ(hdr_.first_active_layer, 0);
+ 
+   limits.max_payload_len -= header_size_;
+   limits.first_packet_reduction_len += first_packet_extra_header_size_;
+@@ -357,8 +359,8 @@ bool RtpPacketizerVp9::NextPacket(RtpPacketToSend* packet) {
+ 
+   // Ensure end_of_picture is always set on top spatial layer when it is not
+   // dropped.
+-  RTC_DCHECK(hdr_.spatial_idx < hdr_.num_spatial_layers - 1 ||
+-             hdr_.end_of_picture);
++  RTC_CHECK(hdr_.spatial_idx < hdr_.num_spatial_layers - 1 ||
++            hdr_.end_of_picture);
+ 
+   packet->SetMarker(layer_end && hdr_.end_of_picture);
+   return true;