New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 854975 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

ec: Fuzzing of some host side tests

Project Member Reported by drinkcat@chromium.org, Jun 21 2018

Issue description

It would be great to fuzz the external EC interfaces that can be tested on host.

Namely:
 - Host command interface
 - USB updater interface (for hammer and derivatives)
 - Maybe nvmem_vars

and any other test that can accept some random input buffer.
 
And TPM commands (as in common/tpm_registers.c)

while it is not currently tested on host, this seems definitely possible ?

Comment 2 by ngm@google.com, Jun 21 2018

On a related note, there are some host side fuzz tests for the reference tpm2 code here: https://chromium.googlesource.com/chromiumos/third_party/tpm2/+/master/fuzz/

The tests are run continuously as part of the oss-fuzz project:
https://github.com/google/oss-fuzz/tree/master/projects/tpm2
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/69153048757481a312c0f9f4747ed45565a30c1e

commit 69153048757481a312c0f9f4747ed45565a30c1e
Author: Nicolas Boichat <drinkcat@chromium.org>
Date: Thu Jun 28 05:07:11 2018

core/host: Fall back to udelay when task is invalid

When running fuzzing tests, the sanitizer library may call usleep
from the main thread, and our implementation thinks that usleep
is called from idle task (task_id == 0), and just waits for an
event that will never arrive.

Make sure the default task id is invalid, and fall back to udelay
if we are in an invalid task.

BRANCH=none
BUG=chromium:854975
TEST=Fuzzing tests do not fail with strange errors.

Change-Id: Icc3fdce30b54dfb06913a3d6cbabaa07e1266ba6
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1116623
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

[modify] https://crrev.com/69153048757481a312c0f9f4747ed45565a30c1e/core/host/task.c
[modify] https://crrev.com/69153048757481a312c0f9f4747ed45565a30c1e/core/host/timer.c

Project Member

Comment 4 by bugdroid1@chromium.org, Jun 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/7d55645209d726c4140b439a6ea0de3957f694f8

commit 7d55645209d726c4140b439a6ea0de3957f694f8
Author: Nicolas Boichat <drinkcat@chromium.org>
Date: Thu Jun 28 05:07:11 2018

core/host/task: Fix task_set_event

task_set_event is expected to _add_ the event bit to the current
mask, not reset the whole mask.

Also, fix all operations to use atomics.

BRANCH=none
BUG=chromium:854975
TEST=No more timeouts when running usb_pd fuzzing tests.

Change-Id: Id17428e15f6fb8b52891bed33281f866fbc2be8f
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1116624
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

[modify] https://crrev.com/7d55645209d726c4140b439a6ea0de3957f694f8/core/host/task.c

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/8e2765c4208dc86077ee7932d4e414aa79c3e354

commit 8e2765c4208dc86077ee7932d4e414aa79c3e354
Author: Nicolas Boichat <drinkcat@chromium.org>
Date: Thu Jun 28 14:06:53 2018

host_command: Fix response_size to match data that was copied

Both host_command_read_test and host_command_test_protocol write
back an incorrect response_size, that does not match the number
of bytes that were actually copied.

This is easily noticed when fuzzing with verbose host command
printing, as host_command_debug_request attempts to print
the whole response, reading the response buffer out of bounds.

BRANCH=none
BUG=chromium:854975
TEST=
  #define FUZZ_HOSTCMD_VERBOSE in test/test_config.h

  echo AwoAAAAALADvDAE= | base64 -d > crash
  Request: cmd=0013 data=03df1300007f0b000000007f00007f7f7f7f06
  or
  echo AwMAAEpK | base64 -d > crash
  Request: cmd=0003 data=03650300004a01004a

  make buildfuzztests -j
  ASAN_OPTIONS="log_path=stderr" \
    build/host/host_command_fuzz/host_command_fuzz.exe crash

Change-Id: Ibc8fe958cf6fae38fbfecec558c37ed3d676a51b
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1116199
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

[modify] https://crrev.com/8e2765c4208dc86077ee7932d4e414aa79c3e354/common/host_command.c

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 28 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/cacc1c8ac679b66876cf3e65c58f2bf874498050

commit cacc1c8ac679b66876cf3e65c58f2bf874498050
Author: Nicolas Boichat <drinkcat@chromium.org>
Date: Thu Jun 28 14:06:45 2018

common/printf: snprintf: Return number of bytes on success

