Re: [PATCH v2 3/3] ptp: vmclock: Use hw_cycles from snapshot for precise TSC pairing

[sashiko-bot@kernel.org]

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 <dwmw@amazon.co.uk>

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/20260604095755.64849-1-dwmw2@infradead.org?part=3