Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Uninitialized stack memory read / Undefined Behavior in `vmclock_get_crosststamp()` -- commit 9639c73bd7986c7a2cf17a27c0bf79db7385ce83 Author: David Woodhouse <[email protected]> ptp: vmclock: Use hw_cycles from snapshot for precise TSC pairing When the system clocksource is kvmclock or Hyper-V, vmclock_get_crosststamp() previously fell through to get_cycles(). This patch uses hw_cycles from ktime_get_snapshot_id() when available, giving a perfect pairing for the device time calculation. > diff --git a/drivers/ptp/ptp_vmclock.c b/drivers/ptp/ptp_vmclock.c > index d6a5a533164aa..eebdcd5ebc080 100644 > --- a/drivers/ptp/ptp_vmclock.c > +++ b/drivers/ptp/ptp_vmclock.c [ ... ] > @@ -140,6 +140,10 @@ static int vmclock_get_crosststamp(struct vmclock_state > *st, > if (sts->pre_sts.cs_id == st->cs_id) { > cycle = sts->pre_sts.cycles; > sts->post_sts = sts->pre_sts; > + } else if (sts->pre_sts.hw_csid == st->cs_id && > + sts->pre_sts.hw_cycles) { [Severity: High] This is a pre-existing issue, but by adding these checks, does this code evaluate uninitialized stack memory? Looking at ptp_sys_offset_extended() in drivers/ptp/ptp_chardev.c, the struct ptp_system_timestamp sts is allocated on the stack but not zero-initialized. When ptp_read_system_prets(&sts) calls ktime_get_snapshot_id(), it sets systime_snapshot->valid = false and returns early in failure cases (e.g., if an invalid or unavailable AUX clock ID is passed). In these cases, the cs_id, hw_csid, and hw_cycles fields remain uninitialized stack garbage. vmclock_get_crosststamp() evaluates these branch conditions without first checking if sts->pre_sts.valid is true. Although the sample is eventually discarded by ptp_sys_offset_extended() because sts.pre_sts.valid is false, branching on uninitialized stack memory is undefined behavior and can trigger KMSAN warnings. Could a check for sts->pre_sts.valid be added before evaluating these fields? > + cycle = sts->pre_sts.hw_cycles; > + sts->post_sts = sts->pre_sts; > } else { > cycle = get_cycles(); > ptp_read_system_postts(sts); -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
