bufferSubData causes crash in WebGLBuffer::associateBufferSubData

• Component: WebGL
• Status: RESOLVED FIXED

[Kenneth Russell]

If an element array buffer object is initialized via bufferData(ELEMENT_ARRAY_BUFFER, size, usage) and then filled with bufferSubData(ELEMENT_ARRAY_BUFFER, offset, ArrayBufferView), the index validation code (WebGLBuffer::associateBufferSubData) crashes because the m_elementArrayBuffer has not been allocated.

[Kenneth Russell]

Created attachment 61114 [details]
Patch

From the ChangeLog:

Allocate m_elementArrayBuffer for entry point taking only size. Guard against allocation failures of m_elementArrayBuffer. Guard against any possibility of crashes due to m_elementArrayBuffer being NULL.

[Nate Chapin]

Comment on attachment 61114 [details]
Patch

>      case GraphicsContext3D::ELEMENT_ARRAY_BUFFER:
>      case GraphicsContext3D::ARRAY_BUFFER:
>          m_byteLength = size;
> +        if (m_target == GraphicsContext3D::ELEMENT_ARRAY_BUFFER) {
> +            clearCachedMaxIndices();
> +            m_elementArrayBuffer = ArrayBuffer::create(size, 1);
> +            if (!m_elementArrayBuffer) {
> +                m_byteLength = 0;
> +                return false;
> +            }
> +        }
>          return true;
>      default:
>          return false;

Style nit: exit early if m_target is 0, and remove the switch.

[Kenneth Russell]

(In reply to comment #2)
> (From update of attachment 61114 [details])
> >      case GraphicsContext3D::ELEMENT_ARRAY_BUFFER:
> >      case GraphicsContext3D::ARRAY_BUFFER:
> >          m_byteLength = size;
> > +        if (m_target == GraphicsContext3D::ELEMENT_ARRAY_BUFFER) {
> > +            clearCachedMaxIndices();
> > +            m_elementArrayBuffer = ArrayBuffer::create(size, 1);
> > +            if (!m_elementArrayBuffer) {
> > +                m_byteLength = 0;
> > +                return false;
> > +            }
> > +        }
> >          return true;
> >      default:
> >          return false;
> 
> Style nit: exit early if m_target is 0, and remove the switch.

Will make this change in the landed version.

[Kenneth Russell]

Committed r63017: <http://trac.webkit.org/changeset/63017>