Changeset 272678 in webkit


Ignore:
Timestamp:
Feb 10, 2021 12:34:18 PM (3 years ago)
Author:
Chris Dumez
Message:

WebCore::createBusFromInMemoryAudioFile() may crash under ExtAudioFileRead()
​https://bugs.webkit.org/show_bug.cgi?id=221642
<rdar://72789841>

Reviewed by Geoffrey Garen.

The crash seems to indicate we are passing an AudioBufferList to ExtAudioFileRead()
that contains a null buffer. It is not obvious how this is happening but I have made
the following changes:

  1. createAudioBufferList() / destroyAudioListBuffer() implementation is now shared on both macOS and iOS. The implementation now uses fastCalloc and returns null in case of failure to allocate.
  2. createAudioBufferList() was renamed to tryCreateAudioBufferList() to make it clear it can return null. All call sites now properly deal with tryCreateAudioBufferList() potentially return null
  3. Add a new validateAudioBufferList() function which makes sure that the AudioBufferList we are about to pass to ExtAudioFileRead() does not contain any null buffer. In case of validation failure, we log an error, generate a simulated crash log and early return gracefully instead of crashing later on.
  4. Added more assertions to help catch bugs.
  • SourcesCocoa.txt:
  • WebCore.xcodeproj/project.pbxproj:
  • platform/audio/cocoa/AudioFileReaderCocoa.cpp: Added.

(WebCore::tryCreateAudioBufferList):
(WebCore::destroyAudioBufferList):
(WebCore::validateAudioBufferList):

  • platform/audio/cocoa/AudioFileReaderCocoa.h: Added.
  • platform/audio/ios/AudioFileReaderIOS.cpp:

(WebCore::AudioFileReader::createBus):
(WebCore::createAudioBufferList): Deleted.
(WebCore::destroyAudioBufferList): Deleted.

  • platform/audio/mac/AudioFileReaderMac.cpp:

(WebCore::AudioFileReader::createBus):
(WebCore::createAudioBufferList): Deleted.
(WebCore::destroyAudioBufferList): Deleted.

