Received-SPF: pass (google.com: domain of linux-kernel+bounces-37480-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: Elizabeth Figura <zfigura@codeweavers.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
 linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
 Arnd Bergmann <arnd@arndb.de>
Cc: wine-devel@winehq.org,
 =?ISO-8859-1?Q?Andr=E9?= Almeida <andrealmeid@igalia.com>,
 Wolfram Sang <wsa@kernel.org>, Arkadiusz Hiler <ahiler@codeweavers.com>,
 Peter Zijlstra <peterz@infradead.org>
Subject: Re: [RFC PATCH 5/9] ntsync: Introduce NTSYNC_IOC_WAIT_ANY.
Date: Wed, 24 Jan 2024 12:02:32 -0600
Message-ID: <4864383.GXAFRqVoOG@camazotz>
In-Reply-To: <18c814fa-b458-48f9-b7e8-88b23a1825e2@app.fastmail.com>
References:
 <20240124004028.16826-1-zfigura@codeweavers.com>
 <20240124004028.16826-6-zfigura@codeweavers.com>
 <18c814fa-b458-48f9-b7e8-88b23a1825e2@app.fastmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"

On Wednesday, 24 January 2024 01:56:52 CST Arnd Bergmann wrote:
> On Wed, Jan 24, 2024, at 01:40, Elizabeth Figura wrote:
> 
> > +	if (args->timeout) {
> > +		struct timespec64 to;
> > +
> > +		if (get_timespec64(&to, u64_to_user_ptr(args->timeout)))
> > +			return -EFAULT;
> > +		if (!timespec64_valid(&to))
> > +			return -EINVAL;
> > +
> > +		timeout = timespec64_to_ns(&to);
> > +	}
> 
> Have you considered just passing the nanosecond value here?
> Since you do not appear to write it back, that would avoid
> the complexities of dealing with timespec layout differences
> and indirection.

That'd be nicer in general. I think there was some documentation that advised
using timespec64 for new ioctl interfaces but it may have been outdated or
misread.

> 
> > +	ids = kmalloc_array(count, sizeof(*ids), GFP_KERNEL);
> > +	if (!ids)
> > +		return -ENOMEM;
> > +	if (copy_from_user(ids, u64_to_user_ptr(args->objs),
> > +			   array_size(count, sizeof(*ids)))) {
> > +		kfree(ids);
> > +		return -EFAULT;
> > +	}
> 
> This looks like memdup_user() would be slightly simpler.

That's useful, thanks.