netdev - Re: [PATCH net-next v4 4/6] ptp: support event queue reader channel masks

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: <ZR/pA46huaSGgatd@kernel.org>
Date: Fri, 6 Oct 2023 13:01:23 +0200
From: Simon Horman <horms@...nel.org>
To: Xabier Marquiegui <reibax@...il.com>
Cc: netdev@...r.kernel.org, richardcochran@...il.com, tglx@...utronix.de,
	jstultz@...gle.com, chrony-dev@...ony.tuxfamily.org,
	mlichvar@...hat.com, ntp-lists@...tcorallo.com,
	vinicius.gomes@...el.com, alex.maftei@....com, davem@...emloft.net,
	rrameshbabu@...dia.com, shuah@...nel.org
Subject: Re: [PATCH net-next v4 4/6] ptp: support event queue reader channel
 masks

On Thu, Oct 05, 2023 at 03:53:14PM +0200, Xabier Marquiegui wrote:
> On systems with multiple timestamp event channels, some readers might
> want to receive only a subset of those channels.
> 
> This patch adds the necessary modifications to support timestamp event
> channel filtering, including two IOCTL operations:
> 
> - Clear all channels
> - Enable one channel
> 
> The mask modification operations will be applied exclusively on the
> event queue assigned to the file descriptor used on the IOCTL operation,
> so the typical procedure to have a reader receiving only a subset of the
> enabled channels would be:
> 
> - Open device file
> - ioctl: clear all channels
> - ioctl: enable one channel
> - start reading
> 
> Calling the enable one channel ioctl more than once will result in
> multiple enabled channels.
> 
> Signed-off-by: Xabier Marquiegui <reibax@...il.com>
> Suggested-by: Richard Cochran <richardcochran@...il.com>
> Suggested-by: Vinicius Costa Gomes <vinicius.gomes@...el.com>
> ---
> v4:
>   - split modifications in different patches for improved organization
>   - filter modifications exclusive to currently open instance for
>     simplicity and security
>   - expand mask to 2048 channels
>   - remove unnecessary tests
> v3: https://lore.kernel.org/netdev/20230928133544.3642650-4-reibax@gmail.com/
>   - filter application by object id, aided by process id
>   - friendlier testptp implementation of event queue channel filters
> v2: https://lore.kernel.org/netdev/20230912220217.2008895-3-reibax@gmail.com/
>   - fix testptp compilation error: unknown type name 'pid_t'
>   - rename mask variable for easier code traceability
>   - more detailed commit message with two examples
> v1: https://lore.kernel.org/netdev/20230906104754.1324412-4-reibax@gmail.com/
> ---
>  drivers/ptp/ptp_chardev.c      | 24 ++++++++++++++++++++++++
>  drivers/ptp/ptp_clock.c        | 12 ++++++++++--
>  drivers/ptp/ptp_private.h      |  3 +++
>  include/uapi/linux/ptp_clock.h |  2 ++
>  4 files changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
> index abe94bb80cf6..dbbe551a044f 100644
> --- a/drivers/ptp/ptp_chardev.c
> +++ b/drivers/ptp/ptp_chardev.c
> @@ -110,6 +110,10 @@ int ptp_open(struct posix_clock_context *pccontext, fmode_t fmode)
>  	queue = kzalloc(sizeof(*queue), GFP_KERNEL);
>  	if (!queue)
>  		return -EINVAL;
> +	queue->mask = bitmap_alloc(PTP_MAX_CHANNELS, GFP_KERNEL);
> +	if (!queue->mask)

Hi Xabier,

queue appears to be leaked here.

As flagged by Smatch.

> +		return -EINVAL;
> +	bitmap_set(queue->mask, 0, PTP_MAX_CHANNELS);
>  	spin_lock_init(&queue->lock);
>  	list_add_tail(&queue->qlist, &ptp->tsevqs);
>  	pccontext->private_clkdata = queue;