Bug 219630 - [GTK] Add an overflow check to remove the gcc warning since r270506.
Summary: [GTK] Add an overflow check to remove the gcc warning since r270506.
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joonghun Park
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-12-08 00:43 PST by Joonghun Park
Modified: 2022-02-10 16:40 PST (History)
10 users (show)

See Also:


Attachments
Patch (1.50 KB, patch)
2020-12-08 00:46 PST, Joonghun Park
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joonghun Park 2020-12-08 00:43:43 PST
It seems that it needs to add an overflow check and ASSERT to remove the gcc warning below.

warning: ‘void* memcpy(void*, const void*, size_t)’ specified bound 18446744073709551614 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
Comment 1 Joonghun Park 2020-12-08 00:46:25 PST
Created attachment 415616 [details]
Patch
Comment 2 Darin Adler 2020-12-08 12:58:53 PST
Comment on attachment 415616 [details]
Patch

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

> Source/WebCore/Modules/mediastream/H264Utils.cpp:138
> +    if (spsPpsLength > std::numeric_limits<size_t>::max() - 2) {
> +        ASSERT_NOT_REACHED();
> +        return { };
> +    }

Vector::resize will call CRASH if the size is too large. This instead returns an empty buffer. I don’t think there’s a good reason to be inconsistent about this. So this should just be CRASH().

An alternate way to do this would be to use checked arithmetic. CheckedSize exists for cases like this. That fix would be below.

> Source/WebCore/Modules/mediastream/H264Utils.cpp:141
>      buffer.resize(spsPpsLength + 2);

Could do this instead:

    buffer.grow((CheckedSize(spsPpsLength) + 2).unsafeGet());

Using unsafeGet gives us the same crash behavior as Vector::resize. Using Vector::grow instead of Vector::resize is the best practice when starting with a new vector, skips unneeded logic for shrinking the buffer.

Doing it this way has the advantage of tying the check directly to the arithmetic instead of having to make sure the preflight matches the arithmetic below.
Comment 3 Joonghun Park 2021-09-11 00:25:04 PDT
This file and the statement caused the build warning no longer exist,
so the build warning was removed.