ec: Fix MKBP FIFO locking |
|||||
Issue descriptionmkbp_fifo_add() is called by various tasks and locks a mutex to prevent reentrancy. fifo_remove() is called from the hostcmd task only and doesn't lock a mutex. mkbp_fifo_add() and fifo_remove() are carefully written to not be in conflict, and I think they would work fine, if not for keyboard_clear_buffer(). keyboard_clear_buffer() is called from the keyscan task and will zero all FIFO variables (and the FIFO itself) without any locking. If keyboard_clear_buffer() preempts fifo_remove just before the call to atomic_sub(), fifo_entries will become (uint32_t)-1. If keyboard_clear_buffer() preempts mkbp_fifo_add() just before the call to atomic_add(), fifo_entries will become 1.
,
Nov 14 2017
,
Jan 22 2018
,
Jan 23 2018
,
Mar 1 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/ec/+/d3eb899c6f5ab5bf7a11ab79d4fd631b0c7241a5 commit d3eb899c6f5ab5bf7a11ab79d4fd631b0c7241a5 Author: Shawn Nematbakhsh <shawnn@chromium.org> Date: Thu Mar 01 18:13:22 2018 keyboard_mkbp: Fix FIFO locking keyboard_clear_buffer() should not trash FIFO contents without synchronization from fifo_add() / fifo_remove(), otherwise a bad FIFO state may ensue. BUG= chromium:781554 BRANCH=gru TEST=Verify KB is functional on kevin through suspend / resume, verify keyboard functions as S3 wake. Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org> Change-Id: I5d28c72359f6e1ce8778725a15c51cdfcd8ab90b Reviewed-on: https://chromium-review.googlesource.com/761300 Commit-Ready: Shawn N <shawnn@chromium.org> Tested-by: Shawn N <shawnn@chromium.org> Reviewed-by: Aseda Aboagye <aaboagye@chromium.org> (cherry picked from commit 14ef8d73f1df635c26477687033de3539f6591e9) Reviewed-on: https://chromium-review.googlesource.com/938322 Tested-by: Aseda Aboagye <aaboagye@chromium.org> [modify] https://crrev.com/d3eb899c6f5ab5bf7a11ab79d4fd631b0c7241a5/common/keyboard_mkbp.c |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by bugdroid1@chromium.org
, Nov 11 2017