linux-kernel - Re: [PATCH v3 2/4] ptp: vmclock: support device notifications

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <da92811ade53edfa1ced7995fc4344f557a4d431.camel@infradead.org>
Date: Wed, 03 Dec 2025 16:52:02 +0000
From: David Woodhouse <dwmw2@...radead.org>
To: "Chalios, Babis" <bchalios@...zon.es>, "robh@...nel.org"
 <robh@...nel.org>,  "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
 "conor+dt@...nel.org" <conor+dt@...nel.org>,  "richardcochran@...il.com"
 <richardcochran@...il.com>, "andrew+netdev@...n.ch"
 <andrew+netdev@...n.ch>,  "davem@...emloft.net" <davem@...emloft.net>,
 "edumazet@...gle.com" <edumazet@...gle.com>,  "kuba@...nel.org"
 <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>
Cc: "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, 
 "netdev@...r.kernel.org"
	 <netdev@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	 <linux-kernel@...r.kernel.org>, "Graf (AWS), Alexander" <graf@...zon.de>, 
 "mzxreary@...inter.de"
	 <mzxreary@...inter.de>, "Cali, Marco" <xmarcalx@...zon.co.uk>
Subject: Re: [PATCH v3 2/4] ptp: vmclock: support device notifications

On Wed, 2025-12-03 at 12:36 +0000, Chalios, Babis wrote:
> From: "Chalios, Babis" <bchalios@...zon.es>
> 
> Add optional support for device notifications in VMClock. When
> supported, the hypervisor will send a device notification every time it
> updates the seq_count to a new even value.
> 
> Moreover, add support for poll() in VMClock as a means to propagate this
> notification to user space. poll() will return a POLLIN event to
> listeners every time seq_count changes to a value different than the one
> last seen (since open() or last read()/pread()). This means that when
> poll() returns a POLLIN event, listeners need to use read() to observe
> what has changed and update the reader's view of seq_count. In other
> words, after a poll() returned, all subsequent calls to poll() will
> immediately return with a POLLIN event until the listener calls read().
> 
> The device advertises support for the notification mechanism by setting
> flag VMCLOCK_FLAG_NOTIFICATION_PRESENT in vmclock_abi flags field. If
> the flag is not present the driver won't setup the ACPI notification
> handler and poll() will always immediately return POLLHUP.
> 
> Signed-off-by: Babis Chalios <bchalios@...zon.es>

Mostly looks good to me; thanks. However... 

> +static __poll_t vmclock_miscdev_poll(struct file *fp, poll_table *wait)
> +{
> +	struct vmclock_file_state *fst = fp->private_data;
> +	struct vmclock_state *st = fst->st;
> +	uint32_t seq;
> +
> +	/*
> +	 * Hypervisor will not send us any notifications, so fail immediately
> +	 * to avoid having caller sleeping for ever.
> +	 */
> +	if (!(st->clk->flags & VMCLOCK_FLAG_NOTIFICATION_PRESENT))
> +		return POLLHUP;

Missing le64_to_cpu() there. And I guess *strictly* speaking we should
do the seq_lock dance whenever we read the dynamic fields, although
that only ever matters if a hypervisor were to bump seq_count to an odd
value, *then* set the VMCLOCK_FLAG_NOTIFICATION_PRESENT flag, then
clear the flag again before bumping seq_count to even, and blame the
*guest* for looking at the wrong time. Which is frankly insane, and I
don't think I care.

So with the le64_to_cpu() added, 

Reviewed-by: David Woodhouse <dwmw@...zon.co.uk>

Don't post again for a few days though; three versions in 48 hours is
enough :)


> +static int vmclock_miscdev_release(struct inode *inode, struct file *fp)
> +{
> +	kfree(fp->private_data);
> +	return 0;
> +}

Still bugs me a little. I note simple_attr_release() is exported and
does the same, but I guess we'd want to move and rename that before we
try to use it from places like this.


Download attachment "smime.p7s" of type "application/pkcs7-signature" (5069 bytes)