As indicated in the man page:
"""
Upon successful return, these functions return the number of
characters printed (excluding the null byte used to end output to
strings).
"""

There are no users of the return value currently in the EC code,
but this matters when doing fuzzing, as libFuzzer calls
std::to_string, which expects the correct return value.

BRANCH=none
BUG=chromium:854975
TEST=make buildfuzztests -j && ASAN_OPTIONS="log_path=stderr" \
     build/host/usb_pd_fuzz/usb_pd_fuzz.exe -jobs=10
     actually creates 10 output files.
TEST=make run-utils_str -j

Change-Id: If6a040f690dd847f4c88c3b8566554afdfbabc32
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1116625
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

[modify] https://crrev.com/cacc1c8ac679b66876cf3e65c58f2bf874498050/common/printf.c
[modify] https://crrev.com/cacc1c8ac679b66876cf3e65c58f2bf874498050/test/utils_str.c

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 21

Labels: merge-merged-firmware-cr50-9308.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/4fd62b28f3ed3a2e90e60697c96a849525e15dd8

commit 4fd62b28f3ed3a2e90e60697c96a849525e15dd8
Author: Nicolas Boichat <drinkcat@chromium.org>
Date: Sat Jul 21 00:25:18 2018

common/printf: snprintf: Return number of bytes on success

As indicated in the man page:
"""
Upon successful return, these functions return the number of
characters printed (excluding the null byte used to end output to
strings).
"""

 Conflicts:
	test/utils_str.c not included in this patch

There are no users of the return value currently in the EC code,
but this matters when doing fuzzing, as libFuzzer calls
std::to_string, which expects the correct return value.

BRANCH=none
BUG=chromium:854975
TEST=make buildfuzztests -j && ASAN_OPTIONS="log_path=stderr" \
     build/host/usb_pd_fuzz/usb_pd_fuzz.exe -jobs=10
     actually creates 10 output files.
TEST=make run-utils_str -j

Change-Id: If6a040f690dd847f4c88c3b8566554afdfbabc32
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1116625
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
(cherry picked from commit cacc1c8ac679b66876cf3e65c58f2bf874498050)
Reviewed-on: https://chromium-review.googlesource.com/1145845
Tested-by: Vadim Bendebury <vbendeb@chromium.org>
Commit-Queue: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>

[modify] https://crrev.com/4fd62b28f3ed3a2e90e60697c96a849525e15dd8/common/printf.c

Project Member

Comment 8 by bugdroid1@chromium.org, Aug 3

Labels: merge-merged-firmware-cr50-mp-release-9308.87.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/ef835813087ec3ba5e667ec25271cd0c6acdf132

commit ef835813087ec3ba5e667ec25271cd0c6acdf132
Author: Nicolas Boichat <drinkcat@chromium.org>
Date: Fri Aug 03 19:50:49 2018

common/printf: snprintf: Return number of bytes on success

As indicated in the man page:
"""
Upon successful return, these functions return the number of
characters printed (excluding the null byte used to end output to
strings).
"""

 Conflicts:
	test/utils_str.c not included in this patch

There are no users of the return value currently in the EC code,
but this matters when doing fuzzing, as libFuzzer calls
std::to_string, which expects the correct return value.

BRANCH=none
BUG=chromium:854975
TEST=make buildfuzztests -j && ASAN_OPTIONS="log_path=stderr" \
     build/host/usb_pd_fuzz/usb_pd_fuzz.exe -jobs=10
     actually creates 10 output files.
TEST=make run-utils_str -j

Change-Id: If6a040f690dd847f4c88c3b8566554afdfbabc32
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1116625
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
(cherry picked from commit cacc1c8ac679b66876cf3e65c58f2bf874498050)
Reviewed-on: https://chromium-review.googlesource.com/1145845
Tested-by: Vadim Bendebury <vbendeb@chromium.org>
Commit-Queue: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1162612
Commit-Queue: Mary Ruthven <mruthven@chromium.org>
Tested-by: Mary Ruthven <mruthven@chromium.org>
Reviewed-by: Mary Ruthven <mruthven@chromium.org>

[modify] https://crrev.com/ef835813087ec3ba5e667ec25271cd0c6acdf132/common/printf.c

Project Member

Comment 9 by bugdroid1@chromium.org, Aug 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/4a4e2c71a0f6aaa50e0728922f84a7d54c14380a

commit 4a4e2c71a0f6aaa50e0728922f84a7d54c14380a
Author: Nicolas Boichat <drinkcat@chromium.org>
Date: Thu Aug 16 07:30:08 2018

