netdev - Re: [PATCH RFC net-next] ptp: Add file permission checks on PHC chardevs

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20250201160237.GB211663@kernel.org>
Date: Sat, 1 Feb 2025 16:02:37 +0000
From: Simon Horman <horms@...nel.org>
To: Wojtek Wasko <wwasko@...dia.com>
Cc: netdev@...r.kernel.org, Richard Cochran <richardcochran@...il.com>
Subject: Re: [PATCH RFC net-next] ptp: Add file permission checks on PHC
 chardevs

On Fri, Jan 31, 2025 at 06:29:23PM +0000, Wojtek Wasko wrote:
> Udev sets strict 600 permissions on /dev/ptp* devices, preventing
> unprivileged users from accessing the time [1]. This patch enables
> more granular permissions and allows readonly access to the PTP clocks.
> 
> Add permission checking for ioctls which modify the state of device.
> Notably, require WRITE for polling as it is only used for later reading
> timestamps from the queue (there is no peek support). POSIX clock
> operations (settime, adjtime) are checked in the POSIX layer.
> 
> [1] https://lists.nwtime.org/sympa/arc/linuxptp-users/2024-01/msg00036.html
> 
> Signed-off-by: Wojtek Wasko <wwasko@...dia.com>

...

> @@ -516,9 +554,15 @@ __poll_t ptp_poll(struct posix_clock_context *pccontext, struct file *fp,
>  {
>  	struct ptp_clock *ptp =
>  		container_of(pccontext->clk, struct ptp_clock, clock);
> +	struct ptp_private_ctxdata *ctxdata;
>  	struct timestamp_event_queue *queue;
>  
> -	queue = pccontext->private_clkdata;
> +	ctxdata = pccontext->private_clkdata;
> +	if (!ctxdata)
> +		return EPOLLERR;
> +	if ((ctxdata->fmode & FMODE_WRITE) == 0)
> +		return EACCES;

Hi Wojtek,

This is not a full review, but rather, something to take into account
if this idea goes forwards:

The return type of this function is __poll_t, not int.
So I think this should be EPOLLERR rather than EACCESS.

> +	queue = ctxdata->queue;
>  	if (!queue)
>  		return EPOLLERR;
>  

...