Bug 220710 - Validate ItemHandles when decoding them in GPUProcess
Summary: Validate ItemHandles when decoding them in GPUProcess
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks: 219097
  Show dependency treegraph
 
Reported: 2021-01-18 07:10 PST by youenn fablet
Modified: 2021-01-22 01:34 PST (History)
5 users (show)

See Also:


Attachments
Patch (22.53 KB, patch)
2021-01-18 08:10 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (22.60 KB, patch)
2021-01-18 09:22 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (22.55 KB, patch)
2021-01-19 02:14 PST, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (22.53 KB, patch)
2021-01-22 01:05 PST, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2021-01-18 07:10:29 PST
Validate ItemHandles when decoding them in GPUProcess
Comment 1 youenn fablet 2021-01-18 08:10:13 PST
Created attachment 417832 [details]
Patch
Comment 2 youenn fablet 2021-01-18 08:10:41 PST
<rdar://problem/72931549>
Comment 3 youenn fablet 2021-01-18 09:22:22 PST
Created attachment 417837 [details]
Patch
Comment 4 youenn fablet 2021-01-19 02:14:01 PST
Created attachment 417865 [details]
Patch
Comment 5 Wenson Hsieh 2021-01-21 08:39:25 PST
Comment on attachment 417865 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=417865&action=review

> Source/WebCore/platform/graphics/displaylists/DisplayListItemBuffer.cpp:543
> +template<typename, typename = void> inline constexpr bool HasIsValid = false;
> +template<typename T> inline constexpr bool HasIsValid<T, std::void_t<decltype(std::declval<T>().isValid())>> = true;

This is a really neat trick!

> Source/WebCore/platform/graphics/displaylists/DisplayListItemBuffer.cpp:559
> +bool ItemHandle::decodeInto(ItemHandle destination) const

Nit - I think the notion of "copying" is more accurate here than "decoding" (the latter of which sounds like it would involve marshaling of data to and from buffers, à la IPC encoding/decoding). Perhaps "createValidCopy" or "copyWithValidation"?
Comment 6 youenn fablet 2021-01-22 01:05:29 PST
Created attachment 418117 [details]
Patch
Comment 7 youenn fablet 2021-01-22 01:06:03 PST
Thanks for the review.

> > Source/WebCore/platform/graphics/displaylists/DisplayListItemBuffer.cpp:559
> > +bool ItemHandle::decodeInto(ItemHandle destination) const
> 
> Nit - I think the notion of "copying" is more accurate here than "decoding"
> (the latter of which sounds like it would involve marshaling of data to and
> from buffers, à la IPC encoding/decoding). Perhaps "createValidCopy" or
> "copyWithValidation"?

I changed to safeCopy
Comment 8 EWS 2021-01-22 01:34:16 PST
Committed r271741: <https://trac.webkit.org/changeset/271741>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 418117 [details].