Bug 50048

Summary: Need to initialize destination variables before calling GL
Product: WebKit Reporter: Gregg Tavares <gman>
Component: WebGLAssignee: Nobody <webkit-unassigned>
Severity: Normal CC: commit-queue, kbr
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: OS X 10.5   
Description Flags
Patch none

Description Gregg Tavares 2010-11-24 18:14:33 PST
Need to initialize variables before calling GL because if GL has lost the context then those variables will remain uninitialized.

For example

  GLsize len;
  glGetProgramiv(service_id_, GL_INFO_LOG_LENGTH, &len);
  char* dst = new char[len];

In the case above, if GL has lost the const, len will be uninitialized and quite possibly try to allocate a large amount of memory which may easily cause a crash. The solution is to always initialized the variables.

  GLsize len = 0;  <-----
  glGetProgramiv(service_id_, GL_INFO_LOG_LENGTH, &len);
  char* dst = new char[len];
Comment 1 Gregg Tavares 2010-11-24 18:15:15 PST
I have a patch to fix the places in WebKit where this is an issue.
Comment 2 Kenneth Russell 2010-11-26 12:49:55 PST
It would be great if you could submit your patch. Run prepare-ChangeLog, edit the modified ChangeLogs appropriately, and webkit-patch upload to upload it. See http://webkit.org/coding/contributing.html .
Comment 3 Gregg Tavares 2010-11-29 15:57:19 PST
Created attachment 75075 [details]
Comment 4 Kenneth Russell 2010-11-29 17:11:20 PST
Comment on attachment 75075 [details]

The code changes look fine, and sorry for the nitpick, but could you please change your editor's settings to not modify existing whitespace, and regenerate the patch? I've been advised by other WebKit reviewers that whitespace changes should not be done in conjunction with code changes. It would be great if you would file a separate bug and submit a patch just for the whitespace cleanups.
Comment 5 Gregg Tavares 2010-12-02 18:20:45 PST
Created attachment 75447 [details]
Comment 6 David Levin 2010-12-02 20:01:03 PST
Comment on attachment 75447 [details]

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

> WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:846
> +    m_maxTextureSize = 0;

I'm surprised this isn't initialized in the constructor.
Comment 7 WebKit Commit Bot 2010-12-02 23:25:24 PST
Comment on attachment 75447 [details]

Clearing flags on attachment: 75447

Committed r73244: <http://trac.webkit.org/changeset/73244>
Comment 8 WebKit Commit Bot 2010-12-02 23:25:29 PST
All reviewed patches have been landed.  Closing bug.