Pepper: Make sure 'out' params are initialized for all code paths.

Pepper: Make sure 'out' params are initialized for all code paths.
Change PPB_ImageData_Proxy to always initialize 'out' params, to avoid
sending uninitialized memory from renderer to plugin.

BUG=391023
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=282692

[bbudge, 2014-07-11 15:13:23]

This reverts my previous fix, which is on the wrong side of the proxy. The
problem is in the renderer, when sending the sync reply message. Tuples
containing the reply params are created in IPC::DispatchWithSendParams but not
completely initialized. The dispatch method must fill in these out params for
all code paths. This CL fixes that.

https://code.google.com/p/chromium/codesearch#chromium/src/ipc/ipc_message_ut...

[earthdok, 2014-07-11 15:19:34]

On 2014/07/11 15:13:23, bbudge (OOO July 1-8) wrote:
> This reverts my previous fix, which is on the wrong side of the proxy. The
> problem is in the renderer, when sending the sync reply message. Tuples
> containing the reply params are created in IPC::DispatchWithSendParams but not
> completely initialized. The dispatch method must fill in these out params for
> all code paths. This CL fixes that.
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/ipc/ipc_message_ut...

Confirmed that this gets rid of the MSan report.

[dmichael (off chromium), 2014-07-11 15:22:26]

lgtm

https://codereview.chromium.org/383143003/diff/20001/ppapi/proxy/ppb_image_da...
File ppapi/proxy/ppb_image_data_proxy.cc (right):

https://codereview.chromium.org/383143003/diff/20001/ppapi/proxy/ppb_image_da...
ppapi/proxy/ppb_image_data_proxy.cc:625: memset(desc, 0, sizeof(*desc));
*desc = PP_ImageDataDesc();
?

https://codereview.chromium.org/383143003/diff/20001/ppapi/proxy/ppb_instance...
File ppapi/proxy/ppb_instance_proxy.cc (right):

https://codereview.chromium.org/383143003/diff/20001/ppapi/proxy/ppb_instance...
ppapi/proxy/ppb_instance_proxy.cc:502: memset(components, 0,
sizeof(*components));
ditto

[bbudge, 2014-07-11 15:23:40]

https://codereview.chromium.org/383143003/diff/20001/ppapi/proxy/ppb_image_da...
File ppapi/proxy/ppb_image_data_proxy.cc (right):

https://codereview.chromium.org/383143003/diff/20001/ppapi/proxy/ppb_image_da...
ppapi/proxy/ppb_image_data_proxy.cc:625: memset(desc, 0, sizeof(*desc));
David, the reason I've gone back to memset is to ensure that padding bytes are
also cleared. In this case there are none, but this seems like a good pattern to
set.

[dmichael (off chromium), 2014-07-11 15:51:52]

lgtm

https://codereview.chromium.org/383143003/diff/20001/ppapi/proxy/ppb_image_da...
File ppapi/proxy/ppb_image_data_proxy.cc (right):

https://codereview.chromium.org/383143003/diff/20001/ppapi/proxy/ppb_image_da...
ppapi/proxy/ppb_image_data_proxy.cc:625: memset(desc, 0, sizeof(*desc));
On 2014/07/11 15:23:40, bbudge (OOO July 1-8) wrote:
> David, the reason I've gone back to memset is to ensure that padding bytes are
> also cleared. In this case there are none, but this seems like a good pattern
to
> set.
Okay, that makes sense. still lgtm

[bbudge, 2014-07-11 16:01:54]

The CQ bit was checked by bbudge@chromium.org

[commit-bot: I haz the power, 2014-07-11 16:02:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bbudge@chromium.org/383143003/20001

[bbudge, 2014-07-11 17:01:55]

https://codereview.chromium.org/383143003/diff/20001/ppapi/proxy/ppb_image_da...
File ppapi/proxy/ppb_image_data_proxy.cc (right):

https://codereview.chromium.org/383143003/diff/20001/ppapi/proxy/ppb_image_da...
ppapi/proxy/ppb_image_data_proxy.cc:625: memset(desc, 0, sizeof(*desc));
On 2014/07/11 15:51:52, dmichael wrote:
> On 2014/07/11 15:23:40, bbudge (OOO July 1-8) wrote:
> > David, the reason I've gone back to memset is to ensure that padding bytes
are
> > also cleared. In this case there are none, but this seems like a good
pattern
> to
> > set.
> Okay, that makes sense. still lgtm

Sorry for churn, switching back to *desc = PP_ImageDataDesc() for clarity. This
isn't the right place to try to fix information leaks through padding.

https://codereview.chromium.org/383143003/diff/20001/ppapi/proxy/ppb_instance...
File ppapi/proxy/ppb_instance_proxy.cc (right):

https://codereview.chromium.org/383143003/diff/20001/ppapi/proxy/ppb_instance...
ppapi/proxy/ppb_instance_proxy.cc:502: memset(components, 0,
sizeof(*components));
On 2014/07/11 15:22:26, dmichael wrote:
> ditto

Backed out my change here. |components| may be NULL. If not, code below makes
sure components isn't uninitialized.

[bbudge, 2014-07-11 17:02:42]

The CQ bit was checked by bbudge@chromium.org

[commit-bot: I haz the power, 2014-07-11 17:03:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/bbudge@chromium.org/383143003/40001

[commit-bot: I haz the power, 2014-07-11 21:00:28]

Change committed as 282692