Uninitialized num_bytes in UnserializedMessageContext::UnserializedMessageContext |
||
Issue descriptionNoticed this while working on intercepting and dumping Mojo messages. It seems that in https://chromium.googlesource.com/chromium/src/+/e1ecbeb34a503d9369a02539af31a47110918341/mojo/public/cpp/bindings/lib/unserialized_message_context.cc, UnserializedMessageContext::UnserializedMessageContext does not initialize header_->num_bytes. When this is later used to create the mojo::Message in Message::Message(ScopedMessageHandle), the uninitialized header is copied (https://chromium.googlesource.com/chromium/src/+/5ccef67b14bf0132e879d4ea724494434a5f841a/mojo/public/cpp/bindings/lib/message.cc#244). This leads to bad values returned by payload() and payload_num_bytes().
,
May 29 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e8e1ad752e5922a61ce888e890b46e642aedc82d commit e8e1ad752e5922a61ce888e890b46e642aedc82d Author: Oliver Chang <ochang@chromium.org> Date: Tue May 29 21:15:24 2018 Initialize header_.num_bytes in UnserializedMessageContext. Bug: 847388 R=rockot@chromium.org Change-Id: I02a661071e41a7513b4e3aa9bc929e127c78c74f Reviewed-on: https://chromium-review.googlesource.com/1076109 Reviewed-by: Ken Rockot <rockot@chromium.org> Commit-Queue: Oliver Chang <ochang@chromium.org> Cr-Commit-Position: refs/heads/master@{#562591} [modify] https://crrev.com/e8e1ad752e5922a61ce888e890b46e642aedc82d/mojo/public/cpp/bindings/lib/unserialized_message_context.cc
,
May 29 2018
The uninitialized memory is fixed, but per rockot@ from the CL review: I'm not sure this is quite the right thing to do. I get not wanting uninitialized bytes to be copied, and this seems harmless otherwise, so LGTM. But nobody should call payload() on an unserialized Message. Maybe that should DCHECK. |
||
►
Sign in to add a comment |
||
Comment 1 by och...@chromium.org
, May 29 2018