usb: typec: ucsi: displayport: data race on dp->header/ vdo_data/vdo_size -- work reads them unlocked while writers hold con->lock (VDM mismatch)

[徐晗翔 <xuhx@hust.edu.cn>]

The UCSI displayport altmode's reply triple (dp->header / vdo_data /
vdo_size, ucsi/displayport.c:28-30) is written by three writers that
all hold con->lock (enter :97-102, exit :142-147, vdm :234+) but read
by ucsi_displayport_work (:282-295) with NO lock at all -- the work
is scheduled asynchronously (:104/:149/:265) and never acquires
con->lock, so the lock only serializes the writers among themselves
and does not cover the reader.  When the worker runs while a writer
updates the triple (the mode_selection activate path entering
ucsi_displayport_enter, or the next EC event arriving while the
worker is inside typec_altmode_vdm -> the partner driver), the VDM
message assembled from the mixed fields is sent to the partner
driver: a stale/mixed header with fresh data or vice versa -- a DP
state machine glitch that the partner sees as a malformed or
misdirected VDM (self-healing on the next event, but still a protocol
violation and a plain KCSAN data race).  Lifetime is NOT at risk
(vdo_data only ever points at the persistent &dp->data.status or
NULL; the worker is cancel_work_sync'ed before teardown), so this is
a small locking fix: read/clear the triple under the same lock (or a
dedicated dp->lock) in the worker.

The detailed description:

* The racy pair (Linux 7.3, drivers/usb/typec/ucsi/displayport.c)

  Writers (all inside ucsi_con_mutex_lock ... unlock, :57/:119/:209):
    - enter path: :97-102 sets dp->header = ENTER_ACK (with
      vdo_data/vdo_size) and schedules dp->work (:104);
    - exit path: :142-147 + :149;
    - vdm path: :234+ (with :183-184 for the ATTENTION-derived
      variant) + :265.

  Reader (no lock): ucsi_displayport_work (:282-295) reads
  dp->header / dp->vdo_data / dp->vdo_size (:287-288) and clears
  them (:292-294), then calls typec_altmode_vdm(dp->alt, ...) ->
  bus.c:214-235 -> the partner driver's dp_altmode_vdm
  (altmodes/displayport.c:372-443, under the driver's own dp->lock).

  The worker never takes con->lock, and nothing stops a writer from
  taking con->lock while the unlocked worker runs (the mode_selection
  activate path enters ucsi_displayport_enter without holding
  con->lock -- it takes it inside).  schedule_work coalescing does
  not close the window.

* Concrete interleavings

  1. mode_selection activate vs an in-flight worker: the activate
     path (thread A) writes ENTER_ACK and schedules; the previous
     round's worker (thread B) is concurrently reading the old
     header/vdo_data at :287-288 -- the partner receives a mixed
     old/new VDM.
  2. back-to-back vdm events: the worker is inside
     typec_altmode_vdm (which enters the partner driver and takes
     its dp->lock -- nonzero time), the next EC event drives
     ucsi_displayport_vdm to overwrite the triple under con->lock --
     same mixed-VDM outcome.

* Scope / severity

  vdo_data only ever points at the persistent &dp->data.status (or
  NULL, :101/:146/:183) and vdo_size is a u8 -- no dangling pointer,
  no UAF; the worker is drained by ucsi_displayport_remove_partner's
  cancel_work_sync (:297-313, :308) before the altmode teardown
  (verified: ucsi.c:834-841 ordering).  So the impact is: KCSAN
  data-race reports plus transient VDM mismatches (partner-side DP
  state machine warnings/retries; self-healing).  Severity low; the
  fix is small.

* Suggested fix 

  Have ucsi_displayport_work() snapshot the triple under
  ucsi_con_mutex_lock(dp->con) (or a new dedicated dp->lock shared by
  writers and the worker), then send the snapshot outside the lock.
  Alternatively, writers coalesce under the lock with a
  cancel_work_sync before overwriting -- more costly; the snapshot
  is the clean minimal fix.


  Please keep me in Cc; I am not subscribed to the lists.



Regards,
Hanxiang Xu
xuhx@hust.edu.cn