Skip to content

Commit

Permalink
chore: cherry-pick 5 changes from Release-0-M116 (#39559)
Browse files Browse the repository at this point in the history
* chore: [22-x-y] cherry-pick 3 changes from Release-0-M116

* 8ff63d378f2c from v8
* 5486190be556 from angle
* d671b099a57d from v8

* chore: cherry-pick missing changes

* chore: update patches

---------

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
  • Loading branch information
ppontes and patchup[bot] committed Aug 21, 2023
1 parent 2dee878 commit 2598dc0
Show file tree
Hide file tree
Showing 10 changed files with 520 additions and 0 deletions.
1 change: 1 addition & 0 deletions patches/angle/.patches
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ cherry-pick-ce029c91a662.patch
cherry-pick-aed05b609629.patch
translator_limit_the_size_of_private_variables_in_webgl_shaders.patch
webgl_limit_total_size_of_private_data.patch
cherry-pick-5486190be556.patch
41 changes: 41 additions & 0 deletions patches/angle/cherry-pick-5486190be556.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Geoff Lang <geofflang@chromium.org>
Date: Fri, 21 Jul 2023 13:45:52 -0400
Subject: Fix read size validation for RGBX formats.

GL_RGBX8_ANGLE is the only format where the upload format is 3-channel
RGB, whilethe download format is 4-channel RGBX. As such, the internal
format corresponding to format+type expects 3-byte input/output. The
format is fixed here for readPixels to output 4 bytes per pixel.

Bug: chromium:1458046
Change-Id: Iec737ed64bade003cfab50dc5f595eb4875e81e4
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4706957
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
(cherry picked from commit 430a4f559cbc2bcd5d026e8b36ee46ddd80e9651)
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4765136
Commit-Queue: Daniel Yip <danielyip@google.com>
Auto-Submit: Daniel Yip <danielyip@google.com>
(cherry picked from commit 4a372ad49ceddea6c13f79adb212a777ec770a66)

diff --git a/src/libANGLE/formatutils.cpp b/src/libANGLE/formatutils.cpp
index 4014953311976b0f1ae2d1842e8fced75ffecfc9..6c8d3680b7a8bb78894120ca6cac7882827a98e4 100644
--- a/src/libANGLE/formatutils.cpp
+++ b/src/libANGLE/formatutils.cpp
@@ -1710,7 +1710,15 @@ const InternalFormat &GetInternalFormatInfo(GLenum internalFormat, GLenum type)
GLuint InternalFormat::computePixelBytes(GLenum formatType) const
{
const auto &typeInfo = GetTypeInfo(formatType);
- GLuint components = typeInfo.specialInterpretation ? 1u : componentCount;
+ GLuint components = componentCount;
+ if (sizedInternalFormat == GL_RGBX8_ANGLE)
+ {
+ components = 4;
+ }
+ else if (typeInfo.specialInterpretation)
+ {
+ components = 1;
+ }
return components * typeInfo.bytes;
}

1 change: 1 addition & 0 deletions patches/chromium/.patches
Original file line number Diff line number Diff line change
Expand Up @@ -169,3 +169,4 @@ m114_webcodecs_fix_crash_when_changing_temporal_layer_count_in_av1.patch
cherry-pick-933b9fad3a53.patch
cherry-pick-b03973561862.patch
cherry-pick-c60a1ab717c7.patch
networkcontext_don_t_access_url_loader_factories_during_destruction.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Adam Rice <ricea@chromium.org>
Date: Tue, 8 Aug 2023 08:48:51 +0000
Subject: NetworkContext: Don't access url_loader_factories_ during destruction

Move the contents of `url_loader_factories_` to a temporary variable in
the destructor of network::NetworkContext so that re-entrant calls to
DestroyURLLoaderFactory() don't happen after it has started being
destroyed.

BUG=1465833

(cherry picked from commit e579b20308290df03f045c5d0ccb852d96b24ce3)

Change-Id: I476f0865256bdcba4ec934688597e69991968f84
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4733351
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1177648}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4756334
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Adam Rice <ricea@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5845@{#1252}
Cr-Branched-From: 5a5dff63a4a4c63b9b18589819bebb2566c85443-refs/heads/main@{#1160321}

diff --git a/services/network/network_context.cc b/services/network/network_context.cc
index 6dc46954b070795de082d8542f8bae56d7b52dd4..2b12040f5ef2b7fd466a9b6d92e75faea8bd1808 100644
--- a/services/network/network_context.cc
+++ b/services/network/network_context.cc
@@ -712,6 +712,8 @@ NetworkContext::NetworkContext(
}

NetworkContext::~NetworkContext() {
+ is_destructing_ = true;
+
// May be nullptr in tests.
if (network_service_)
network_service_->DeregisterNetworkContext(this);
@@ -775,6 +777,12 @@ NetworkContext::~NetworkContext() {
}
}
#endif // BUILDFLAG(IS_DIRECTORY_TRANSFER_REQUIRED)
+
+ // Clear `url_loader_factories_` before deleting the contents, as it can
+ // result in re-entrant calls to DestroyURLLoaderFactory().
+ std::set<std::unique_ptr<cors::CorsURLLoaderFactory>,
+ base::UniquePtrComparator>
+ url_loader_factories = std::move(url_loader_factories_);
}

// static
@@ -993,6 +1001,9 @@ void NetworkContext::DisableQuic() {

void NetworkContext::DestroyURLLoaderFactory(
cors::CorsURLLoaderFactory* url_loader_factory) {
+ if (is_destructing_) {
+ return;
+ }
auto it = url_loader_factories_.find(url_loader_factory);
DCHECK(it != url_loader_factories_.end());
url_loader_factories_.erase(it);
diff --git a/services/network/network_context.h b/services/network/network_context.h
index 402a9de330140e78f14dbfae204f4c637803fcef..9e4db6927a42308b7ac30455f27ad97b92e2b0c3 100644
--- a/services/network/network_context.h
+++ b/services/network/network_context.h
@@ -915,6 +915,10 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
// DCHECKs on APIs used by external callers.
bool require_network_isolation_key_ = false;

+ // True once the destructor has been called. Used to guard against re-entrant
+ // calls to DestroyURLLoaderFactory().
+ bool is_destructing_ = false;
+
// Indicating whether
// https://fetch.spec.whatwg.org/#cors-non-wildcard-request-header-name is
// supported.
@@ -923,13 +927,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext

// CorsURLLoaderFactory assumes that fields owned by the NetworkContext always
// live longer than the factory. Therefore we want the factories to be
- // destroyed before other fields above. In particular:
- // - This must be below |url_request_context_| so that the URLRequestContext
- // outlives all the URLLoaderFactories and URLLoaders that depend on it;
- // for the same reason, it must also be below |network_context_|.
- // - This must be below |loader_count_per_process_| that is touched by
- // CorsURLLoaderFactory::DestroyURLLoader (see also
- // https://crbug.com/1174943).
+ // destroyed before other fields above. This is accomplished by explicitly
+ // clearing `url_loader_factories_` in the destructor.
std::set<std::unique_ptr<cors::CorsURLLoaderFactory>,
base::UniquePtrComparator>
url_loader_factories_;
1 change: 1 addition & 0 deletions patches/skia/.patches
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
enforce_program_stack_limits_on_function_parameters.patch
enforce_size_limits_on_struct_and_array_declarations.patch
enforce_an_upper_limit_of_715_million_path_verbs_in_skpath.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: John Stiles <johnstiles@google.com>
Date: Thu, 3 Aug 2023 13:33:52 -0400
Subject: Enforce an upper limit of 715 million path verbs in SkPath.

Bug: chromium:1464215
Change-Id: Iedb7d73fc80de5ffb881b664dd77314cc2c6b108
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/735316
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>

diff --git a/relnotes/path_715M.md b/relnotes/path_715M.md
new file mode 100644
index 0000000000000000000000000000000000000000..7be9a40f1fc5b4f6432c490725b05d536d497fb1
--- /dev/null
+++ b/relnotes/path_715M.md
@@ -0,0 +1 @@
+SkPath now enforces an upper limit of 715 million path verbs.
diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
index 7caac5ca6f05d89a2986c3eea432eedcd201203f..1a9279784b5b7bab3e0ca6dad9d1fd49a9038965 100644
--- a/src/core/SkPath.cpp
+++ b/src/core/SkPath.cpp
@@ -27,6 +27,7 @@
#include "src/pathops/SkPathOpsPoint.h"

#include <cmath>
+#include <limits.h>
#include <utility>

struct SkPath_Storage_Equivalent {
@@ -3401,43 +3402,52 @@ bool SkPath::IsCubicDegenerate(const SkPoint& p1, const SkPoint& p2,

SkPathVerbAnalysis sk_path_analyze_verbs(const uint8_t vbs[], int verbCount) {
SkPathVerbAnalysis info = {false, 0, 0, 0};
-
bool needMove = true;
bool invalid = false;
- for (int i = 0; i < verbCount; ++i) {
- switch ((SkPathVerb)vbs[i]) {
- case SkPathVerb::kMove:
- needMove = false;
- info.points += 1;
- break;
- case SkPathVerb::kLine:
- invalid |= needMove;
- info.segmentMask |= kLine_SkPathSegmentMask;
- info.points += 1;
- break;
- case SkPathVerb::kQuad:
- invalid |= needMove;
- info.segmentMask |= kQuad_SkPathSegmentMask;
- info.points += 2;
- break;
- case SkPathVerb::kConic:
- invalid |= needMove;
- info.segmentMask |= kConic_SkPathSegmentMask;
- info.points += 2;
- info.weights += 1;
- break;
- case SkPathVerb::kCubic:
- invalid |= needMove;
- info.segmentMask |= kCubic_SkPathSegmentMask;
- info.points += 3;
- break;
- case SkPathVerb::kClose:
- invalid |= needMove;
- needMove = true;
- break;
- default:
- invalid = true;
- break;
+
+ if (verbCount >= (INT_MAX / 3)) {
+ // A path with an extremely high number of quad, conic or cubic verbs could cause
+ // `info.points` to overflow. To prevent against this, we reject extremely large paths. This
+ // check is conservative and assumes the worst case (in particular, it assumes that every
+ // verb consumes 3 points, which would only happen for a path composed entirely of cubics).
+ // This limits us to 700 million verbs, which is large enough for any reasonable use case.
+ invalid = true;
+ } else {
+ for (int i = 0; i < verbCount; ++i) {
+ switch ((SkPathVerb)vbs[i]) {
+ case SkPathVerb::kMove:
+ needMove = false;
+ info.points += 1;
+ break;
+ case SkPathVerb::kLine:
+ invalid |= needMove;
+ info.segmentMask |= kLine_SkPathSegmentMask;
+ info.points += 1;
+ break;
+ case SkPathVerb::kQuad:
+ invalid |= needMove;
+ info.segmentMask |= kQuad_SkPathSegmentMask;
+ info.points += 2;
+ break;
+ case SkPathVerb::kConic:
+ invalid |= needMove;
+ info.segmentMask |= kConic_SkPathSegmentMask;
+ info.points += 2;
+ info.weights += 1;
+ break;
+ case SkPathVerb::kCubic:
+ invalid |= needMove;
+ info.segmentMask |= kCubic_SkPathSegmentMask;
+ info.points += 3;
+ break;
+ case SkPathVerb::kClose:
+ invalid |= needMove;
+ needMove = true;
+ break;
+ default:
+ invalid = true;
+ break;
+ }
}
}
info.valid = !invalid;
3 changes: 3 additions & 0 deletions patches/v8/.patches
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,6 @@ cherry-pick-2e76270cf65e.patch
utf-8_q_shared-struct_20fix_20using_20shared_20objects_20as.patch
merged_runtime_set_instance_prototypes_directly_on_maps.patch
merged_compiler_stackcheck_can_have_side_effects.patch
promise_allsettled_mark_values_array_as_cow.patch
merged_builtins_clear_fixedarray_slot_in_promise_builtins.patch
merged_compiler_check_for_read-only_property_on.patch

0 comments on commit 2598dc0

Please sign in to comment.