test: host_command_fuzz: fuzzing test

Writing fuzzing tests is a little tricky, as clang takes over the main
function. Instead, we start the test main function in a thread, and
have LLVMFuzzerTestOneInput prepare the host command buffer, and
wake the TEST_RUNNER task.

To make fuzzing faster, we only send somehow correctly formed requests,
with a valid checksum and length (this can be disabled with an option).

We also make sure that the emulator does not hibernate, reboot or jump
to a different image when fuzzing is enabled.

BRANCH=none
BUG=chromium:854975
TEST=make buildfuzztests -j
     ASAN_OPTIONS="log_path=stderr" \
         build/host/host_command_fuzz/host_command_fuzz.exe -timeout=5

Change-Id: I27b25e44c405f118dfc1296247479245e15e54b4
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1107523
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Jonathan Metzman <metzman@chromium.org>

[modify] https://crrev.com/4a4e2c71a0f6aaa50e0728922f84a7d54c14380a/include/system.h
[modify] https://crrev.com/4a4e2c71a0f6aaa50e0728922f84a7d54c14380a/common/system.c
[modify] https://crrev.com/4a4e2c71a0f6aaa50e0728922f84a7d54c14380a/include/test_util.h
[modify] https://crrev.com/4a4e2c71a0f6aaa50e0728922f84a7d54c14380a/chip/host/reboot.c
[add] https://crrev.com/4a4e2c71a0f6aaa50e0728922f84a7d54c14380a/test/host_command_fuzz.tasklist
[modify] https://crrev.com/4a4e2c71a0f6aaa50e0728922f84a7d54c14380a/test/build.mk
[modify] https://crrev.com/4a4e2c71a0f6aaa50e0728922f84a7d54c14380a/chip/host/reboot.h
[modify] https://crrev.com/4a4e2c71a0f6aaa50e0728922f84a7d54c14380a/chip/host/system.c
[modify] https://crrev.com/4a4e2c71a0f6aaa50e0728922f84a7d54c14380a/chip/host/uart.c
[add] https://crrev.com/4a4e2c71a0f6aaa50e0728922f84a7d54c14380a/test/host_command_fuzz.c
[modify] https://crrev.com/4a4e2c71a0f6aaa50e0728922f84a7d54c14380a/Makefile.toolchain
[modify] https://crrev.com/4a4e2c71a0f6aaa50e0728922f84a7d54c14380a/core/host/task.c
[modify] https://crrev.com/4a4e2c71a0f6aaa50e0728922f84a7d54c14380a/test/test_config.h
[modify] https://crrev.com/4a4e2c71a0f6aaa50e0728922f84a7d54c14380a/core/host/main.c
[modify] https://crrev.com/4a4e2c71a0f6aaa50e0728922f84a7d54c14380a/Makefile.rules

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/a5bcb27761a24a8ce132eaf0f4ddf3c2e41dd667

commit a5bcb27761a24a8ce132eaf0f4ddf3c2e41dd667
Author: Nicolas Boichat <drinkcat@chromium.org>
Date: Thu Aug 16 11:25:49 2018

chromeos-ec: Build and install fuzzer when USE flag is set

CQ-DEPEND=CL:1107523
BUG=chromium:854975
TEST=USE="fuzzer" emerge-amd64-generic -av chromeos-ec
   sudo chroot ./chromiumos/chroot/build/amd64-generic
   ASAN_OPTIONS="log_path=stderr" \
      /usr/libexec/fuzzers/ec_host_command_fuzzer

Change-Id: I7ca3a97d093d437e519b7371761929cdf113843f
Reviewed-on: https://chromium-review.googlesource.com/1113708
Commit-Ready: Manoj Gupta <manojgupta@chromium.org>
Tested-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>

[modify] https://crrev.com/a5bcb27761a24a8ce132eaf0f4ddf3c2e41dd667/chromeos-base/chromeos-ec/chromeos-ec-9999.ebuild

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/be816f9f86e34ca0fe854d6ce66df04b50d09087

commit be816f9f86e34ca0fe854d6ce66df04b50d09087
Author: Nicolas Boichat <drinkcat@chromium.org>
Date: Thu Aug 16 16:16:13 2018

virtual/chromium-os-fuzzers: Add chromeos-ec to fuzzers

BUG=chromium:854975
TEST=tryjob