Location:
trunk/Source/WebCore
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r272665 r272678  
     12021-02-10  Chris Dumez  <cdumez@apple.com>
     2
     3        WebCore::createBusFromInMemoryAudioFile() may crash under ExtAudioFileRead()
     4        https://bugs.webkit.org/show_bug.cgi?id=221642
     5        <rdar://72789841>
     6
     7        Reviewed by Geoffrey Garen.
     8
     9        The crash seems to indicate we are passing an AudioBufferList to ExtAudioFileRead()
     10        that contains a null buffer. It is not obvious how this is happening but I have made
     11        the following changes:
     12        1. createAudioBufferList() / destroyAudioListBuffer() implementation is now shared
     13           on both macOS and iOS. The implementation now uses fastCalloc and returns null
     14           in case of failure to allocate.
     15        2. createAudioBufferList() was renamed to tryCreateAudioBufferList() to make it clear
     16           it can return null. All call sites now properly deal with tryCreateAudioBufferList()
     17           potentially return null
     18        3. Add a new validateAudioBufferList() function which makes sure that the AudioBufferList
     19           we are about to pass to ExtAudioFileRead() does not contain any null buffer. In case
     20           of validation failure, we log an error, generate a simulated crash log and early return
     21           gracefully instead of crashing later on.
     22        4. Added more assertions to help catch bugs.
     23
     24        * SourcesCocoa.txt:
     25        * WebCore.xcodeproj/project.pbxproj:
     26        * platform/audio/cocoa/AudioFileReaderCocoa.cpp: Added.
     27        (WebCore::tryCreateAudioBufferList):
     28        (WebCore::destroyAudioBufferList):
     29        (WebCore::validateAudioBufferList):
     30        * platform/audio/cocoa/AudioFileReaderCocoa.h: Added.
     31        * platform/audio/ios/AudioFileReaderIOS.cpp:
     32        (WebCore::AudioFileReader::createBus):
     33        (WebCore::createAudioBufferList): Deleted.
     34        (WebCore::destroyAudioBufferList): Deleted.
     35        * platform/audio/mac/AudioFileReaderMac.cpp:
     36        (WebCore::AudioFileReader::createBus):
     37        (WebCore::createAudioBufferList): Deleted.
     38        (WebCore::destroyAudioBufferList): Deleted.
     39
    1402021-02-10  Aditya Keerthi  <akeerthi@apple.com>
    241

  • trunk/Source/WebCore/SourcesCocoa.txt

    r272603 r272678  
    206206platform/audio/AudioSession.cpp
    207207platform/audio/cocoa/AudioDestinationCocoa.cpp
     208platform/audio/cocoa/AudioFileReaderCocoa.cpp
    208209platform/audio/cocoa/AudioOutputUnitAdaptor.cpp
    209210platform/audio/cocoa/AudioSampleBufferList.cpp

  • trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj

    r272630 r272678  
    12621262                4682D2001F79783000C863DB /* StoredCredentialsPolicy.h in Headers */ = {isa = PBXBuildFile; fileRef = 4682D1FF1F79782300C863DB /* StoredCredentialsPolicy.h */; settings = {ATTRIBUTES = (Private, ); }; };
    12631263                468344E01EDDFAAA00B7795B /* DOMRectList.h in Headers */ = {isa = PBXBuildFile; fileRef = 468344DE1EDDFA5F00B7795B /* DOMRectList.h */; settings = {ATTRIBUTES = (Private, ); }; };
     1264                46AAAA3D25D3632000BAF42F /* AudioFileReaderCocoa.h in Headers */ = {isa = PBXBuildFile; fileRef = 46AAAA3A25D3631400BAF42F /* AudioFileReaderCocoa.h */; };
    12641265                46B63F6C1C6E8D19002E914B /* JSEventTargetCustom.h in Headers */ = {isa = PBXBuildFile; fileRef = 46B63F6B1C6E8CDF002E914B /* JSEventTargetCustom.h */; settings = {ATTRIBUTES = (Private, ); }; };
    12651266                46B650DD2296262700FD8AA4 /* PageIdentifier.h in Headers */ = {isa = PBXBuildFile; fileRef = 46B650DB2296262700FD8AA4 /* PageIdentifier.h */; settings = {ATTRIBUTES = (Private, ); }; };
    … …  
    81978198                468344DE1EDDFA5F00B7795B /* DOMRectList.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = DOMRectList.h; sourceTree = "<group>"; };
    81988199                468B8BDE25CC849300F67822 /* JSBaseAudioContextCustom.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = JSBaseAudioContextCustom.cpp; sourceTree = "<group>"; };
     8200                46AAAA3A25D3631400BAF42F /* AudioFileReaderCocoa.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = AudioFileReaderCocoa.h; sourceTree = "<group>"; };
     8201                46AAAA3C25D3631400BAF42F /* AudioFileReaderCocoa.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = AudioFileReaderCocoa.cpp; sourceTree = "<group>"; };
    81998202                46B63F6B1C6E8CDF002E914B /* JSEventTargetCustom.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSEventTargetCustom.h; sourceTree = "<group>"; };
    82008203                46B650DB2296262700FD8AA4 /* PageIdentifier.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PageIdentifier.h; sourceTree = "<group>"; };
    … …  
    2793927942                                413151842357745E00115E6E /* AudioDestinationCocoa.cpp */,
    2794027943                                413151862357745E00115E6E /* AudioDestinationCocoa.h */,
     27944                                46AAAA3C25D3631400BAF42F /* AudioFileReaderCocoa.cpp */,
     27945                                46AAAA3A25D3631400BAF42F /* AudioFileReaderCocoa.h */,
    2794127946                                1DB66D38253678EA00B671B9 /* AudioOutputUnitAdaptor.cpp */,
    2794227947                                1DB66D37253678EA00B671B9 /* AudioOutputUnitAdaptor.h */,
    … …  
    3123731242                                FD31608212B026F700C1A359 /* AudioDSPKernelProcessor.h in Headers */,
    3123831243                                FD31608312B026F700C1A359 /* AudioFileReader.h in Headers */,
     31244                                46AAAA3D25D3632000BAF42F /* AudioFileReaderCocoa.h in Headers */,
    3123931245                                CD5596921475B678001D0BD0 /* AudioFileReaderIOS.h in Headers */,
    3124031246                                FD3160BF12B0272A00C1A359 /* AudioFileReaderMac.h in Headers */,

  • trunk/Source/WebCore/platform/audio/ios/AudioFileReaderIOS.cpp

    r268414 r272678  
    3535#include "AudioBus.h"
    3636#include "AudioFileReader.h"
     37#include "AudioFileReaderCocoa.h"
     38#include "Logging.h"
    3739#include <CoreFoundation/CoreFoundation.h>
    3840#include <wtf/CheckedArithmetic.h>
    … …  
    5254namespace WebCore {
    5355
    54 static WARN_UNUSED_RETURN AudioBufferList* createAudioBufferList(size_t numberOfBuffers)
    55 {
    56     CheckedSize bufferListSize = sizeof(AudioBufferList) - sizeof(AudioBuffer);
    57     bufferListSize += numberOfBuffers * sizeof(AudioBuffer);
    58 
    59     AudioBufferList* bufferList = static_cast<AudioBufferList*>(calloc(1, bufferListSize.unsafeGet()));
    60     if (bufferList)
    61         bufferList->mNumberBuffers = numberOfBuffers;
    62     return bufferList;
    63 }
    64 
    65 static inline void destroyAudioBufferList(AudioBufferList* bufferList)
    66 {
    67     free(bufferList);
    68 }
    69 
    7056AudioFileReader::AudioFileReader(const char* filePath)
    7157    : m_data(0)
    … …  
    185171    AudioFloatArray rightChannel;
    186172
    187     AudioBufferList* bufferList = createAudioBufferList(numberOfChannels);
    188     if (!bufferList)
    189         return nullptr;
     173    AudioBufferList* bufferList = tryCreateAudioBufferList(numberOfChannels);
     174    if (!bufferList) {
     175        RELEASE_LOG_FAULT(WebAudio, "tryCreateAudioBufferList(%ld) returned null", numberOfChannels);
     176        return nullptr;
     177    }
    190178    const size_t bufferSize = numberOfFrames * sizeof(float);
    191179
     180    RELEASE_ASSERT(bufferList->mNumberBuffers == numberOfChannels);
    192181    if (mixToMono && numberOfChannels == 2) {
    193182        leftChannel.resize(numberOfFrames);
    … …  
    202191        bufferList->mBuffers[1].mData = rightChannel.data();
    203192    } else {
    204         ASSERT(!mixToMono || numberOfChannels == 1);
     193        RELEASE_ASSERT(!mixToMono || numberOfChannels == 1);
    205194
    206195        // For True-stereo (numberOfChannels == 4)
    … …  
    210199            bufferList->mBuffers[i].mDataByteSize = bufferSize;
    211200            bufferList->mBuffers[i].mData = audioBus->channel(i)->mutableData();
     201            ASSERT(bufferList->mBuffers[i].mData);
    212202        }
     203    }
     204
     205    if (!validateAudioBufferList(bufferList)) {
     206        RELEASE_LOG_FAULT(WebAudio, "Generated buffer in AudioFileReader::createBus() did not pass validation");
     207        ASSERT_NOT_REACHED();
     208        destroyAudioBufferList(bufferList);
     209        return nullptr;
    213210    }
    214211

  • trunk/Source/WebCore/platform/audio/mac/AudioFileReaderMac.cpp

    r268414 r272678  
    3737#include "AudioBus.h"
    3838#include "AudioFileReader.h"
     39#include "AudioFileReaderCocoa.h"
    3940#include "FloatConversion.h"
     41#include "Logging.h"
    4042#include <CoreFoundation/CoreFoundation.h>
    4143#include <wtf/RetainPtr.h>
    4244
    4345namespace WebCore {
    44 
    45 static AudioBufferList* createAudioBufferList(size_t numberOfBuffers)
    46 {
    47     size_t bufferListSize = sizeof(AudioBufferList) - sizeof(AudioBuffer);
    48     bufferListSize += numberOfBuffers * sizeof(AudioBuffer);
    49 
    50     AudioBufferList* bufferList = static_cast<AudioBufferList*>(calloc(1, bufferListSize));
    51     if (bufferList)
    52         bufferList->mNumberBuffers = numberOfBuffers;
    53 
    54     return bufferList;
    55 }
    56 
    57 static void destroyAudioBufferList(AudioBufferList* bufferList)
    58 {
    59     free(bufferList);
    60 }
    6146
    6247AudioFileReader::AudioFileReader(const char* filePath)
    … …  
    191176   
    192177    // Setup AudioBufferList in preparation for reading
    193     AudioBufferList* bufferList = createAudioBufferList(numberOfChannels);
    194 
     178    AudioBufferList* bufferList = tryCreateAudioBufferList(numberOfChannels);
     179    if (!bufferList) {
     180        RELEASE_LOG_FAULT(WebAudio, "tryCreateAudioBufferList(%ld) returned null", numberOfChannels);
     181        return nullptr;
     182    }
     183
     184    RELEASE_ASSERT(bufferList->mNumberBuffers == numberOfChannels);
    195185    if (mixToMono && numberOfChannels == 2) {
    196186        bufL.resize(numberOfFrames);
    … …  
    207197        bufferList->mBuffers[1].mData = bufferR;
    208198    } else {
    209         ASSERT(!mixToMono || numberOfChannels == 1);
     199        RELEASE_ASSERT(!mixToMono || numberOfChannels == 1);
    210200
    211201        // for True-stereo (numberOfChannels == 4)
    … …  
    214204            bufferList->mBuffers[i].mDataByteSize = numberOfFrames * sizeof(float);
    215205            bufferList->mBuffers[i].mData = audioBus->channel(i)->mutableData();
     206            ASSERT(bufferList->mBuffers[i].mData);
    216207        }
     208    }
     209
     210    if (!validateAudioBufferList(bufferList)) {
     211        RELEASE_LOG_FAULT(WebAudio, "Generated buffer in AudioFileReader::createBus() did not pass validation");
     212        ASSERT_NOT_REACHED();
     213        destroyAudioBufferList(bufferList);
     214        return nullptr;
    217215    }
    218216
Note: See TracChangeset for help on using the changeset viewer.