[mojo-edk] Sanity checks on ports message parsing

[mojo-edk] Sanity checks on ports message parsing

BUG=612364
R=ochang@chromium.org

Committed: https://crrev.com/73af3951c285542c5397293375e0c19bd4a7ce65
Cr-Commit-Position: refs/heads/master@{#394059}

[Ken Rockot(use gerrit already), 2016-05-17 03:07:34]

Thank you for helping to find these mistakes!

[Ken Rockot(use gerrit already), 2016-05-17 03:07:49]

Description was changed from

==========
[mojo-edk] Sanity checks on porst message parsing

BUG=612364
R=ochang@chromium.org
==========

to

==========
[mojo-edk] Sanity checks on ports message parsing

BUG=612364
R=ochang@chromium.org
==========

[Oliver Chang, 2016-05-17 03:45:22]

thanks for the lightning quick fixes!

https://codereview.chromium.org/1987593002/diff/20001/mojo/edk/system/ports/m...
File mojo/edk/system/ports/message.cc (right):

https://codereview.chromium.org/1987593002/diff/20001/mojo/edk/system/ports/m...
mojo/edk/system/ports/message.cc:61: if (num_bytes < *num_header_bytes +
*num_ports_bytes)
not a concrete issue, but as a long term goal, would it be feasible to convert
these kinds of integer arithmetic (dealing with offsets and sizes) to use the
safe numerics library (//base/numerics) ?

https://codereview.chromium.org/1987593002/diff/20001/mojo/edk/system/ports/m...
mojo/edk/system/ports/message.cc:65: if (*num_header_bytes > num_bytes)
Should this be a != comparison instead of >? (based on the DCHECK on line 69,
which can be removed).

[Ken Rockot(use gerrit already), 2016-05-17 03:59:02]

https://codereview.chromium.org/1987593002/diff/20001/mojo/edk/system/ports/m...
File mojo/edk/system/ports/message.cc (right):

https://codereview.chromium.org/1987593002/diff/20001/mojo/edk/system/ports/m...
mojo/edk/system/ports/message.cc:61: if (num_bytes < *num_header_bytes +
*num_ports_bytes)
On 2016/05/17 at 03:45:22, Oliver Chang wrote:
> not a concrete issue, but as a long term goal, would it be feasible to convert
these kinds of integer arithmetic (dealing with offsets and sizes) to use the
safe numerics library (//base/numerics) ?

Absolutely. Filed http://crbug.com/612374 to do it some time soon. I didn't know
this library existed!

https://codereview.chromium.org/1987593002/diff/20001/mojo/edk/system/ports/m...
mojo/edk/system/ports/message.cc:65: if (*num_header_bytes > num_bytes)
On 2016/05/17 at 03:45:22, Oliver Chang wrote:
> Should this be a != comparison instead of >? (based on the DCHECK on line 69,
which can be removed).

Done

[Oliver Chang, 2016-05-17 04:02:45]

LGTM, thanks!

https://codereview.chromium.org/1987593002/diff/20001/mojo/edk/system/ports/m...
File mojo/edk/system/ports/message.cc (right):

https://codereview.chromium.org/1987593002/diff/20001/mojo/edk/system/ports/m...
mojo/edk/system/ports/message.cc:61: if (num_bytes < *num_header_bytes +
*num_ports_bytes)
On 2016/05/17 03:59:01, Ken Rockot wrote:
> On 2016/05/17 at 03:45:22, Oliver Chang wrote:
> > not a concrete issue, but as a long term goal, would it be feasible to
convert
> these kinds of integer arithmetic (dealing with offsets and sizes) to use the
> safe numerics library (//base/numerics) ?
> 
> Absolutely. Filed http://crbug.com/612374 to do it some time soon. I didn't
know
> this library existed!

woohoo!

[Ken Rockot(use gerrit already), 2016-05-17 04:03:15]

The CQ bit was checked by rockot@chromium.org

[commit-bot: I haz the power, 2016-05-17 04:03:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1987593002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1987593002/40001

[commit-bot: I haz the power, 2016-05-17 05:19:43]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-17 05:22:30]

Description was changed from

==========
[mojo-edk] Sanity checks on ports message parsing

BUG=612364
R=ochang@chromium.org
==========

to

==========
[mojo-edk] Sanity checks on ports message parsing

BUG=612364
R=ochang@chromium.org

Committed: https://crrev.com/73af3951c285542c5397293375e0c19bd4a7ce65
Cr-Commit-Position: refs/heads/master@{#394059}
==========

[commit-bot: I haz the power, 2016-05-17 05:22:32]

Patchset 3 (id:??) landed as
https://crrev.com/73af3951c285542c5397293375e0c19bd4a7ce65
Cr-Commit-Position: refs/heads/master@{#394059}