Change-Id: I1edffd2d604c8328fe01b659794919868b41b537
Reviewed-on: https://chromium-review.googlesource.com/1113709
Commit-Ready: Manoj Gupta <manojgupta@chromium.org>
Tested-by: Manoj Gupta <manojgupta@chromium.org>
Reviewed-by: Manoj Gupta <manojgupta@chromium.org>

[modify] https://crrev.com/be816f9f86e34ca0fe854d6ce66df04b50d09087/virtual/chromium-os-fuzzers/chromium-os-fuzzers-1.ebuild
[rename] https://crrev.com/be816f9f86e34ca0fe854d6ce66df04b50d09087/virtual/chromium-os-fuzzers/chromium-os-fuzzers-1-r10.ebuild

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 5

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/d2602418362812f8970616b4dc708d8847291df9

commit d2602418362812f8970616b4dc708d8847291df9
Author: Nicolas Boichat <drinkcat@chromium.org>
Date: Wed Dec 05 09:13:38 2018

test/usb_pd_fuzz: Fuzzing of USB PD data

Setup CC lines, then send up to 8 PD messages, in an attempt to
cause errors while parsing PDO and other messages.

BRANCH=none
BUG=chromium:854975
TEST=make -j buildfuzztests && \
     ./build/host/usb_pd_fuzz/usb_pd_fuzz.exe > /dev/null

Change-Id: Ibb575ea8d464945390d1663dd6fff279bd9d77ea
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1116626
Reviewed-by: Jonathan Metzman <metzman@chromium.org>

[add] https://crrev.com/d2602418362812f8970616b4dc708d8847291df9/fuzz/usb_pd_fuzz.tasklist
[modify] https://crrev.com/d2602418362812f8970616b4dc708d8847291df9/fuzz/build.mk
[modify] https://crrev.com/d2602418362812f8970616b4dc708d8847291df9/chip/host/build.mk
[modify] https://crrev.com/d2602418362812f8970616b4dc708d8847291df9/fuzz/fuzz_config.h
[modify] https://crrev.com/d2602418362812f8970616b4dc708d8847291df9/common/usb_pd_protocol.c
[add] https://crrev.com/d2602418362812f8970616b4dc708d8847291df9/fuzz/usb_pd_fuzz.c
[modify] https://crrev.com/d2602418362812f8970616b4dc708d8847291df9/core/host/main.c

Project Member

Comment 13 by bugdroid1@chromium.org, Dec 6

Labels: merge-merged-firmware-octopus-11297.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/c6890950c14e658ccae906320276fcfd123f7bb7

commit c6890950c14e658ccae906320276fcfd123f7bb7
Author: Nicolas Boichat <drinkcat@chromium.org>
Date: Thu Dec 06 14:40:57 2018

test/usb_pd_fuzz: Fuzzing of USB PD data

Setup CC lines, then send up to 8 PD messages, in an attempt to
cause errors while parsing PDO and other messages.

BRANCH=none
BUG=chromium:854975
TEST=make -j buildfuzztests && \
     ./build/host/usb_pd_fuzz/usb_pd_fuzz.exe > /dev/null

Change-Id: Ibb575ea8d464945390d1663dd6fff279bd9d77ea
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1116626
Reviewed-by: Jonathan Metzman <metzman@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/1365651
Reviewed-by: Jett Rink <jettrink@chromium.org>
Commit-Queue: Jett Rink <jettrink@chromium.org>
Tested-by: Jett Rink <jettrink@chromium.org>
Trybot-Ready: Jett Rink <jettrink@chromium.org>

[add] https://crrev.com/c6890950c14e658ccae906320276fcfd123f7bb7/fuzz/usb_pd_fuzz.tasklist
[modify] https://crrev.com/c6890950c14e658ccae906320276fcfd123f7bb7/fuzz/build.mk
[modify] https://crrev.com/c6890950c14e658ccae906320276fcfd123f7bb7/chip/host/build.mk
[modify] https://crrev.com/c6890950c14e658ccae906320276fcfd123f7bb7/fuzz/fuzz_config.h
[modify] https://crrev.com/c6890950c14e658ccae906320276fcfd123f7bb7/common/usb_pd_protocol.c
[add] https://crrev.com/c6890950c14e658ccae906320276fcfd123f7bb7/fuzz/usb_pd_fuzz.c
[modify] https://crrev.com/c6890950c14e658ccae906320276fcfd123f7bb7/core/host/main.c

Sign in to add a comment